Fix candidate mileage review findings
This commit is contained in:
@@ -50,3 +50,13 @@ Result:
|
|||||||
- `SamplesFromEnvelope` now carries `EventTime` and `DeviceID`, which the new candidate mapping needs.
|
- `SamplesFromEnvelope` now carries `EventTime` and `DeviceID`, which the new candidate mapping needs.
|
||||||
- The upsert SQL is focused on the candidate table and reuses the shared `Execer` interface.
|
- The upsert SQL is focused on the candidate table and reuses the shared `Execer` interface.
|
||||||
- No unrelated stats files were modified.
|
- No unrelated stats files were modified.
|
||||||
|
|
||||||
|
## Review Fix Addendum
|
||||||
|
- Preserved manual `platform_name` values when the candidate upsert receives blank runtime input by switching to `COALESCE(NULLIF(TRIM(VALUES(platform_name)), ''), platform_name)`.
|
||||||
|
- Bootstrapped `vehicle_data_source`, `vehicle_daily_mileage_source`, and `vehicle_daily_mileage` before running alter statements in `Writer.EnsureSchema`.
|
||||||
|
- Added a defensive blank `SourceIP` guard in `UpsertSourceMileage` so malformed candidate rows are skipped instead of written.
|
||||||
|
- Removed the SQL comment that existed only to satisfy a string-match test and updated the focused assertions to check the real `daily_mileage_km` and `platform_name` SQL expressions.
|
||||||
|
- Verification run:
|
||||||
|
- `go test ./internal/stats -run 'TestUpsertSourceMileage|TestWriterEnsuresSchema' -count=1`
|
||||||
|
- `go test ./internal/stats -count=1`
|
||||||
|
- Both passed.
|
||||||
|
|||||||
@@ -47,9 +47,15 @@ func NewWriter(exec Execer, loc *time.Location) *Writer {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (w *Writer) EnsureSchema(ctx context.Context) error {
|
func (w *Writer) EnsureSchema(ctx context.Context) error {
|
||||||
if _, err := w.exec.ExecContext(ctx, DailyMileageTableSQL); err != nil {
|
for _, statement := range []string{
|
||||||
|
DataSourceTableSQL,
|
||||||
|
DailyMileageSourceTableSQL,
|
||||||
|
DailyMileageTableSQL,
|
||||||
|
} {
|
||||||
|
if _, err := w.exec.ExecContext(ctx, statement); err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
}
|
||||||
for _, statement := range DailyMileageAlterSQL {
|
for _, statement := range DailyMileageAlterSQL {
|
||||||
if _, err := w.exec.ExecContext(ctx, statement); err != nil && !isDuplicateColumnError(err) {
|
if _, err := w.exec.ExecContext(ctx, statement); err != nil && !isDuplicateColumnError(err) {
|
||||||
return err
|
return err
|
||||||
|
|||||||
@@ -173,24 +173,30 @@ func TestWriterEnsuresSchemaAndUpsertsDailyMileage(t *testing.T) {
|
|||||||
t.Fatalf("Append() error = %v", err)
|
t.Fatalf("Append() error = %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
if !strings.Contains(exec.calls[0].query, "CREATE TABLE IF NOT EXISTS vehicle_daily_mileage") {
|
if len(exec.calls) != 7 {
|
||||||
t.Fatalf("unexpected schema sql: %s", exec.calls[0].query)
|
|
||||||
}
|
|
||||||
for _, column := range []string{"vehicle_key", "id BIGINT", "AUTO_INCREMENT", "created_at"} {
|
|
||||||
if strings.Contains(exec.calls[0].query, column) {
|
|
||||||
t.Fatalf("schema should not include %s: %s", column, exec.calls[0].query)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if !strings.Contains(exec.calls[0].query, "PRIMARY KEY (vin, stat_date, protocol)") {
|
|
||||||
t.Fatalf("daily mileage table should key by vin/stat_date/protocol: %s", exec.calls[0].query)
|
|
||||||
}
|
|
||||||
if strings.Contains(exec.calls[0].query, "KEY idx_vin (vin)") {
|
|
||||||
t.Fatalf("daily mileage table should not keep redundant vin index covered by the primary key: %s", exec.calls[0].query)
|
|
||||||
}
|
|
||||||
if len(exec.calls) != 5 {
|
|
||||||
t.Fatalf("exec calls = %d", len(exec.calls))
|
t.Fatalf("exec calls = %d", len(exec.calls))
|
||||||
}
|
}
|
||||||
upsertCall := exec.calls[4]
|
for i, want := range []string{
|
||||||
|
"CREATE TABLE IF NOT EXISTS vehicle_data_source",
|
||||||
|
"CREATE TABLE IF NOT EXISTS vehicle_daily_mileage_source",
|
||||||
|
"CREATE TABLE IF NOT EXISTS vehicle_daily_mileage",
|
||||||
|
} {
|
||||||
|
if !strings.Contains(exec.calls[i].query, want) {
|
||||||
|
t.Fatalf("schema call %d = %s, want %s", i, exec.calls[i].query, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
for _, column := range []string{"vehicle_key", "id BIGINT", "AUTO_INCREMENT", "created_at"} {
|
||||||
|
if strings.Contains(exec.calls[2].query, column) {
|
||||||
|
t.Fatalf("schema should not include %s: %s", column, exec.calls[2].query)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !strings.Contains(exec.calls[2].query, "PRIMARY KEY (vin, stat_date, protocol)") {
|
||||||
|
t.Fatalf("daily mileage table should key by vin/stat_date/protocol: %s", exec.calls[2].query)
|
||||||
|
}
|
||||||
|
if strings.Contains(exec.calls[2].query, "KEY idx_vin (vin)") {
|
||||||
|
t.Fatalf("daily mileage table should not keep redundant vin index covered by the primary key: %s", exec.calls[2].query)
|
||||||
|
}
|
||||||
|
upsertCall := exec.calls[6]
|
||||||
if !strings.Contains(upsertCall.query, "ON DUPLICATE KEY UPDATE") {
|
if !strings.Contains(upsertCall.query, "ON DUPLICATE KEY UPDATE") {
|
||||||
t.Fatalf("unexpected upsert sql: %s", upsertCall.query)
|
t.Fatalf("unexpected upsert sql: %s", upsertCall.query)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -71,7 +71,7 @@ func UpsertSourceMileage(ctx context.Context, exec Execer, sample SourceMileageS
|
|||||||
if exec == nil {
|
if exec == nil {
|
||||||
panic("stats execer must not be nil")
|
panic("stats execer must not be nil")
|
||||||
}
|
}
|
||||||
if sample.VIN == "" || sample.SourceKey == "" {
|
if sample.VIN == "" || sample.SourceKey == "" || strings.TrimSpace(sample.SourceIP) == "" {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
if sample.QualityStatus == "" {
|
if sample.QualityStatus == "" {
|
||||||
@@ -110,7 +110,7 @@ ON DUPLICATE KEY UPDATE
|
|||||||
source_endpoint = VALUES(source_endpoint),
|
source_endpoint = VALUES(source_endpoint),
|
||||||
phone = VALUES(phone),
|
phone = VALUES(phone),
|
||||||
device_id = VALUES(device_id),
|
device_id = VALUES(device_id),
|
||||||
platform_name = COALESCE(VALUES(platform_name), platform_name),
|
platform_name = COALESCE(NULLIF(TRIM(VALUES(platform_name)), ''), platform_name),
|
||||||
first_total_mileage_km = CASE
|
first_total_mileage_km = CASE
|
||||||
WHEN first_total_mileage_km IS NULL OR first_total_mileage_km <= 0
|
WHEN first_total_mileage_km IS NULL OR first_total_mileage_km <= 0
|
||||||
THEN VALUES(first_total_mileage_km)
|
THEN VALUES(first_total_mileage_km)
|
||||||
@@ -121,7 +121,6 @@ ON DUPLICATE KEY UPDATE
|
|||||||
THEN VALUES(latest_total_mileage_km)
|
THEN VALUES(latest_total_mileage_km)
|
||||||
ELSE GREATEST(latest_total_mileage_km, VALUES(latest_total_mileage_km))
|
ELSE GREATEST(latest_total_mileage_km, VALUES(latest_total_mileage_km))
|
||||||
END,
|
END,
|
||||||
/* daily_mileage_km = VALUES(daily_mileage_km) */
|
|
||||||
daily_mileage_km = GREATEST(
|
daily_mileage_km = GREATEST(
|
||||||
CASE
|
CASE
|
||||||
WHEN latest_total_mileage_km IS NULL OR latest_total_mileage_km <= 0
|
WHEN latest_total_mileage_km IS NULL OR latest_total_mileage_km <= 0
|
||||||
|
|||||||
@@ -50,8 +50,9 @@ func TestUpsertSourceMileageWritesCandidateRow(t *testing.T) {
|
|||||||
for _, want := range []string{
|
for _, want := range []string{
|
||||||
"INSERT INTO vehicle_daily_mileage_source",
|
"INSERT INTO vehicle_daily_mileage_source",
|
||||||
"ON DUPLICATE KEY UPDATE",
|
"ON DUPLICATE KEY UPDATE",
|
||||||
"daily_mileage_km = VALUES(daily_mileage_km)",
|
"daily_mileage_km = GREATEST(",
|
||||||
"quality_status = VALUES(quality_status)",
|
"quality_status = VALUES(quality_status)",
|
||||||
|
"platform_name = COALESCE(NULLIF(TRIM(VALUES(platform_name)), ''), platform_name)",
|
||||||
} {
|
} {
|
||||||
if !strings.Contains(sql, want) {
|
if !strings.Contains(sql, want) {
|
||||||
t.Fatalf("candidate upsert missing %q: %s", want, sql)
|
t.Fatalf("candidate upsert missing %q: %s", want, sql)
|
||||||
@@ -61,3 +62,22 @@ func TestUpsertSourceMileageWritesCandidateRow(t *testing.T) {
|
|||||||
t.Fatalf("first arg = %#v", got)
|
t.Fatalf("first arg = %#v", got)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestUpsertSourceMileageSkipsBlankSourceIP(t *testing.T) {
|
||||||
|
exec := &recordingExec{}
|
||||||
|
sample := SourceMileageSample{
|
||||||
|
VIN: "LA9GG64L7PBAF4001",
|
||||||
|
StatDate: "2026-07-08",
|
||||||
|
Protocol: envelope.ProtocolJT808,
|
||||||
|
SourceKey: "JT808:13307765812@",
|
||||||
|
SourceIP: " ",
|
||||||
|
Phone: "13307765812",
|
||||||
|
QualityStatus: QualityOK,
|
||||||
|
}
|
||||||
|
if err := UpsertSourceMileage(context.Background(), exec, sample); err != nil {
|
||||||
|
t.Fatalf("UpsertSourceMileage() error = %v", err)
|
||||||
|
}
|
||||||
|
if len(exec.calls) != 0 {
|
||||||
|
t.Fatalf("exec calls = %d, want 0", len(exec.calls))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user