Skip to content

refactor(domain): Consolidate Flag mutation methods by concern — Phase 2 - #63

Merged
amodelandme merged 3 commits into
devfrom
refactor/flag-mutation-consolidation
May 11, 2026
Merged

amodelandme merged 3 commits into
devfrom
refactor/flag-mutation-consolidation

Conversation

@amodelandme

Copy link
Copy Markdown
Owner

Summary

Replaces the trio Flag.SetEnabled / Flag.UpdateStrategy / Flag.Update with a single concern-named mutation Flag.Reconfigure(bool, RolloutStrategy, StrategyConfig). The two field-shaped methods had no production callers — deleting them removes a partial-mutation bug class (toggle enabled while leaving a now-mismatched strategy behind). The atomic Update method is renamed to Reconfigure to declare full replacement, not partial patching. UpdateName and Archive are unchanged. Public HTTP API, DTOs, and IBanderasService signatures are byte-for-byte identical to before.

Changes in this PR

  • Domain + Application — Flag.cs deletes SetEnabled and UpdateStrategy; renames Update → Reconfigure with a concern-named XML doc. BanderasService.UpdateFlagAsync adopts the new method name (one line).
  • Tests — drop deleted-method tests, rename Update* test methods to Reconfigure*, remove the redundant UpdateStrategy mismatch test, and update the optimistic concurrency integration test to race Reconfigure against UpdateName.
  • Docs — spec + implementation notes added under Docs/Decisions/refactor-flag-mutation-consolidation/. Foundation docs updated: architecture.md (Domain Integrity principle + GET-query environment-validation ratification), current-state.md (phase status, archived-terminal note, test counts, 2026-05-11 Lessons Learned), roadmap.md (Phase 2 checkbox + progress sentence), flag-ddd-analysis-backlog.md (backlog item checked off).

Spec

Docs/Decisions/refactor-flag-mutation-consolidation/spec.md

Implementation Notes

Docs/Decisions/refactor-flag-mutation-consolidation/implementation-notes.md

Definition of Done

  • SetEnabled and UpdateStrategy removed from Flag.cs
  • Update renamed to Reconfigure; XML doc rewritten to name the concern
  • BanderasService.UpdateFlagAsync calls flag.Reconfigure(...)
  • No file in repo (excluding the spec) references flag.SetEnabled(, flag.UpdateStrategy(, or flag.Update( — verified by grep
  • FlagArchivedInvariantTests covers Reconfigure, UpdateName, Archive; deleted method tests removed
  • StrategyConfigTests references Reconfigure only; redundant UpdateStrategy mismatch test removed
  • FlagConcurrencyTokenTests continues to demonstrate the optimistic concurrency contract
  • dotnet build -p:TreatWarningsAsErrors=true succeeds across all projects (0 warnings / 0 errors)
  • dotnet csharpier check . reports no violations (104 files checked)
  • All unit tests pass (153/153 — 5 fewer than baseline, all due to deleted-method coverage being folded)
  • All integration tests pass (54/54, count unchanged)
  • Requests/smoke-test.http unchanged; PUT /api/flags/{name} returns the same 200 OK FlagResponse
  • DDD backlog item Consolidate SetEnabled() + UpdateStrategy() + Update() checked off in Docs/Decisions/flag-ddd-analysis-backlog.md

Testing

Domain unit tests (cover AC-1 through AC-5):

dotnet test Banderas.Tests/Banderas.Tests.csproj \
  --filter "FullyQualifiedName~FlagArchivedInvariantTests|FullyQualifiedName~StrategyConfigTests"

Expected: all archived-guard, success, and config-mismatch tests pass; no test references SetEnabled or UpdateStrategy.

Integration suite (covers AC-6 byte-for-byte API parity and AC-7 optimistic concurrency):

dotnet test Banderas.Tests.Integration/Banderas.Tests.Integration.csproj

Expected: 54/54 green. FlagConcurrencyTokenTests.ConcurrentUpdate_SecondSave_ThrowsFlagConcurrencyExceptionAsync demonstrates the optimistic concurrency contract via Reconfigure + UpdateName.

Manual smoke (no expected change vs. baseline):

curl -X PUT http://localhost:5051/api/flags/checkout-flow \
  -H "Content-Type: application/json" \
  -d '{"environment":"Development","isEnabled":true,"strategyType":"None","strategyConfig":null}'

Expected: 200 OK with FlagResponse body, identical to the pre-refactor baseline.

Static check that no caller revived a deleted method:

grep -rn "\.SetEnabled(\|\.UpdateStrategy(\|flag\.Update(" --include="*.cs" .

Expected: zero hits.

Developer and others added 3 commits May 11, 2026 20:14
Replace SetEnabled / UpdateStrategy / Update with a single concern-named
mutation Flag.Reconfigure(bool, RolloutStrategy, StrategyConfig). The two
field-shaped methods had no production callers; deleting them removes a
partial-mutation bug class (toggle enabled with a now-mismatched strategy
left behind). The atomic Update method is renamed to Reconfigure to
declare full replacement, not partial patching. UpdateName and Archive
are unchanged.

No public API change: IBanderasService signatures, request/response
DTOs, controller routes, and HTTP status codes are byte-for-byte
identical to before. Only BanderasService.UpdateFlagAsync (one line)
adopts the new method name.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop the archived-guard and success tests for the deleted SetEnabled and
UpdateStrategy methods; rename the surviving rollout-mutation tests from
Update* to Reconfigure*. Remove the redundant UpdateStrategy mismatch
test in StrategyConfigTests — the rule is now exercised once on
Reconfigure. FlagConcurrencyTokenTests races flagA.Reconfigure against
flagB.UpdateName to demonstrate the optimistic concurrency contract
unchanged.

Unit test count drops from 158 to 153; integration test count is
unchanged at 54. Observable coverage of the success and invariant
paths is preserved on the surviving methods.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…acement

Add spec and implementation notes for the Flag mutation consolidation.
Update foundation docs:
- architecture.md: Domain Integrity principle names Reconfigure /
  UpdateName / Archive as the canonical mutation surface, organized
  by concern rather than by field; adds a subsection ratifying
  EnvironmentRules.RequireValid as the canonical environment-sentinel
  guard, with the GET-vs-mutating-endpoint asymmetry explicitly
  documented.
- current-state.md: marks the consolidation phase complete; updates
  the archived-terminal note (3 methods, not 5); refreshes the test
  counts (153 unit, 54 integration); adds the 2026-05-11 Lessons
  Learned entry on concern-named mutations.
- roadmap.md: checks off the consolidation item under the Phase 2
  Flag-invariants list; removes the resolved env-validation decision
  from Current Focus.
- flag-ddd-analysis-backlog.md: checks off the consolidation item with
  a provenance note pointing back to this PR.

Spec: Docs/Decisions/refactor-flag-mutation-consolidation/spec.md

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@amodelandme
amodelandme marked this pull request as ready for review May 11, 2026 20:16
@amodelandme
amodelandme merged commit f1666b5 into dev May 11, 2026
9 checks passed
@amodelandme
amodelandme deleted the refactor/flag-mutation-consolidation branch May 11, 2026 20:17
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.

1 participant