fix: validate that UUID field is lower-case

[wschurman]

Why

Technically upper-case UUIDs are valid according to https://datatracker.ietf.org/doc/html/rfc4122.

But the way entity is written expects case sensitivity due to them being treated as simple strings. For example, this would be the expected behavior:

const entities = await TestEntity.loader(vc).loadManyByFieldEqualingAsync('id', ['018EBFDA-DC80-782D-A891-22A0AA057D52']);
=> Map(['018EBFDA-DC80-782D-A891-22A0AA057D52', TestEntity[...]])

The issue is that Postgres stores them in a case-insensitive manner (which is correct according to the UUID spec). So in actuality, the above code would theoretically do something like:

const entities = await TestEntity.loader(vc).loadManyByFieldEqualingAsync('id', ['018EBFDA-DC80-782D-A891-22A0AA057D52']);
=> Map(['018ebfda-dc80-782d-a891-22a0aa057d52', TestEntity[...]])

(it actually fails due to this change of case further up in the stack at

entity/packages/entity/src/EntityDatabaseAdapter.ts

Line 182 in 077d53c

`Unexpected object field value during database result transformation: ${objectMapKeyForObject}. This should never happen.`,

, but the reason is the same: postgres transforms to lower case under-the-hood)

This results in breaking invariants the framework provides (all keys in === all keys out).

This PR solves this problem by just enforcing UUID casing in fields to match what Postgres already does to avoid this issue.

Ref: WWW-37

How

I first tried a field normalization approach in #309, but this also had the same problem: the fields in !== fields out. Unless we were to invert the normalization on the way out, but that comes with a lot of added complexity.

So this is the solution I propose: just don't allow any upper-case characters in the UUIDs. This is technically not UUID spec compliant since we no longer accept all technically valid UUIDs.

Test Plan

Run the test.

[wschurman]

• fix: validate that UUID field is lower-case #310 👈 (View in Graphite)
• chore: fix erroneous describe.only and add lint rule to prevent it #308 : 1 other dependent PR (#309 )
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.40%. Comparing base (02f92fc) to head (026dab9).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff            @@
##             main     #310    +/-   ##
========================================
  Coverage   97.40%   97.40%            
========================================
  Files          87       87            
  Lines       11952    11954     +2     
  Branches     1018      608   -410     
========================================
+ Hits        11642    11644     +2     
  Misses        303      303            
  Partials        7        7            

Flag	Coverage Δ
integration	6.09% <0.00%> (-0.01%)	⬇️
unittest	94.31% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[linear]

WWW-37 Invariant Violation: Unexpected object field value during database result transformation: SingleFieldValue(770e1996-cc9b-4ff9-85af-57dadde7615d). This should never happen.

[ide]

Is there a place to document it? The limitation is fine IMO as long as it's documented since it deviates from the spec.

[wschurman]

Merge activity

• Sep 30, 7:48 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• Sep 30, 7:50 PM UTC: Graphite rebased this pull request as part of a merge.
• Sep 30, 7:52 PM UTC: @wschurman merged this pull request with Graphite.