Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions data/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,15 @@ func (model *BaseModel) BeforeCreate(db *gorm.DB) error {
if model.Version <= 0 {
created, err := createdAtFromID(model.ID)
if err != nil {
return err
// Caller-supplied non-xid IDs — typically from test fixtures or
// legacy migrations — cannot derive a deterministic timestamp.
// Fall back to time.Now() and warn so the invariant violation is
// visible without blocking the insert.
util.Log(db.Statement.Context).
WithError(err).
WithField("id", model.ID).
Warn("BaseModel.ID is not a valid xid; falling back to time.Now() for CreatedAt")
Comment on lines +113 to +116
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The log message is redundant because the id and the validation error are already included in the log fields via WithField("id", model.ID) and WithError(err). Simplifying the message improves log clarity and reduces verbosity.

Suggested change
util.Log(db.Statement.Context).
WithError(err).
WithField("id", model.ID).
Warn("BaseModel.ID is not a valid xid; falling back to time.Now() for CreatedAt")
util.Log(db.Statement.Context).
WithError(err).
WithField("id", model.ID).
Warn("falling back to time.Now() for CreatedAt")

created = time.Now()
}
model.CreatedAt = created
model.ModifiedAt = created
Expand All @@ -116,8 +124,9 @@ func (model *BaseModel) BeforeCreate(db *gorm.DB) error {
}

// createdAtFromID returns the time component embedded in a generated xid.
// All BaseModel IDs must be valid xids so sort-by-id ≡ sort-by-created_at
// and hypertable promotions retain monotonic time ordering.
// Production IDs are always xids (via util.IDString), so this is the hot
// path. Non-xid IDs return an error so callers — e.g. BeforeCreate — can
// decide whether to bail out or fall back.
func createdAtFromID(id string) (time.Time, error) {
parsed, err := xid.FromString(id)
if err != nil {
Expand Down
11 changes: 5 additions & 6 deletions datastore/repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -370,26 +370,25 @@ func (s *RepositoryTestSuite) TestCreate() {
name: "create entity with pre-set xid",
setupEntity: func(_ context.Context) *TestEntity {
entity := &TestEntity{
Name: "Entity with ID",
Name: "Entity with xid",
}
// Caller-supplied IDs must be valid xids so CreatedAt can be
// derived deterministically from the embedded timestamp.
entity.ID = util.IDString()
return entity
},
expectError: false,
},
{
name: "create entity with non-xid ID should fail",
name: "create entity with non-xid ID falls back to time.Now()",
setupEntity: func(_ context.Context) *TestEntity {
entity := &TestEntity{
Name: "Entity with legacy id",
}
// Non-xid IDs (typically test fixtures) are accepted with a
// WARN log; CreatedAt falls back to time.Now().
entity.ID = fmt.Sprintf("custom-id-%d", time.Now().UnixNano())
return entity
},
expectError: true,
errorMsg: "is not a valid xid",
expectError: false,
},
{
name: "create entity with version > 0 should fail",
Expand Down
Loading