Intg-1845 VIN decoding adjustments (#298)

* tweaks to decoding logic, remove old stuff, logging

* reorder decoders, add logging

* remove deprecated code

* update tests

* remove unused file

Author: JamesReate

internal/api/common/behaviors.go

@@ -1,10 +1,11 @@
 package handlers
 
 import (
-	"github.com/tidwall/sjson"
 	"strconv"
 	"strings"
 
+	"github.com/tidwall/sjson"
+
 	"github.com/DIMO-Network/device-definitions-api/internal/api/common"
 
 	p_grpc "github.com/DIMO-Network/device-definitions-api/pkg/grpc"

internal/api/handlers/device_definition_handler.go

@@ -6,7 +6,6 @@ import (
 	"database/sql"
 	"fmt"
 	"strconv"
-	"strings"
 	"time"
 
 	"github.com/tidwall/sjson"
@@ -45,11 +44,10 @@ type DecodeVINQueryHandler struct {
 }
 
 type DecodeVINQuery struct {
-	VIN          string `json:"vin"`
-	KnownModel   string `json:"knownModel"`
-	KnownYear    int32  `json:"knownYear"`
-	Country      string `json:"country"`
-	DefinitionID string `json:"definition_id"`
+	VIN        string `json:"vin"`
+	KnownModel string `json:"knownModel"`
+	KnownYear  int32  `json:"knownYear"`
+	Country    string `json:"country"`
 }
 
 func (*DecodeVINQuery) Key() string { return "DecodeVINQuery" }
@@ -93,11 +91,10 @@ func (dc DecodeVINQueryHandler) Handle(ctx context.Context, query mediator.Messa
 		Logger()
 
 	const (
-		VinRequests              = "VIN_All_Request"
-		VinSuccess               = "VIN_Success_Request"
-		VinExists                = "VIN_Exists_Request"
-		VinErrors                = "VIN_Error_Request"
-		DeviceDefinitionOverride = "Device_Definition_Override"
+		VinRequests = "VIN_All_Request"
+		VinSuccess  = "VIN_Success_Request"
+		VinExists   = "VIN_Exists_Request"
+		VinErrors   = "VIN_Error_Request"
 	)
 
 	metrics.Success.With(prometheus.Labels{"method": VinRequests}).Inc()
@@ -119,72 +116,7 @@ func (dc DecodeVINQueryHandler) Handle(ctx context.Context, query mediator.Messa
 		return r, nil
 	}
 
-	// todo: this should be a separate specific gRPC endpoint for setting or updating vin number to DD mapping
-	// If DefinitionID passed in, override VIN decoding
-	localLog.Info().Msgf("Start Decode VIN for vin %s and device definition %s", vin.String(), qry.DefinitionID)
-	if len(qry.DefinitionID) > 0 {
-		tblDef, _, err := dc.deviceDefinitionOnChainService.GetDefinitionByID(ctx, qry.DefinitionID)
-		if err != nil {
-			return nil, errors.Wrapf(err, "failed to get device definition id: %s", qry.DefinitionID)
-		}
-		makeSlug := strings.Split(tblDef.ID, "_")[0]
-		dm, err := dc.identity.GetManufacturer(makeSlug)
-		if err != nil {
-			return nil, errors.Wrapf(err, "failed to get device make for: %s", qry.DefinitionID)
-		}
-		dbWMI, err := dc.vinRepository.GetOrCreateWMI(ctx, wmi, dm.Name)
-		if err != nil {
-			metrics.InternalError.With(prometheus.Labels{"method": VinErrors}).Inc()
-			localLog.Error().Err(err).Msgf("failed to get or create wmi for vin %s", vin.String())
-			return resp, nil
-		}
-
-		// insert vin_numbers
-		vinDecodeNumber = &models.VinNumber{
-			Vin:              vin.String(),
-			ManufacturerName: dm.Name,
-			Wmi:              null.StringFrom(dbWMI.Wmi),
-			VDS:              null.StringFrom(vin.VDS()),
-			Vis:              null.StringFrom(vin.VIS()),
-			CheckDigit:       null.StringFrom(vin.CheckDigit()),
-			SerialNumber:     vin.SerialNumber(),
-			DecodeProvider:   null.StringFrom("manual"),
-			Year:             tblDef.Year,
-			DefinitionID:     tblDef.ID,
-		}
-
-		split := strings.Split(vinDecodeNumber.DefinitionID, "_")
-		if len(split) != 3 {
-			return nil, errors.New("invalid definition ID encountered: " + vinDecodeNumber.DefinitionID)
-		}
-
-		// no style, maybe for future way to pick the Style from Admin
-
-		// note we use a transaction here all throughout and commit at the end
-		if err = vinDecodeNumber.Insert(ctx, txVinNumbers, boil.Infer()); err != nil {
-			localLog.Err(err).
-				Str("definition_id", tblDef.ID).
-				Msg("failed to insert to vin_numbers")
-		}
-		err = txVinNumbers.Commit()
-		if err != nil {
-			return nil, errors.Wrap(err, "error when commiting transaction for inserting vin_number")
-		}
-
-		resp.Manufacturer = dm.Name
-		resp.Year = int32(vinDecodeNumber.Year)
-		resp.Source = vinDecodeNumber.DecodeProvider.String
-		pt, err := dc.powerTrainTypeService.ResolvePowerTrainType(split[0], split[1], null.JSON{}, null.JSON{})
-		if err != nil {
-			pt = coremodels.ICE.String()
-		}
-		resp.Powertrain = pt
-		resp.DefinitionId = tblDef.ID
-
-		metrics.Success.With(prometheus.Labels{"method": DeviceDefinitionOverride}).Inc()
-
-		return resp, nil
-	}
+	localLog.Info().Msgf("Start Decode VIN ")
 
 	_, err = models.DeviceTypes(models.DeviceTypeWhere.ID.EQ(common.DefaultDeviceType)).One(ctx, dc.dbs().Reader)
 	if err != nil {

internal/core/queries/decode_vin.go

@@ -5,10 +5,11 @@ import (
 	"context"
 	"database/sql"
 	"fmt"
+	"strings"
+
 	coremodels "github.com/DIMO-Network/device-definitions-api/internal/core/models"
 	"github.com/DIMO-Network/device-definitions-api/internal/core/services"
 	"github.com/DIMO-Network/shared/pkg/logfields"
-	"strings"
 
 	"github.com/DIMO-Network/device-definitions-api/internal/core/mediator"
 	"github.com/DIMO-Network/device-definitions-api/internal/infrastructure/db/models"

internal/core/queries/get_vin_profile.go

@@ -79,7 +79,7 @@ func (c vinDecodingService) GetVIN(ctx context.Context, vin string, provider cor
 	if len(providersToTry) == 0 {
 		if provider == coremodels.AllProviders {
 			// fill in the list, future could do something country specific
-			providersToTry = append(providersToTry, coremodels.DrivlyProvider, coremodels.AutoIsoProvider, coremodels.VincarioProvider, coremodels.DATGroupProvider)
+			providersToTry = append(providersToTry, coremodels.DrivlyProvider, coremodels.VincarioProvider, coremodels.Japan17VIN, coremodels.AutoIsoProvider, coremodels.DATGroupProvider)
 		} else {
 			// use the specified override
 			providersToTry = append(providersToTry, provider)
@@ -89,7 +89,6 @@ func (c vinDecodingService) GetVIN(ctx context.Context, vin string, provider cor
 
 	for _, p := range providersToTry {
 		// try all the options, but need to continue if get an error
-		// todo: break if decode works, continue if need to try with next
 		switch p {
 		case coremodels.TeslaProvider:
 			v := vinutil.VIN(vin)
@@ -107,33 +106,41 @@ func (c vinDecodingService) GetVIN(ctx context.Context, vin string, provider cor
 				FuelType: "electric",
 				MetaData: null.JSONFrom(bytes),
 			}
+			err := validateVinDecoding(result)
+			if err != nil {
+				errFinal = err
+				continue
+			}
 			localLog.Info().Msgf("decoded with tesla: %+v", result)
 			return result, nil
 		case coremodels.DrivlyProvider:
+			localLog.Info().Msgf("trying to decode VIN: %s with drivly", vin)
 			vinDrivlyInfo, err := c.drivlyAPISvc.GetVINInfo(vin)
 			if err != nil {
 				errFinal = errors.Wrapf(err, "unable to decode vin: %s with drivly", vin)
 				continue
 			}
-			result, err = buildFromDrivly(vinDrivlyInfo)
+			result, err = buildFromDrivly(vinDrivlyInfo) // already does validation
 			if err != nil {
 				errFinal = errors.Wrapf(err, "unable to decode vin: %s with drivly", vin)
 				continue
 			}
 			return result, nil
 		case coremodels.VincarioProvider:
+			localLog.Info().Msgf("trying to decode VIN: %s with vincario", vin)
 			vinVincarioInfo, err := c.vincarioAPISvc.DecodeVIN(vin)
 			if err != nil {
 				errFinal = errors.Wrapf(err, "unable to decode vin: %s with vincario", vin)
 				continue
 			}
-			result, err = buildFromVincario(vinVincarioInfo)
+			result, err = buildFromVincario(vinVincarioInfo) // already does validation
 			if err != nil {
 				errFinal = err
 				continue
 			}
 			return result, nil
 		case coremodels.Japan17VIN:
+			localLog.Info().Msgf("trying to decode VIN: %s with 17vin", vin)
 			mmy, raw, err := c.japan17VINAPI.GetVINInfo(vin)
 			if err != nil {
 				errFinal = errors.Wrapf(err, "unable to decode vin: %s with japan17vin", vin)
@@ -155,6 +162,7 @@ func (c vinDecodingService) GetVIN(ctx context.Context, vin string, provider cor
 			}
 			return result, nil
 		case coremodels.CarVXVIN:
+			localLog.Info().Msgf("trying to decode VIN: %s with carvx", vin)
 			info, raw, err := c.carvxAPI.GetVINInfo(vin)
 			if err != nil {
 				errFinal = errors.Wrapf(err, "unable to decode vin: %s with carvx", vin)
@@ -180,87 +188,35 @@ func (c vinDecodingService) GetVIN(ctx context.Context, vin string, provider cor
 			}
 			return result, nil
 		case coremodels.AutoIsoProvider:
+			localLog.Info().Msgf("trying to decode VIN: %s with autoiso", vin)
 			vinAutoIsoInfo, err := c.autoIsoAPIService.GetVIN(vin)
 			if err != nil {
 				errFinal = errors.Wrapf(err, "unable to decode vin: %s with autoiso", vin)
 				continue
 			}
-			result, err = buildFromAutoIso(vinAutoIsoInfo)
+			result, err = buildFromAutoIso(vinAutoIsoInfo) // already does validation
 			if err != nil {
 				errFinal = err
 				continue
 			}
 			return result, nil
 		case coremodels.DATGroupProvider:
+			localLog.Info().Msgf("trying to decode VIN: %s with datgroup", vin)
 			// todo lookup country for two letter equiv
 			vinInfo, err := c.DATGroupAPIService.GetVINv2(vin, country) // try with Turkey
 			if err != nil {
 				errFinal = errors.Wrapf(err, "unable to decode vin: %s with DATGroup", vin)
 				continue
 			}
-			result, err = buildFromDATGroup(vinInfo)
+			result, err = buildFromDATGroup(vinInfo) // already does validation
 			if err != nil {
 				errFinal = err
 				continue
 			}
 			return result, nil
 		case coremodels.AllProviders:
-			//vinDrivlyInfo, err := c.drivlyAPISvc.GetVINInfo(vin)
-			//if err != nil {
-			//	localLog.Warn().Err(err).Msg("AllProviders decode - unable decode vin with drivly")
-			//} else {
-			//	result, err = buildFromDrivly(vinDrivlyInfo)
-			//	if err != nil {
-			//		localLog.Warn().Err(err).Msg("AllProviders decode -could not decode vin with drivly")
-			//	} else {
-			//		metadata, err := common.BuildDeviceTypeAttributes(buildDrivlyVINInfoToUpdateAttr(vinDrivlyInfo), dt)
-			//		if err != nil {
-			//			localLog.Warn().Err(err).Msg("AllProviders decode - unable to build metadata attributes")
-			//		}
-			//		result.MetaData = metadata
-			//	}
-			//}
-			//
-			//// if nothing from drivly, try autoiso
-			//if result == nil || result.Source == "" {
-			//	autoIsoInfo, err := c.autoIsoAPIService.GetVIN(vin)
-			//	if err != nil {
-			//		localLog.Warn().Err(err).Msg("AllProviders decode -could not decode vin with autoiso")
-			//	} else {
-			//		result, err = buildFromAutoIso(autoIsoInfo)
-			//		if err != nil {
-			//			localLog.Warn().Err(err).Msg("AllProviders decode -could not build struct from autoiso data")
-			//		}
-			//	}
-			//}
-			//
-			//// if nothing,try vincario
-			//if result == nil || result.Source == "" {
-			//	vinVincarioInfo, err := c.vincarioAPISvc.DecodeVIN(vin)
-			//	if err != nil {
-			//		localLog.Warn().Err(err).Msg("AllProviders decode -could not decode vin with vincario")
-			//	} else {
-			//		result, err = buildFromVincario(vinVincarioInfo)
-			//		if err != nil {
-			//			localLog.Warn().Err(err).Msg("AllProviders decode -could not build struct from vincario data")
-			//		}
-			//	}
-			//}
-			//
-			//// if nothing from vincario, try DATGroup
-			//if result == nil || result.Source == "" {
-			//	// idea: only accept WMI's for DATgroup that they have succesfully decoded in the past
-			//	datGroupInfo, err := c.DATGroupAPIService.GetVINv2(vin, country)
-			//	if err != nil {
-			//		localLog.Warn().Err(err).Msg("AllProviders decode -could not decode vin with DATGroup")
-			//	} else {
-			//		result, err = buildFromDATGroup(datGroupInfo)
-			//		localLog.Info().Msgf("datgroup result: %+v", result) // temporary for debugging
-			//		if err != nil {
-			//			localLog.Warn().Err(err).Msg("AllProviders decode - could not build struct from DATGroup data")
-			//		}
-			//	}
-			//}
+			// this should never hit
+			errFinal = fmt.Errorf("all providers - invalid option reached")
 		}
 	}