Fix code generation when EntityAssociation is used with string constructor

[Daniel-Svensson]

Fix #571

Summary by CodeRabbit

• Chores
  • Updated entity relationship metadata declarations and improved internal handling of association attributes for enhanced consistency.

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes address code generation failures during Association to EntityAssociation migration by updating the builder to treat association key members as IReadOnlyList<string> instead of string[], eliminating problematic casts while preserving single vs multi-key constructor branching logic.

Changes

Cohort / File(s)	Summary
Builder Type Handling src/OpenRiaServices.Tools/Framework/MetadataPipeline/EntityAssociationAttributeBuilder.cs	Refactored GetAttributeDeclaration to use IReadOnlyList<string> for association key members, removing unnecessary casts and switching constructor selection condition from array .Length to list .Count.
Test Case Migration src/Test/Desktop/OpenRiaServices.Common.DomainServices.Test/Cities/CityEntities.cs	Updated two navigation property attributes from Association to EntityAssociation with explicit string array member lists, aligning test cases with the builder changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• PR #512: Modifies how EntityAssociationAttribute keys are represented and handled, addressing the same attribute generation/consumption path affected by this migration.

Poem

🐰 From strings in arrays to lists we bound,
No casts shall haunt our code around,
One key, or many—logic flows,
As EntityAssociation grows! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main fix: resolving code generation failures when EntityAssociation is used with string constructor arguments.
Linked Issues check	✅ Passed	The changes address issue #571 by fixing EntityAssociationAttributeBuilder to handle readonly list types instead of string arrays, eliminating the casting error.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the EntityAssociation code generation issue; no unrelated modifications are present.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/OpenRiaServices.Tools/Framework/MetadataPipeline/EntityAssociationAttributeBuilder.cs (1)

34-35: 🧹 Nitpick | 🔵 Trivial

Optional: drop redundant .ToArray() materialization.

Now that the locals are IReadOnlyList<string>, you could materialize once into a list (or keep .ToArray() — either is fine). Not blocking; leaving as-is is acceptable since AssociationAttribute.ThisKeyMembers returns IEnumerable<string> and must be materialized before the .Count/indexing operations below.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/OpenRiaServices.Tools/Framework/MetadataPipeline/EntityAssociationAttributeBuilder.cs`
around lines 34 - 35, The locals thisKey and otherKey are being materialized
with .ToArray() though they are typed as IReadOnlyList<string>; to avoid
redundant allocations change the materialization to a single list (e.g., call
.ToList() once) or assign the IEnumerable directly if you only need read-only
indexing—replace the current associationAttribute.ThisKeyMembers.ToArray() and
associationAttribute.OtherKeyMembers.ToArray() usages with a single
materialization (.ToList()) or an appropriate IReadOnlyList<string> construction
so subsequent Count/indexing uses operate on that single materialized collection
(refer to the locals thisKey, otherKey and
associationAttribute.ThisKeyMembers/OtherKeyMembers).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In
`@src/OpenRiaServices.Tools/Framework/MetadataPipeline/EntityAssociationAttributeBuilder.cs`:
- Around line 34-35: The locals thisKey and otherKey are being materialized with
.ToArray() though they are typed as IReadOnlyList<string>; to avoid redundant
allocations change the materialization to a single list (e.g., call .ToList()
once) or assign the IEnumerable directly if you only need read-only
indexing—replace the current associationAttribute.ThisKeyMembers.ToArray() and
associationAttribute.OtherKeyMembers.ToArray() usages with a single
materialization (.ToList()) or an appropriate IReadOnlyList<string> construction
so subsequent Count/indexing uses operate on that single materialized collection
(refer to the locals thisKey, otherKey and
associationAttribute.ThisKeyMembers/OtherKeyMembers).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 71c28fdb-a31d-4cd8-8cd6-14df3d6e9449

📥 Commits

Reviewing files that changed from the base of the PR and between cbb540b and 55735b7.

📒 Files selected for processing (2)

• src/OpenRiaServices.Tools/Framework/MetadataPipeline/EntityAssociationAttributeBuilder.cs
• src/Test/Desktop/OpenRiaServices.Common.DomainServices.Test/Cities/CityEntities.cs

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud