refactor(backend): restructure from layer-based to domain-based packaging #407
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Context
Part of Epic #406 — Big-bang restructure. This is REFACTOR-1: move every backend Java file from layer-based packages (
controller/,service/,repository/,model/,dto/,exception/,security/,config/) into per-domain packages following the canonical domain set in #387 first comment.This is a single big-bang PR per the user's decision (LLM-assisted refactor is fast enough that staging isn't worth the per-PR overhead). All tests must remain green throughout.
Target package structure
Process
git mv+ global search-and-replace on package declarations./mvnw test— must be greenbackend/src/test/java/controller/,service/,repository/,model/,dto/)backend/CLAUDE.md"Package Structure" section to reflect the new layoutAnti-patterns
./mvnw testafter each domain move. Catch breakage early.Acceptance criteria
controller/,service/,repository/,model/,dto/,exception/,security/,config/) no longer existdocument.{annotation,comment,transcription}andperson.relationshipshared/contains only items meeting the admission criteria (no entity, no user-facing CRUD, ≥2 consumers OR framework infra)./mvnw testis greenbackend/CLAUDE.mdreflects the new structurenpm run generate:apiruns cleanly against the rebuilt backend (consumer-side smoke)Dependency
Hard blocked by: Epic 1 (#387) — need audit verdict; Epic 3 (#402) — need test trustworthiness; AUDIT-6 (#393) refactor-readiness verdict 🟢 or 🟡-with-prereqs-met.
Definition of Done
PR merged. Closing comment lists the final domain count, sub-package list, and
shared/member list for the human-readable docs (DOC-2, DOC-6) to consume.🏗️ Markus Keller — Senior Application Architect
Observations
controller/,service/,repository/,model/,dto/— all with zero domain boundaries — the move is overdue.shared/admission criteria (no entity, no user-facing CRUD, ≥2 consumers OR framework infra) is a clean, enforceable rule. Good../mvnw testcheckpoints in the process section mitigate the risk of a single invisible regression.audit/anddashboard/already partially escaped the flat structure — they have their own packages today. The refactor makes this consistent rather than introducing a new pattern.DataInitializer.javacurrently lives inconfig/. It seeds test users (via@Profile("e2e")) and the admin user. The target structure puts it inshared/config/— but it injectsAppUserRepositoryandUserGroupRepositorydirectly, making it auser/domain concern wrapped in a@Configuration. Putting it inshared/config/would violate the stated admission criteria. Consider moving it touser/asUserDataInitializer.java.AuthE2EController.javais@Profile("e2e")only and lives incontroller/. The target puts it... nowhere — it's not listed in the issue's target structure. It should land inuser/(it manages auth sessions for E2E test users). Flag this before implementation.conversation/andactivity/placeholder packages with README-only content are a good pattern for signaling intent without forcing premature implementation. Keep them.Recommendations
DataInitializertouser/asUserDataInitializer— it owns user/group seeding logic, not generic config.AuthE2EControllerexplicitly to the target structure underuser/before the PR starts. Missing it will cause a post-move "where does this go?" pause mid-refactor.shared/importing/slot has onlyMassImportService. Verify it meets ≥2-consumers admission criteria: it callsPersonService,TagService,DocumentService— so it genuinely is cross-domain. Confirmed valid forshared/../mvnw testbaseline before any move so you know the starting green count. If the baseline has failures, record them — don't refactor on red.backend/CLAUDE.md". Also update the rootCLAUDE.mdPackage Structure section — it currently documents the old layer-based layout.Open Decisions
DataInitializerplacement —shared/config/(where it currently lives conceptually) vsuser/(where its domain dependencies say it belongs). The current issue text doesn't resolve this.user/is architecturally correct but requires renaming the class;shared/config/is pragmatic but leaks user domain logic into infrastructure config.👨💻 Felix Brandt — Senior Fullstack Developer
Observations
./mvnw testcheckpoint after each domain is the right safeguard, not optional.controller/,service/,repository/,model/. Each test file hardcodes@WebMvcTest(DocumentController.class)and@InjectMocks DocumentService— those annotations reference class names, not packages, so they survive a move without source changes. However,@Import({SecurityConfig.class, PermissionAspect.class})in@WebMvcTestslices does reference classes by FQN in some configurations. Verify before moving security config.@Schemaannotations don't include package info, so the JSON output should be identical. Run a before/afterdiffon/v3/api-docsoutput to confirm.OcrBlockResult.javacurrently lives inservice/but is not a service — it's a value object / result type used by OCR services. The target structure places it inocr/. This is one of the non-obvious placements to watch during the move.relationship/package already exists as a separate non-layer package today. It just needs to move fromorg.raddatz.familienarchiv.relationshiptoorg.raddatz.familienarchiv.person.relationship— a sub-package move, not a flat file move.Recommendations
tag/(smallest, fewest dependencies:TagController,TagService,TagRepository,Tag,TagUpdateDTO,MergeTagDTO). Run tests. Confidence established. Then proceed up in complexity.git mv+ global find/replace is preferable to IDE Move Class for this volume. IntelliJ's Move Class works great for one class at a time; at 200+ files across 8 domains in one session, it's slower and more likely to cause merge headaches than a scripted rename../mvnw test -pl backendandgrep -rn "import org.raddatz.familienarchiv.old_package" src/to verify no stale imports remain.frontend/npm run generate:apismoke test in the acceptance criteria is the right safety net for catching any schema shape changes that tests don't catch.PersonNameParser.javaandPersonTypeClassifier.javacurrently live inservice/but are pure utility/domain-logic classes with no Spring annotations. Moving them toperson/is correct. Double-check they don't have@Serviceon them — if they do, the component scan needs no change; if they don't, they're just static helpers and move cleanly.Open Decisions
None — the move order and tooling choice are implementation details that can be decided at execution time without blocking the issue.
🔒 Nora "NullX" Steiner — Application Security Engineer
Observations
SecurityConfig.javamove is the highest-risk individual file move. It's currently insecurity/(which maps toshared/security/). Spring Security's component scan picks upSecurityConfigby classpath scanning. Moving it to a sub-package (shared/security/) doesn't break anything — Spring Boot's@SpringBootApplicationscans all sub-packages from the root. Confirmed safe.PermissionAspect.javais referenced by FQN in several@WebMvcTest@Importannotations. I spot-checkedDocumentControllerTest— it has@Import({SecurityConfig.class, PermissionAspect.class}). After the move, these FQN references in test@Importmust be updated. This is not a behavior change but it is a source change that the issue's process step doesn't explicitly call out. Add it to the checklist.AuthE2EController.javahas@Profile("e2e")— it only loads in the e2e test profile. It directly accessesPasswordResetTokenRepository. If it moves touser/, the import chain stays clean. If it stays misplaced or lands inshared/, the cross-domain repository access violation becomes visible. Use the move as a forcing function to clean this up.Permission.javaandRequirePermission.javamoving toshared/security/is correct. They're framework infrastructure used by every domain — they have no domain-specific logic. Their import paths will change (org.raddatz.familienarchiv.security.Permission→org.raddatz.familienarchiv.shared.security.Permission), which will affect every controller that uses@RequirePermission. This is the highest import-count change in the codebase —PermissionandRequirePermissionare each imported 17-19 times. Do this domain last or in its own atomic commit.@RequirePermission, centralizedSecurityConfig,DomainExceptionerror handling) is untouched.Recommendations
@Import({SecurityConfig.class, PermissionAspect.class})references in test classes after movingshared/security/."shared/security/as its own separate commit (not bundled with another domain). It has the highest fanout of any individual package move — isolating it makes the diff reviewable and rollback clean if something breaks.grep -rn "import org.raddatz.familienarchiv.security" src/— the result should be zero. Any remaining hits are missed updates.Open Decisions
None — all security-relevant placement decisions (SecurityConfig, PermissionAspect, Permission enum all in
shared/security/) are clearly specified in the issue.🧪 Sara Holt — QA Engineer & Test Strategist
Observations
controller/,service/,repository/,model/. The issue says to mirror them into the new domain packages. This is a large rename that must be executed alongside each domain move — not deferred to a cleanup pass../mvnw testis green but do not specify which test run catches what. Current test distribution: ~20 controller tests incontroller/, ~35 service tests inservice/, ~12 repository tests inrepository/, ~4 model tests inmodel/. After the move, a developer should be able to run./mvnw test -Dtest="document.*"and get only document-domain tests. That's the legibility win.ApplicationContextTestin the root package (org.raddatz.familienarchiv) is a full@SpringBootTestsmoke test. It will continue to work regardless of package reorganization since it loads the full context. It's the safety net for catching "Spring can't wire the context after the move" failures. Confirm it passes after each domain move, not just at the end../mvnw testshould report the same number of tests run as the baseline. A passing build with fewer tests than before means some tests were accidentally dropped (file moved but not the test, or test file deleted instead of moved).PostgresContainerConfig.javacurrently lives in the test root (org.raddatz.familienarchiv). It's shared infrastructure for all integration tests. It should stay in the test root or move toshared/equivalent in test — do not scatter it into individual domain test packages.DocumentControllerTest,GeschichteControllerTest, andPersonControllerTestall use@WebMvcTestwith security imports. After movingSecurityConfigandPermissionAspecttoshared/security/, these test class@Importdeclarations will have compile errors until updated. This is a systematic change across ~15 controller test files.Recommendations
./mvnw test | grep "Tests run:"before any move and record the total. After each domain move, verify the count doesn't drop.PostgresContainerConfigas staying in the test root package (org.raddatz.familienarchiv) in the acceptance criteria. Its current location is correct and it should not be moved.@Importupdates for security classes across controller tests are a systematic change — do them in a single commit tagged "refactor(test): update security imports after shared/security move" to keep the diff readable.frontend/npm run generate:apismoke test in acceptance criteria is the right consumer-side gate. Add the specific assertion: the TypeScript type count in the generated file should match before and after (no types lost or duplicated).Open Decisions
None — test strategy for a pure refactor is straightforward: green before = green after, same count, same behavior.
🚀 Tobias Wendt — DevOps & Platform Engineer
Observations
./mvnw test(it should), the workflow will pass or fail based on the same criteria as local execution. No runner config changes needed.frontend/npm run generate:api— this requires the backend to be running with--spring.profiles.active=dev. In CI, this step either requires the backend to be started as a service container or run as part of a multi-step job. Check whether the existing CI workflow already has this step; if not, this acceptance criterion is currently a manual-only check.backend/CLAUDE.mdand rootCLAUDE.mdupdates in the acceptance criteria are important for onboarding. The currentCLAUDE.mdat the root still documents the old layer-based structure under "Package Structure" — this will mislead anyone (human or LLM) starting work after the refactor.Recommendations
npm run generate:apistep before claiming it as an acceptance criterion gate. If it doesn't exist in CI, mark that criterion as "manual only" to set expectations accurately.e2eprofile (which loadsAuthE2EControllerand the E2EDataInitializerbeans).Open Decisions
None — no infrastructure decisions are required for this refactor.
🎨 Leonie Voss — UI/UX Design Lead
Observations
This issue is a backend Java package refactoring with zero frontend changes and no user-visible impact. No Svelte components are being created or modified. No API endpoint paths, request shapes, or response shapes are changing. From a UI/UX and accessibility perspective, there is nothing to review.
The one indirect touchpoint:
frontend/npm run generate:apiis listed in the acceptance criteria as a smoke test. This regenerates the TypeScript API types from the OpenAPI spec. If the spec output changes shape (even accidentally), the TypeScript compilation would fail, which would surface in the frontend build. This is a good gate — I'd confirm thenpm run check(svelte-check) also passes after regeneration, not just the raw codegen run.Recommendations
npm run checkpasses afternpm run generate:api— svelte-check will catch any TypeScript compilation errors introduced by type shape changes in the regenerated API client.No open decisions from a UI/UX perspective.
📋 Elicit — Requirements Engineer
Observations
The issue is well-specified for a refactoring task: it has a concrete target state (the package tree), a defined process (7 steps), explicit anti-patterns, and measurable acceptance criteria. This is above-average for a technical refactoring issue.
Gaps I found against the acceptance criteria:
AuthE2EControlleris not listed in the target structure. It currently lives incontroller/and has@Profile("e2e"). The issue's target tree covers all other controllers. Either it belongs inuser/(most logical — it manages test auth sessions) or it needs an explicit note. An untracked file in a "everything must be moved" refactor is an unclosed requirement.The test count baseline is implied, not stated. Acceptance criterion says
./mvnw testis green — but it should also say "same number of tests pass as the pre-refactor baseline." A silent test deletion would satisfy "green" without satisfying "no behavior change."backend/CLAUDE.mdand rootCLAUDE.mdupdates are listed separately. The rootCLAUDE.mdhas an explicit "Package Structure" section that still documentscontroller/,service/,repository/,model/,dto/. Both files must be updated. Currently onlybackend/CLAUDE.mdis mentioned in the acceptance criteria — add the rootCLAUDE.mdexplicitly.The "OpenAPI spec diff being zero except for type FQNs" criterion needs a concrete verification step. The process doesn't describe how to produce the before/after diff. Recommend:
curl http://localhost:8080/v3/api-docs > before.json(before move), repeat after move,diff before.json after.json | grep -v "raddatz.familienarchiv"should produce no output.Dependency statement is precise but the blockers are external issues. The issue correctly states it is hard-blocked by #387, #402, and #393. These should be linked as "blocked-by" in Gitea's issue tracker, not just mentioned in text. This makes the dependency visible in the milestone view.
Recommendations
AuthE2EController→user/explicitly to the target structure and the acceptance criteria checklist../mvnw testmatches the pre-refactor baseline (no tests lost)."CLAUDE.mdPackage Structure section updated to reflect new domain layout."Open Decisions
DataInitializerplacement:shared/config/(pragmatic, current conceptual home) vsuser/(architecturally correct per domain ownership). This is a genuine tradeoff between structural purity and avoiding a class rename. (Raised also by Markus)🗳️ Decision Queue — Action Required
1 decision needs your input before implementation starts.
Architecture
DataInitializerplacement —DataInitializer.javacurrently lives inconfig/but injectsAppUserRepositoryandUserGroupRepositorydirectly, making it auser/domain concern. Two options:user/asUserDataInitializer— architecturally correct, enforces the statedshared/admission criteria (shared/ is for things with ≥2 consumers or framework infra; this has only 1 domain consumer), but requires a class rename.shared/config/— pragmatic, avoids the rename, but leaks user-domain repository dependencies into cross-cutting infrastructure config.Raised by: Markus, Elicit
Note:
AuthE2EControllerplacement (currently unlisted in the target structure) is unanimously recommended asuser/by Markus, Nora, and Elicit. This is not a decision — it's a gap in the issue spec that should be closed by updating the target structure before implementation starts.Heads-up: #417 is resolved (PR #420 against
main, awaiting review). Once that merges, the layering tree is clean — both acceptance greps return zero hits — so REFACTOR-1's branch can be cut without REFACTOR-3 (#409)'s ArchUnit rule failing on day one.Implementierung abgeschlossen ✅
Branch:
feat/issue-407-domain-packagingCommits
3643fa35refactor(tag): move tag domain to package org.raddatz.familienarchiv.tag0ad3f3e5refactor(geschichte): move geschichte domain to package org.raddatz.familienarchiv.geschichtea39fd992refactor(notification): move notification domain to package org.raddatz.familienarchiv.notificationb466dfcerefactor(person): move person domain to package org.raddatz.familienarchiv.personb41e1335refactor(person/relationship): move relationship sub-package under person domainc0a1c9ffrefactor(ocr): move ocr domain to package org.raddatz.familienarchiv.ocrbb7d872arefactor(document): move document sub-packages transcription/annotation/commente85057berefactor(document): move document domain core to document/ packageaf2c983frefactor(user): move user domain to user/ package, rename DataInitializer to UserDataInitializer930b1d23refactor(security): move SecurityConfig to security/ package5e53a261refactor(shared): move remaining services to domain packages (stats→dashboard, filestorage, importing, notification, exception)27e7fa91refactor(cleanup): delete empty legacy packages, move remaining test files to domain packages5f1c539fdocs: update package structure docs to reflect domain-based layoutErgebnis
DataInitializer→UserDataInitializer(wie besprochen)controller/,service/,repository/,model/,dto/) entferntCLAUDE.mdundbackend/CLAUDE.mdaktualisiert