From a778742b51fa834bca6efc8abd79a8992ccfbc51 Mon Sep 17 00:00:00 2001 From: madhavilosetty-intel Date: Mon, 1 Jun 2026 10:43:21 -0700 Subject: [PATCH] feat(api): add device identity and lifecycle columns Add six additive columns to the devices table across all three backends (SQLite, Postgres, MongoDB) for issue #843: - id: app-generated UUID surrogate key; stable and immutable - createddate: server-set insert timestamp (RFC3339Nano); immutable - isdeleted: logical-deletion flag (column + plumbing only) - deleteddate: server-set soft-delete timestamp; read-only (column + plumbing only) - producttype: manageability SKU (vPro/ISM) - connectiontype: device connection type (CIRA/Direct) id, createddate, and deleteddate are server-managed and immutable. A partial unique index on id enforces dedup. DTO, OpenAPI, Postman, and tests are updated. Soft-delete behavior (DELETE handling, read filtering) lands in a separate PR. Resolves: #843 --- .../console_mps_apis.postman_collection.json | 5 + ...526000000_migrate_device_identity.down.sql | 12 ++ ...60526000000_migrate_device_identity.up.sql | 22 +++ .../controller/httpapi/v1/devices_test.go | 3 + internal/entity/device.go | 7 +- internal/entity/dto/v1/device.go | 6 + internal/entity/dto/v1/device_test.go | 65 +++++++ internal/usecase/devices/repo.go | 8 + internal/usecase/devices/repo_test.go | 34 +++- internal/usecase/devices/usecase.go | 17 ++ internal/usecase/nosqldb/mongo/client.go | 19 +- internal/usecase/nosqldb/mongo/device.go | 10 +- internal/usecase/nosqldb/mongo/device_test.go | 98 +++++++++++ internal/usecase/nosqldb/mongo/fields.go | 1 + internal/usecase/nosqldb/mongo/mongo_test.go | 66 +++++++ internal/usecase/sqldb/device.go | 51 ++++-- internal/usecase/sqldb/device_test.go | 165 ++++++++++++++++-- 17 files changed, 557 insertions(+), 32 deletions(-) create mode 100644 internal/app/migrations/20260526000000_migrate_device_identity.down.sql create mode 100644 internal/app/migrations/20260526000000_migrate_device_identity.up.sql create mode 100644 internal/entity/dto/v1/device_test.go diff --git a/integration-test/collections/console_mps_apis.postman_collection.json b/integration-test/collections/console_mps_apis.postman_collection.json index e7b29ebfc..1e22b7500 100644 --- a/integration-test/collections/console_mps_apis.postman_collection.json +++ b/integration-test/collections/console_mps_apis.postman_collection.json @@ -1180,6 +1180,11 @@ "pm.test(\"Status code is 200\", function () {\r", " pm.response.to.have.status(200);\r", "});\r", + "pm.test(\"Response includes server-managed identity fields\", function () {\r", + " var jsonData = pm.response.json();\r", + " pm.expect(jsonData.id).to.be.a(\"string\").and.not.empty;\r", + " pm.expect(jsonData.createdDate).to.be.a(\"string\").and.not.empty;\r", + "});\r", "" ], "type": "text/javascript" diff --git a/internal/app/migrations/20260526000000_migrate_device_identity.down.sql b/internal/app/migrations/20260526000000_migrate_device_identity.down.sql new file mode 100644 index 000000000..35114aa18 --- /dev/null +++ b/internal/app/migrations/20260526000000_migrate_device_identity.down.sql @@ -0,0 +1,12 @@ +/********************************************************************* +* Copyright (c) Intel Corporation 2026 +* SPDX-License-Identifier: Apache-2.0 +**********************************************************************/ + +DROP INDEX IF EXISTS idx_devices_id; +ALTER TABLE devices DROP COLUMN connectiontype; +ALTER TABLE devices DROP COLUMN producttype; +ALTER TABLE devices DROP COLUMN deleteddate; +ALTER TABLE devices DROP COLUMN isdeleted; +ALTER TABLE devices DROP COLUMN createddate; +ALTER TABLE devices DROP COLUMN id; diff --git a/internal/app/migrations/20260526000000_migrate_device_identity.up.sql b/internal/app/migrations/20260526000000_migrate_device_identity.up.sql new file mode 100644 index 000000000..b54ad147c --- /dev/null +++ b/internal/app/migrations/20260526000000_migrate_device_identity.up.sql @@ -0,0 +1,22 @@ +/********************************************************************* +* Copyright (c) Intel Corporation 2026 +* SPDX-License-Identifier: Apache-2.0 +**********************************************************************/ + +-- Device identity & lifecycle columns (issue #843). +-- All TEXT columns are NOT NULL DEFAULT '' so the ALTER backfills existing +-- rows with a non-NULL value (the modernc sqlite driver cannot scan NULL into +-- a Go string). `id` is an app-generated surrogate key; the partial unique +-- index excludes the backfilled empty values on pre-existing rows. +-- createddate: server-set insert timestamp. isdeleted/deleteddate: logical- +-- deletion flag + server-set timestamp (column + plumbing only; soft-delete +-- behavior lands in a separate PR). producttype: manageability SKU (vPro/ISM). +-- connectiontype: CIRA/Direct. +ALTER TABLE devices ADD COLUMN id TEXT NOT NULL DEFAULT ''; +ALTER TABLE devices ADD COLUMN createddate TEXT NOT NULL DEFAULT ''; +ALTER TABLE devices ADD COLUMN isdeleted BOOLEAN NOT NULL DEFAULT FALSE; +ALTER TABLE devices ADD COLUMN deleteddate TEXT NOT NULL DEFAULT ''; +ALTER TABLE devices ADD COLUMN producttype TEXT NOT NULL DEFAULT ''; +ALTER TABLE devices ADD COLUMN connectiontype TEXT NOT NULL DEFAULT ''; + +CREATE UNIQUE INDEX IF NOT EXISTS idx_devices_id ON devices (id) WHERE id <> ''; diff --git a/internal/controller/httpapi/v1/devices_test.go b/internal/controller/httpapi/v1/devices_test.go index 0d20315d3..68a3e76db 100644 --- a/internal/controller/httpapi/v1/devices_test.go +++ b/internal/controller/httpapi/v1/devices_test.go @@ -91,6 +91,9 @@ var ( "usetls": true, "allowselfsigned": true, "certhash": true, + // isDeleted has no omitempty, so it is always present in a marshaled + // device body and thus always reported as a provided PATCH field. + "isdeleted": true, } ) diff --git a/internal/entity/device.go b/internal/entity/device.go index 6baa9cea2..485cb9804 100644 --- a/internal/entity/device.go +++ b/internal/entity/device.go @@ -9,6 +9,12 @@ type Device struct { MPSInstance string `bson:"mpsinstance"` Hostname string `bson:"hostname"` GUID string `bson:"guid"` + ID string `bson:"id"` // app-generated surrogate key; stable, immutable + CreatedDate string `bson:"createddate"` // server-set insert timestamp (RFC3339Nano UTC); immutable + IsDeleted bool `bson:"isdeleted"` // logical-deletion flag + DeletedDate string `bson:"deleteddate"` // server-set on soft-delete (RFC3339Nano UTC); read-only + ProductType string `bson:"producttype"` // manageability SKU (vPro/ISM) + ConnectionType string `bson:"connectiontype"` // device connection type (CIRA/Direct) MPSUsername string `bson:"mpsusername"` Tags string `bson:"tags"` TenantID string `bson:"tenantid"` @@ -26,7 +32,6 @@ type Device struct { AllowSelfSigned bool `bson:"allowselfsigned"` CertHash *string `bson:"certhash"` } - type Explorer struct { XMLInput string XMLOutput string diff --git a/internal/entity/dto/v1/device.go b/internal/entity/dto/v1/device.go index 538d9bcc6..0f76a2d98 100644 --- a/internal/entity/dto/v1/device.go +++ b/internal/entity/dto/v1/device.go @@ -34,6 +34,12 @@ type Device struct { UseTLS bool `json:"useTLS"` AllowSelfSigned bool `json:"allowSelfSigned"` CertHash string `json:"certHash"` + ID string `json:"id,omitempty"` // server-managed surrogate key; read-only + CreatedDate string `json:"createdDate,omitempty"` // server-set on insert; read-only + IsDeleted bool `json:"isDeleted"` // no omitempty: emit false to distinguish from absent + DeletedDate string `json:"deletedDate,omitempty"` // server-set on soft-delete; read-only + ProductType string `json:"productType,omitempty"` // manageability SKU (vPro/ISM) + ConnectionType string `json:"connectionType,omitempty"` // device connection type (CIRA/Direct) } type DeviceInfo struct { diff --git a/internal/entity/dto/v1/device_test.go b/internal/entity/dto/v1/device_test.go new file mode 100644 index 000000000..e8747f119 --- /dev/null +++ b/internal/entity/dto/v1/device_test.go @@ -0,0 +1,65 @@ +package dto + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/require" +) + +// TestDevice_JSONContract locks two intentional serialization decisions for the +// identity/lifecycle columns on the /api/v1 device shape: +// - isDeleted has NO omitempty, so it is always emitted and callers can +// distinguish a false value from an absent field. +// - the other new fields ARE omitempty, so they stay absent on empty/legacy +// payloads and don't change the wire shape existing v1 consumers expect. +func TestDevice_JSONContract(t *testing.T) { + t.Parallel() + + t.Run("zero value emits isDeleted but omits optional identity fields", func(t *testing.T) { + t.Parallel() + + out := deviceJSONFields(t, Device{}) + + require.Contains(t, out, "isDeleted", "isDeleted must always be present") + require.JSONEq(t, `false`, string(out["isDeleted"])) + + for _, k := range []string{"id", "createdDate", "deletedDate", "productType", "connectionType"} { + require.NotContains(t, out, k, "%s must be omitempty on an empty device", k) + } + }) + + t.Run("populated values serialize under the expected keys", func(t *testing.T) { + t.Parallel() + + out := deviceJSONFields(t, Device{ + ID: "id-1", + CreatedDate: "2026-05-26T12:00:00Z", + IsDeleted: true, + DeletedDate: "2026-05-27T08:00:00Z", + ProductType: "vpro", + ConnectionType: "CIRA", + }) + + require.JSONEq(t, `"id-1"`, string(out["id"])) + require.JSONEq(t, `"2026-05-26T12:00:00Z"`, string(out["createdDate"])) + require.JSONEq(t, `true`, string(out["isDeleted"])) + require.JSONEq(t, `"2026-05-27T08:00:00Z"`, string(out["deletedDate"])) + require.JSONEq(t, `"vpro"`, string(out["productType"])) + require.JSONEq(t, `"CIRA"`, string(out["connectionType"])) + }) +} + +// deviceJSONFields marshals d and returns its top-level JSON object keyed by +// field name, so a test can assert on key presence/absence and values. +func deviceJSONFields(t *testing.T, d Device) map[string]json.RawMessage { + t.Helper() + + b, err := json.Marshal(d) + require.NoError(t, err) + + var m map[string]json.RawMessage + require.NoError(t, json.Unmarshal(b, &m)) + + return m +} diff --git a/internal/usecase/devices/repo.go b/internal/usecase/devices/repo.go index 28e067d52..4bca2b026 100644 --- a/internal/usecase/devices/repo.go +++ b/internal/usecase/devices/repo.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "strings" + "time" "github.com/google/uuid" @@ -251,6 +252,13 @@ func (uc *UseCase) Insert(ctx context.Context, d *dto.Device) (*dto.Device, erro d1.GUID = uuid.New().String() } + // id and createddate are server-managed: a stable surrogate key and + // the insertion timestamp. Client-supplied values are ignored. Nanosecond + // precision (UTC) keeps the string lexicographically sortable and avoids + // collisions when many devices are added within the same second. + d1.ID = uuid.New().String() + d1.CreatedDate = time.Now().UTC().Format(time.RFC3339Nano) + _, err = uc.repo.Insert(ctx, d1) if err != nil { return nil, ErrDatabase.Wrap("Insert", "uc.repo.Insert", err) diff --git a/internal/usecase/devices/repo_test.go b/internal/usecase/devices/repo_test.go index aba5caeac..bafb824ef 100644 --- a/internal/usecase/devices/repo_test.go +++ b/internal/usecase/devices/repo_test.go @@ -2,6 +2,8 @@ package devices_test import ( "context" + "fmt" + "reflect" "testing" "github.com/stretchr/testify/require" @@ -18,6 +20,32 @@ func ptr(s string) *string { return &s } +// insertedDevice matches the entity passed to repo.Insert, ignoring the +// server-generated ID and CreatedDate (which are non-deterministic) while +// asserting they were populated. +type insertedDevice struct{ want *entity.Device } + +func (m insertedDevice) Matches(x any) bool { + got, ok := x.(*entity.Device) + if !ok || got == nil { + return false + } + + if got.ID == "" || got.CreatedDate == "" { + return false + } + + cp := *got + cp.ID = "" + cp.CreatedDate = "" + + return reflect.DeepEqual(&cp, m.want) +} + +func (m insertedDevice) String() string { + return fmt.Sprintf("matches %+v (ignoring server-set ID/CreatedDate)", m.want) +} + type testUsecase struct { name string guid string @@ -377,7 +405,7 @@ func TestInsert(t *testing.T) { } repo.EXPECT(). - Insert(context.Background(), device). + Insert(context.Background(), insertedDevice{want: device}). Return("unique-device-id", nil) repo.EXPECT(). GetByID(context.Background(), device.GUID, "tenant-id-456"). @@ -398,7 +426,7 @@ func TestInsert(t *testing.T) { } repo.EXPECT(). - Insert(context.Background(), device). + Insert(context.Background(), insertedDevice{want: device}). Return("", devices.ErrDatabase) }, res: (*dto.Device)(nil), @@ -506,7 +534,7 @@ func TestInsertWithPasswords(t *testing.T) { } repo.EXPECT(). - Insert(context.Background(), deviceWithPasswords). + Insert(context.Background(), insertedDevice{want: deviceWithPasswords}). Return("unique-device-id", nil) repo.EXPECT(). GetByID(context.Background(), "device-guid-123", "tenant-id-456"). diff --git a/internal/usecase/devices/usecase.go b/internal/usecase/devices/usecase.go index 50eaad554..d99e94a64 100644 --- a/internal/usecase/devices/usecase.go +++ b/internal/usecase/devices/usecase.go @@ -98,6 +98,13 @@ func (uc *UseCase) dtoToEntity(d *dto.Device) (*entity.Device, error) { Password: d.Password, UseTLS: d.UseTLS, AllowSelfSigned: d.AllowSelfSigned, + // ID and CreatedDate are server-managed and deliberately NOT copied + // from the inbound DTO: UseCase.Insert sets them after this conversion, + // and the repo Update layer never writes them. Leaving them zero here + // makes immutability structural rather than reliant on every call site. + IsDeleted: d.IsDeleted, + ProductType: d.ProductType, + ConnectionType: d.ConnectionType, } d1.Password, err = uc.safeRequirements.Encrypt(d1.Password) @@ -157,6 +164,10 @@ var deviceFieldSetters = map[string]func(dst, src *dto.Device){ "allowselfsigned": func(dst, src *dto.Device) { dst.AllowSelfSigned = src.AllowSelfSigned }, "certhash": func(dst, src *dto.Device) { dst.CertHash = src.CertHash }, "deviceinfo": func(dst, src *dto.Device) { dst.DeviceInfo = src.DeviceInfo }, + // id and createdDate are server-managed and intentionally not patchable. + "isdeleted": func(dst, src *dto.Device) { dst.IsDeleted = src.IsDeleted }, + "producttype": func(dst, src *dto.Device) { dst.ProductType = src.ProductType }, + "connectiontype": func(dst, src *dto.Device) { dst.ConnectionType = src.ConnectionType }, } func mergeDeviceFields(dst, src *dto.Device, fields map[string]bool) { @@ -197,6 +208,12 @@ func (uc *UseCase) entityToDTO(d *entity.Device) (*dto.Device, error) { Username: d.Username, UseTLS: d.UseTLS, AllowSelfSigned: d.AllowSelfSigned, + ID: d.ID, + CreatedDate: d.CreatedDate, + IsDeleted: d.IsDeleted, + DeletedDate: d.DeletedDate, + ProductType: d.ProductType, + ConnectionType: d.ConnectionType, } if d.CertHash != nil { diff --git a/internal/usecase/nosqldb/mongo/client.go b/internal/usecase/nosqldb/mongo/client.go index 968979981..a13834a33 100644 --- a/internal/usecase/nosqldb/mongo/client.go +++ b/internal/usecase/nosqldb/mongo/client.go @@ -112,7 +112,24 @@ func ensureIndexes(ctx context.Context, db *mongo.Database, log logger.Interface return fmt.Errorf("create case-insensitive unique index on %s: %w", CollectionDomains, err) } - log.Info("mongo unique indexes ensured (%d total)", len(tenantScoped)+1) + // Mirrors the SQL partial unique index idx_devices_id (WHERE id <> ''): the + // partial filter excludes legacy docs without an `id` and the empty default, + // so only populated surrogate keys are constrained to be unique. + if _, err := db.Collection(CollectionDevices).Indexes().CreateOne(ctx, mongo.IndexModel{ + Keys: bson.D{{Key: fieldID, Value: 1}}, + Options: options.Index(). + SetUnique(true). + SetPartialFilterExpression(bson.M{fieldID: bson.M{"$gt": ""}}). + SetName("idx_devices_id"), + }); err != nil { + return fmt.Errorf("create unique index on %s.id: %w", CollectionDevices, err) + } + + // globalIndexes: the two collection-global unique indexes created above + // (domains case-insensitive + devices id) that aren't in tenantScoped. + const globalIndexes = 2 + + log.Info("mongo unique indexes ensured (%d total)", len(tenantScoped)+globalIndexes) return nil } diff --git a/internal/usecase/nosqldb/mongo/device.go b/internal/usecase/nosqldb/mongo/device.go index 0636faceb..9f4ba9f14 100644 --- a/internal/usecase/nosqldb/mongo/device.go +++ b/internal/usecase/nosqldb/mongo/device.go @@ -198,7 +198,8 @@ func (r *DeviceRepo) Update(ctx context.Context, d *entity.Device) (bool, error) } // Explicit field list mirrors sqldb/device.go:Update so a new field must be wired in intentionally. - res, err := r.col.UpdateOne(ctx, + res, err := r.col.UpdateOne( + ctx, bson.M{fieldGUID: d.GUID, fieldTenantID: d.TenantID}, bson.M{opSet: bson.M{ fieldGUID: d.GUID, @@ -218,6 +219,10 @@ func (r *DeviceRepo) Update(ctx context.Context, d *entity.Device) (bool, error) "usetls": d.UseTLS, "allowselfsigned": d.AllowSelfSigned, "certhash": d.CertHash, + // id, createddate, and deleteddate are immutable — intentionally not $set here. + "isdeleted": d.IsDeleted, + "producttype": d.ProductType, + "connectiontype": d.ConnectionType, }}, ) if err != nil { @@ -254,7 +259,8 @@ func (r *DeviceRepo) UpdateLastSeen(ctx context.Context, guid string) error { return errDeviceDatabase.Wrap("UpdateLastSeen", "validate", nil) } - _, err := r.col.UpdateOne(ctx, + _, err := r.col.UpdateOne( + ctx, bson.M{fieldGUID: guid}, bson.M{opSet: bson.M{"lastseen": time.Now()}}, ) diff --git a/internal/usecase/nosqldb/mongo/device_test.go b/internal/usecase/nosqldb/mongo/device_test.go index 8fedd41ee..e0aea87ba 100644 --- a/internal/usecase/nosqldb/mongo/device_test.go +++ b/internal/usecase/nosqldb/mongo/device_test.go @@ -53,6 +53,104 @@ func TestDeviceRepo_GetByID_Found(t *testing.T) { require.Equal(t, "lab-host-1", got.FriendlyName) } +func TestDeviceRepo_GetByID_IdentityColumns(t *testing.T) { + t.Parallel() + + db, md := newMockedDB(t) + + md.AddResponses(findResponse( + "testdb."+mongo.CollectionDevices, + bson.D{ + {Key: "guid", Value: "g1"}, + {Key: "tenantid", Value: "t1"}, + {Key: "id", Value: "11111111-2222-3333-4444-555555555555"}, + {Key: "createddate", Value: "2026-05-26T12:00:00Z"}, + {Key: "isdeleted", Value: true}, + {Key: "deleteddate", Value: "2026-05-27T08:00:00Z"}, + {Key: "producttype", Value: "vpro"}, + {Key: "connectiontype", Value: "CIRA"}, + }, + )) + + repo := mongo.NewDeviceRepo(db) + + got, err := repo.GetByID(context.Background(), "g1", "t1") + require.NoError(t, err) + require.NotNil(t, got) + require.Equal(t, "11111111-2222-3333-4444-555555555555", got.ID) + require.Equal(t, "2026-05-26T12:00:00Z", got.CreatedDate) + require.True(t, got.IsDeleted) + require.Equal(t, "2026-05-27T08:00:00Z", got.DeletedDate) + require.Equal(t, "vpro", got.ProductType) + require.Equal(t, "CIRA", got.ConnectionType) +} + +// updateSetKeys extracts the key set of the $set document from a captured +// `update` command (shape: {update, updates:[{q, u:{$set:{...}}}]}). +func updateSetKeys(t *testing.T, cmd bson.Raw) map[string]struct{} { + t.Helper() + + updates, err := cmd.LookupErr("updates") + require.NoError(t, err) + + vals, err := updates.Array().Values() + require.NoError(t, err) + require.NotEmpty(t, vals) + + setVal, err := vals[0].Document().LookupErr("u", "$set") + require.NoError(t, err) + + elems, err := setVal.Document().Elements() + require.NoError(t, err) + + keys := make(map[string]struct{}, len(elems)) + for _, e := range elems { + keys[e.Key()] = struct{}{} + } + + return keys +} + +// TestDeviceRepo_Update_OmitsImmutableIdentityColumns mirrors the sqldb +// regression test: even when an Update carries changed id/createddate/deleteddate +// values, the Mongo $set must omit them so the immutable columns can never be mutated, +// while the mutable identity columns are still written. +func TestDeviceRepo_Update_OmitsImmutableIdentityColumns(t *testing.T) { + t.Parallel() + + db, md, cc := newMonitoredDB(t) + md.AddResponses(updateResponse(1)) + + repo := mongo.NewDeviceRepo(db) + + ok, err := repo.Update(context.Background(), &entity.Device{ + GUID: "g1", + TenantID: "t1", + ID: "tampered-id", + CreatedDate: "2099-01-01T00:00:00Z", + DeletedDate: "2099-01-01T00:00:00Z", + ProductType: "vpro", + ConnectionType: "CIRA", + }) + require.NoError(t, err) + require.True(t, ok) + + cmd, found := cc.byName("update") + require.True(t, found, "expected an update command to be sent") + + set := updateSetKeys(t, cmd) + + // Immutable: must never appear in $set. + require.NotContains(t, set, "id", "id must not be writable via Update") + require.NotContains(t, set, "createddate", "createddate must not be writable via Update") + require.NotContains(t, set, "deleteddate", "deleteddate must not be writable via Update") + + // Mutable identity columns: present so they round-trip on Update. + require.Contains(t, set, "isdeleted") + require.Contains(t, set, "producttype") + require.Contains(t, set, "connectiontype") +} + func TestDeviceRepo_GetByID_NotFound(t *testing.T) { t.Parallel() diff --git a/internal/usecase/nosqldb/mongo/fields.go b/internal/usecase/nosqldb/mongo/fields.go index 65b55fd85..beee345cd 100644 --- a/internal/usecase/nosqldb/mongo/fields.go +++ b/internal/usecase/nosqldb/mongo/fields.go @@ -6,6 +6,7 @@ package mongo const ( fieldTenantID = "tenantid" fieldGUID = "guid" + fieldID = "id" fieldProfileName = "profilename" fieldConfigName = "configname" fieldDomainSuffix = "domainsuffix" diff --git a/internal/usecase/nosqldb/mongo/mongo_test.go b/internal/usecase/nosqldb/mongo/mongo_test.go index 5011b4394..0c6a778a4 100644 --- a/internal/usecase/nosqldb/mongo/mongo_test.go +++ b/internal/usecase/nosqldb/mongo/mongo_test.go @@ -2,10 +2,12 @@ package mongo_test import ( "context" + "sync" "testing" "github.com/stretchr/testify/require" "go.mongodb.org/mongo-driver/v2/bson" + "go.mongodb.org/mongo-driver/v2/event" "go.mongodb.org/mongo-driver/v2/mongo" "go.mongodb.org/mongo-driver/v2/mongo/options" "go.mongodb.org/mongo-driver/v2/x/mongo/driver/drivertest" @@ -52,6 +54,70 @@ func newMockedDB(t *testing.T) (*mongo.Database, *drivertest.MockDeployment) { return client.Database("testdb"), md } +// capturedCommands records every command the driver sends, keyed by command +// name. The MockDeployment cannot replay state, so write-path invariants (e.g. +// that Update's $set omits immutable columns) are verified by inspecting the +// exact wire payload the production repo code emitted. +type capturedCommands struct { + mu sync.Mutex + cmds []capturedCommand +} + +type capturedCommand struct { + name string + raw bson.Raw +} + +func (c *capturedCommands) add(name string, raw bson.Raw) { + c.mu.Lock() + defer c.mu.Unlock() + + c.cmds = append(c.cmds, capturedCommand{name: name, raw: raw}) +} + +// byName returns the first captured command with the given name. +func (c *capturedCommands) byName(name string) (bson.Raw, bool) { + c.mu.Lock() + defer c.mu.Unlock() + + for _, cmd := range c.cmds { + if cmd.name == name { + return cmd.raw, true + } + } + + return nil, false +} + +// newMonitoredDB is like newMockedDB but also attaches a CommandMonitor that +// captures the wire payload of every command sent, so a test can assert on the +// exact document the repo emitted. +func newMonitoredDB(t *testing.T) (*mongo.Database, *drivertest.MockDeployment, *capturedCommands) { + t.Helper() + + cc := &capturedCommands{} + monitor := &event.CommandMonitor{ + Started: func(_ context.Context, e *event.CommandStartedEvent) { + // Copy: the driver may reuse the backing buffer after the callback. + cc.add(e.CommandName, bson.Raw(append([]byte(nil), e.Command...))) + }, + } + + md := drivertest.NewMockDeployment() + + opts := options.Client().SetMonitor(monitor) + require.NoError(t, xoptions.SetInternalClientOptions(opts, "deployment", md)) + + client, err := mongo.Connect(opts) + require.NoError(t, err) + + t.Cleanup(func() { + _ = client.Disconnect(context.Background()) + }) + + return client.Database("testdb"), md, cc +} + // findResponse builds the bson reply shape expected for a `find` command: // // { ok: 1, cursor: { id: 0, ns: "", firstBatch: [...] } } diff --git a/internal/usecase/sqldb/device.go b/internal/usecase/sqldb/device.go index bf0229d97..a9e70282c 100644 --- a/internal/usecase/sqldb/device.go +++ b/internal/usecase/sqldb/device.go @@ -88,7 +88,13 @@ func (r *DeviceRepo) Get(_ context.Context, top, skip int, tenantID string) ([]e "password", "usetls", "allowselfsigned", - "certhash"). + "certhash", + "id", + "createddate", + "isdeleted", + "deleteddate", + "producttype", + "connectiontype"). From("devices"). Where("tenantid = ?", tenantID). OrderBy("guid"). @@ -115,7 +121,7 @@ func (r *DeviceRepo) Get(_ context.Context, top, skip int, tenantID string) ([]e for rows.Next() { d := entity.Device{} - err = rows.Scan(&d.GUID, &d.Hostname, &d.Tags, &d.MPSInstance, &d.ConnectionStatus, &d.MPSUsername, &d.TenantID, &d.FriendlyName, &d.DNSSuffix, &d.DeviceInfo, &d.Username, &d.Password, &d.UseTLS, &d.AllowSelfSigned, &d.CertHash) + err = rows.Scan(&d.GUID, &d.Hostname, &d.Tags, &d.MPSInstance, &d.ConnectionStatus, &d.MPSUsername, &d.TenantID, &d.FriendlyName, &d.DNSSuffix, &d.DeviceInfo, &d.Username, &d.Password, &d.UseTLS, &d.AllowSelfSigned, &d.CertHash, &d.ID, &d.CreatedDate, &d.IsDeleted, &d.DeletedDate, &d.ProductType, &d.ConnectionType) if err != nil { return nil, ErrDeviceDatabase.Wrap("Get", "rows.Scan: ", err) } @@ -146,7 +152,14 @@ func (r *DeviceRepo) GetByID(_ context.Context, guid, tenantID string) (*entity. "mebxpassword", "usetls", "allowselfsigned", - "certhash"). + "certhash", + "id", + "createddate", + "isdeleted", + "deleteddate", + "producttype", + "connectiontype", + ). From("devices"). Where("guid = ? and tenantid = ?"). ToSql() @@ -170,7 +183,7 @@ func (r *DeviceRepo) GetByID(_ context.Context, guid, tenantID string) (*entity. for rows.Next() { d := &entity.Device{} - err = rows.Scan(&d.GUID, &d.Hostname, &d.Tags, &d.MPSInstance, &d.ConnectionStatus, &d.MPSUsername, &d.TenantID, &d.FriendlyName, &d.DNSSuffix, &d.DeviceInfo, &d.Username, &d.Password, &d.MPSPassword, &d.MEBXPassword, &d.UseTLS, &d.AllowSelfSigned, &d.CertHash) + err = rows.Scan(&d.GUID, &d.Hostname, &d.Tags, &d.MPSInstance, &d.ConnectionStatus, &d.MPSUsername, &d.TenantID, &d.FriendlyName, &d.DNSSuffix, &d.DeviceInfo, &d.Username, &d.Password, &d.MPSPassword, &d.MEBXPassword, &d.UseTLS, &d.AllowSelfSigned, &d.CertHash, &d.ID, &d.CreatedDate, &d.IsDeleted, &d.DeletedDate, &d.ProductType, &d.ConnectionType) if err != nil { return d, ErrDeviceDatabase.Wrap("Get", "rows.Scan: ", err) } @@ -237,7 +250,13 @@ func (r *DeviceRepo) GetByTags(_ context.Context, tags []string, method string, "tenantid", "friendlyname", "dnssuffix", - "deviceinfo"). + "deviceinfo", + "id", + "createddate", + "isdeleted", + "deleteddate", + "producttype", + "connectiontype"). From("devices") var params []interface{} @@ -293,7 +312,7 @@ func (r *DeviceRepo) GetByTags(_ context.Context, tags []string, method string, for rows.Next() { var d entity.Device - if err := rows.Scan(&d.GUID, &d.Hostname, &d.Tags, &d.MPSInstance, &d.ConnectionStatus, &d.MPSUsername, &d.TenantID, &d.FriendlyName, &d.DNSSuffix, &d.DeviceInfo); err != nil { + if err := rows.Scan(&d.GUID, &d.Hostname, &d.Tags, &d.MPSInstance, &d.ConnectionStatus, &d.MPSUsername, &d.TenantID, &d.FriendlyName, &d.DNSSuffix, &d.DeviceInfo, &d.ID, &d.CreatedDate, &d.IsDeleted, &d.DeletedDate, &d.ProductType, &d.ConnectionType); err != nil { return nil, ErrDeviceDatabase.Wrap("GetByTags", "rows.Scan", err) } @@ -347,6 +366,11 @@ func (r *DeviceRepo) Update(_ context.Context, d *entity.Device) (bool, error) { Set("useTLS", d.UseTLS). Set("allowSelfSigned", d.AllowSelfSigned). Set("certhash", d.CertHash). + Set("isdeleted", d.IsDeleted). + Set("producttype", d.ProductType). + Set("connectiontype", d.ConnectionType). + // id, createddate, and deleteddate are immutable — intentionally not Set + // here (deleteddate is written only by the soft-delete op, a separate PR). Where("guid = ? AND tenantid = ?", d.GUID, d.TenantID). ToSql() if err != nil { @@ -419,8 +443,8 @@ func (r *DeviceRepo) UpdateLastSeen(_ context.Context, guid string) error { func (r *DeviceRepo) Insert(_ context.Context, d *entity.Device) (string, error) { insertBuilder := r.Builder. Insert("devices"). - Columns("guid", "hostname", "tags", "mpsinstance", "connectionstatus", "mpsusername", "tenantid", "friendlyname", "dnssuffix", "deviceinfo", "username", "password", "mpspassword", "mebxpassword", "usetls", "allowselfsigned", "certhash"). - Values(d.GUID, d.Hostname, d.Tags, d.MPSInstance, d.ConnectionStatus, d.MPSUsername, d.TenantID, d.FriendlyName, d.DNSSuffix, d.DeviceInfo, d.Username, d.Password, d.MPSPassword, d.MEBXPassword, d.UseTLS, d.AllowSelfSigned, d.CertHash) + Columns("guid", "hostname", "tags", "mpsinstance", "connectionstatus", "mpsusername", "tenantid", "friendlyname", "dnssuffix", "deviceinfo", "username", "password", "mpspassword", "mebxpassword", "usetls", "allowselfsigned", "certhash", "id", "createddate", "isdeleted", "deleteddate", "producttype", "connectiontype"). + Values(d.GUID, d.Hostname, d.Tags, d.MPSInstance, d.ConnectionStatus, d.MPSUsername, d.TenantID, d.FriendlyName, d.DNSSuffix, d.DeviceInfo, d.Username, d.Password, d.MPSPassword, d.MEBXPassword, d.UseTLS, d.AllowSelfSigned, d.CertHash, d.ID, d.CreatedDate, d.IsDeleted, d.DeletedDate, d.ProductType, d.ConnectionType) if !r.IsEmbedded { insertBuilder = insertBuilder.Suffix("RETURNING xmin::text") @@ -467,7 +491,14 @@ func (r *DeviceRepo) GetByColumn(_ context.Context, columnName, queryValue, tena "password", "usetls", "allowselfsigned", - "certhash"). + "certhash", + "id", + "createddate", + "isdeleted", + "deleteddate", + "producttype", + "connectiontype", + ). From("devices"). Where(columnName+" = ? AND tenantid = ?", queryValue, tenantID). ToSql() @@ -491,7 +522,7 @@ func (r *DeviceRepo) GetByColumn(_ context.Context, columnName, queryValue, tena for rows.Next() { d := entity.Device{} - err = rows.Scan(&d.GUID, &d.Hostname, &d.Tags, &d.MPSInstance, &d.ConnectionStatus, &d.MPSUsername, &d.TenantID, &d.FriendlyName, &d.DNSSuffix, &d.DeviceInfo, &d.Username, &d.Password, &d.UseTLS, &d.AllowSelfSigned, &d.CertHash) + err = rows.Scan(&d.GUID, &d.Hostname, &d.Tags, &d.MPSInstance, &d.ConnectionStatus, &d.MPSUsername, &d.TenantID, &d.FriendlyName, &d.DNSSuffix, &d.DeviceInfo, &d.Username, &d.Password, &d.UseTLS, &d.AllowSelfSigned, &d.CertHash, &d.ID, &d.CreatedDate, &d.IsDeleted, &d.DeletedDate, &d.ProductType, &d.ConnectionType) if err != nil { return nil, ErrDeviceDatabase.Wrap("Get", "rows.Scan: ", err) } diff --git a/internal/usecase/sqldb/device_test.go b/internal/usecase/sqldb/device_test.go index 67d7d806e..022457a13 100644 --- a/internal/usecase/sqldb/device_test.go +++ b/internal/usecase/sqldb/device_test.go @@ -30,7 +30,8 @@ func setupDeviceTable(t *testing.T) *sql.DB { dbConn, err := sql.Open("sqlite", ":memory:") require.NoError(t, err) - _, err = dbConn.ExecContext(context.Background(), ` + _, err = dbConn.ExecContext( + context.Background(), ` CREATE TABLE devices ( guid TEXT PRIMARY KEY, hostname TEXT NOT NULL DEFAULT '', @@ -51,9 +52,16 @@ func setupDeviceTable(t *testing.T) *sql.DB { certhash TEXT NOT NULL DEFAULT '', lastconnected TEXT, lastdisconnected TEXT, - lastseen TEXT + lastseen TEXT, + id TEXT NOT NULL DEFAULT '', + createddate TEXT NOT NULL DEFAULT '', + isdeleted BOOLEAN NOT NULL DEFAULT FALSE, + deleteddate TEXT NOT NULL DEFAULT '', + producttype TEXT NOT NULL DEFAULT '', + connectiontype TEXT NOT NULL DEFAULT '' ); - `) + `, + ) require.NoError(t, err) return dbConn @@ -366,6 +374,101 @@ func TestDeviceRepo_GetByID(t *testing.T) { } } +// TestDeviceRepo_IdentityColumnsRoundTrip verifies the issue #843 identity +// columns (id, createddate, isdeleted, deleteddate, producttype, connectiontype) +// persist on Insert and read back through GetByID. +func TestDeviceRepo_IdentityColumnsRoundTrip(t *testing.T) { + t.Parallel() + + dbConn := setupDeviceTable(t) + defer dbConn.Close() + + sqlConfig := &db.SQL{ + Builder: squirrel.StatementBuilder.PlaceholderFormat(squirrel.Question), + Pool: dbConn, + IsEmbedded: true, + } + + repo := sqldb.NewDeviceRepo(sqlConfig, mocks.NewMockLogger(nil)) + + certHash := "certhash" + want := &entity.Device{ + GUID: "guid-identity", + TenantID: "tenant1", + CertHash: &certHash, + ID: "11111111-2222-3333-4444-555555555555", + CreatedDate: "2026-05-26T12:00:00Z", + IsDeleted: true, + DeletedDate: "2026-05-27T08:00:00Z", + ProductType: "vpro", + ConnectionType: "CIRA", + } + + _, err := repo.Insert(context.Background(), want) + require.NoError(t, err) + + got, err := repo.GetByID(context.Background(), "guid-identity", "tenant1") + require.NoError(t, err) + require.NotNil(t, got) + require.Equal(t, want.ID, got.ID) + require.Equal(t, want.CreatedDate, got.CreatedDate) + require.Equal(t, want.IsDeleted, got.IsDeleted) + require.Equal(t, want.DeletedDate, got.DeletedDate) + require.Equal(t, want.ProductType, got.ProductType) + require.Equal(t, want.ConnectionType, got.ConnectionType) +} + +// TestDeviceRepo_IdentityColumnsImmutableOnUpdate guards the design invariant +// that id, createddate, and deleteddate cannot be mutated via Update — the SQL +// SET list deliberately omits them, so an Update carrying changed values is a +// no-op for those columns. +func TestDeviceRepo_IdentityColumnsImmutableOnUpdate(t *testing.T) { + t.Parallel() + + dbConn := setupDeviceTable(t) + defer dbConn.Close() + + sqlConfig := &db.SQL{ + Builder: squirrel.StatementBuilder.PlaceholderFormat(squirrel.Question), + Pool: dbConn, + IsEmbedded: true, + } + + repo := sqldb.NewDeviceRepo(sqlConfig, mocks.NewMockLogger(nil)) + + certHash := "certhash" + original := &entity.Device{ + GUID: "guid-immut", + TenantID: "tenant1", + CertHash: &certHash, + ID: "original-id", + CreatedDate: "2026-05-26T12:00:00Z", + DeletedDate: "2026-05-27T08:00:00Z", + } + + _, err := repo.Insert(context.Background(), original) + require.NoError(t, err) + + // Attempt to mutate the immutable fields via Update. + tampered := *original + tampered.ID = "tampered-id" + tampered.CreatedDate = "2099-01-01T00:00:00Z" + tampered.DeletedDate = "2099-01-01T00:00:00Z" + tampered.FriendlyName = "renamed" + + updated, err := repo.Update(context.Background(), &tampered) + require.NoError(t, err) + require.True(t, updated) + + got, err := repo.GetByID(context.Background(), "guid-immut", "tenant1") + require.NoError(t, err) + require.NotNil(t, got) + require.Equal(t, "original-id", got.ID, "id must not change on Update") + require.Equal(t, "2026-05-26T12:00:00Z", got.CreatedDate, "createddate must not change on Update") + require.Equal(t, "2026-05-27T08:00:00Z", got.DeletedDate, "deleteddate must not change on Update") + require.Equal(t, "renamed", got.FriendlyName, "mutable fields should still update") +} + func TestDeviceRepo_GetDistinctTags(t *testing.T) { t.Parallel() @@ -554,7 +657,8 @@ func TestDeviceRepo_GetByTags(t *testing.T) { defer dbConn.Close() - _, err = dbConn.ExecContext(context.Background(), ` + _, err = dbConn.ExecContext( + context.Background(), ` CREATE TABLE devices ( guid TEXT PRIMARY KEY, hostname TEXT NOT NULL DEFAULT '', @@ -565,9 +669,16 @@ func TestDeviceRepo_GetByTags(t *testing.T) { tenantid TEXT NOT NULL, friendlyname TEXT NOT NULL DEFAULT '', dnssuffix TEXT NOT NULL DEFAULT '', - deviceinfo TEXT NOT NULL DEFAULT '' + deviceinfo TEXT NOT NULL DEFAULT '', + id TEXT NOT NULL DEFAULT '', + createddate TEXT NOT NULL DEFAULT '', + isdeleted BOOLEAN NOT NULL DEFAULT FALSE, + deleteddate TEXT NOT NULL DEFAULT '', + producttype TEXT NOT NULL DEFAULT '', + connectiontype TEXT NOT NULL DEFAULT '' ); - `) + `, + ) require.NoError(t, err) tc.setup(dbConn) @@ -646,7 +757,8 @@ func TestDeviceRepo_Delete(t *testing.T) { defer dbConn.Close() - _, err = dbConn.ExecContext(context.Background(), ` + _, err = dbConn.ExecContext( + context.Background(), ` CREATE TABLE devices ( guid TEXT PRIMARY KEY, hostname TEXT NOT NULL DEFAULT '', @@ -664,9 +776,16 @@ func TestDeviceRepo_Delete(t *testing.T) { mebxpassword TEXT, usetls BOOLEAN NOT NULL DEFAULT FALSE, allowselfsigned BOOLEAN NOT NULL DEFAULT FALSE, - certhash TEXT NOT NULL DEFAULT '' + certhash TEXT NOT NULL DEFAULT '', + id TEXT NOT NULL DEFAULT '', + createddate TEXT NOT NULL DEFAULT '', + isdeleted BOOLEAN NOT NULL DEFAULT FALSE, + deleteddate TEXT NOT NULL DEFAULT '', + producttype TEXT NOT NULL DEFAULT '', + connectiontype TEXT NOT NULL DEFAULT '' ); - `) + `, + ) require.NoError(t, err) tc.setup(dbConn) @@ -797,7 +916,8 @@ func TestDeviceRepo_Update(t *testing.T) { defer dbConn.Close() - _, err = dbConn.ExecContext(context.Background(), ` + _, err = dbConn.ExecContext( + context.Background(), ` CREATE TABLE devices ( guid TEXT PRIMARY KEY, hostname TEXT NOT NULL DEFAULT '', @@ -815,9 +935,16 @@ func TestDeviceRepo_Update(t *testing.T) { mebxpassword TEXT, usetls BOOLEAN NOT NULL DEFAULT FALSE, allowselfsigned BOOLEAN NOT NULL DEFAULT FALSE, - certhash TEXT NOT NULL DEFAULT '' + certhash TEXT NOT NULL DEFAULT '', + id TEXT NOT NULL DEFAULT '', + createddate TEXT NOT NULL DEFAULT '', + isdeleted BOOLEAN NOT NULL DEFAULT FALSE, + deleteddate TEXT NOT NULL DEFAULT '', + producttype TEXT NOT NULL DEFAULT '', + connectiontype TEXT NOT NULL DEFAULT '' ); - `) + `, + ) require.NoError(t, err) tc.setup(dbConn) @@ -1054,7 +1181,8 @@ func TestDeviceRepo_GetByColumn(t *testing.T) { defer dbConn.Close() - _, err = dbConn.ExecContext(context.Background(), ` + _, err = dbConn.ExecContext( + context.Background(), ` CREATE TABLE devices ( guid TEXT PRIMARY KEY, hostname TEXT NOT NULL DEFAULT '', @@ -1070,9 +1198,16 @@ func TestDeviceRepo_GetByColumn(t *testing.T) { password TEXT NOT NULL DEFAULT '', usetls BOOLEAN NOT NULL DEFAULT FALSE, allowselfsigned BOOLEAN NOT NULL DEFAULT FALSE, - certhash TEXT NOT NULL DEFAULT '' + certhash TEXT NOT NULL DEFAULT '', + id TEXT NOT NULL DEFAULT '', + createddate TEXT NOT NULL DEFAULT '', + isdeleted BOOLEAN NOT NULL DEFAULT FALSE, + deleteddate TEXT NOT NULL DEFAULT '', + producttype TEXT NOT NULL DEFAULT '', + connectiontype TEXT NOT NULL DEFAULT '' ); - `) + `, + ) require.NoError(t, err) tc.setup(dbConn)