Skip to content

Separate POCO defaults from value-expression evaluation in TableOperations#37

Open
ritchiecarroll wants to merge 1 commit into
developmentfrom
refactor-table-operations
Open

Separate POCO defaults from value-expression evaluation in TableOperations#37
ritchiecarroll wants to merge 1 commit into
developmentfrom
refactor-table-operations

Conversation

@ritchiecarroll

@ritchiecarroll ritchiecarroll commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Decouples TableOperations<T> from the Gemstone.Expressions type registry so the common "just construct and go" case works without any external setup.

Previously, TableOperations<T>.NewRecord() always evaluated attribute-based value expressions (DefaultValueExpressionAttribute / UpdateValueExpressionAttribute) against the type registry. When a model expression referenced an uninitialized dependency (e.g. Settings), simple usage such as new TableOperations<Device>(connection).NewRecord() would throw — an unknowable hidden dependency for a casual user.

Changes

  • TableOperations<T> is now pure POCO. It honors only the standard System.ComponentModel.DefaultValueAttribute via a self-contained compiled applier built with System.Linq.Expressions — no type-registry / Settings coupling. Removed the expression scope, the three expression delegates, InitializeType(), and the static TypeRegistry property.
  • New ExpressionTableOperations<T> : TableOperations<T> is the opt-in surface that restores the full value-expression behavior (a strict superset, identical to the prior default). Callers using it own the responsibility of initializing any dependencies their expressions reference.
  • Added three protected virtual seams (CreateRecordInstance, ApplyRecordDefaultValues, ApplyRecordUpdateValues) that the derived type overrides.
  • SecureTableOperations<T> now wraps ExpressionTableOperations<T> to preserve existing behavior.

Verification

  • Gemstone.Data builds clean (0 errors).
  • New DB-free unit tests (TableOperationsDefaultsTest) confirm: base NewRecord() applies [DefaultValue] but does not evaluate expressions; ExpressionTableOperations<T>.NewRecord() evaluates both.

Breaking change

External callers relying on the base TableOperations<T> to auto-evaluate value expressions must switch to ExpressionTableOperations<T>.

Related PRs

This is the core library change. Consumer updates:

🤖 Generated with Claude Code

StephenCWills
StephenCWills previously approved these changes Jul 9, 2026
@StephenCWills StephenCWills dismissed their stale review July 9, 2026 17:54

It's an all-or-nothing sort of fix.

@StephenCWills

Copy link
Copy Markdown
Contributor

Here is an example of a commonly used DefaultValueExpression which actually does (more or less) model the database defaults and does not require any hidden dependencies.

[DefaultValueExpression("DateTime.UtcNow")]
public DateTime CreatedOn { get; set; }

The changes here suggest that we cannot support any default value expressions if we don't depend on all of the type registry's defaults in their entirety. As a result, various database models will not be automatically populated with database defaults when using TableOperations<T>. Not only does this discourage the use of TableOperations<T>, but it provides no middle ground for applications which are simply using IConfiguration instead of the Settings API.

Furthermore, I would argue that our C# models should, first and foremost, reflect the database they're modelling. Second, that dependencies should be hooked by the application rather than expected by the library. The way I see it, this PR intends to continue having the library expect dependencies so long as the application opts in. However, a quick look at the related PRs shows that it's still trivially easy to hide the dependency behind a convenience method, such as IndependentAdapterManagerExtensions.GetMeasurementRecord(). Developers may unintentionally end up leaving traps throughout the codebase for any application that does not supply the expected dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants