Merge/57 with 32 user persistence - #83
Conversation
- Added User entity and UserRepository - Implemented OIDC user synchronization service - Added UserController with /api/me endpoint - Updated SecurityConfig to enable OIDC login and user sync - Updated Project and File controllers to enforce ownership - Rewrote Frontend Profile page to display real user data - Updated backend tests to use OIDC mocks
- Updated README with sudo commands for docker compose - Refactored backend Dockerfile to optionally build frontend - Updated docker-compose.prod.yaml to bundle frontend in backend - Added docker-compose.dev.yaml and .dockerignore - Configured Vite proxy for development
- Updated FileController and ProjectController base paths - Updated frontend API calls to match new endpoints
…ture ## Build Optimizations - Skip frontend build during backend tests (95% faster: 1m38s → 3-5s) - Add conditional frontend build via -PbuildFrontend flag - Add buildProduction task for explicit production builds - Update Dockerfile to use new build flags ## Testing Infrastructure - Add Vitest + React Testing Library for frontend testing - Add root package.json for monorepo test orchestration - Add Gradle tasks: testAll, testFrontend, testAllSequential - Add parallel test execution (backend + frontend in ~5-10s) - Add sample tests and test setup configuration ## Test Endpoints Fix - Update all test endpoints from /file to /api/file - Update all test endpoints from /project to /api/project - All 33 backend tests now passing ## Documentation - Add comprehensive Testing section to README - Add Build Commands documentation - Add examples for writing tests - Document all test execution options ## Performance Impact - Backend tests: 1m38s → 3-5s (95% improvement) - Frontend tests: ~2-3s - Combined tests (parallel): ~5-10s total - Production builds: unchanged (~2m with frontend) Closes #32
## Architecture Changes - Restructure backend to follow hexagonal/clean architecture principles - Separate concerns into core, application, and infrastructure layers ## Core Layer (Domain) - Move all domain models to core/filesystem/model package - Consolidate User, File, Directory, Project, and related models - Keep domain logic independent of frameworks ## Application Layer (Use Cases) - Create service layer in application/filesystem/service - Add AuthService, UserService, FileService, ProjectService - Move OidcUserSyncService to application layer - Implement business logic and orchestration ## Infrastructure Layer (Adapters) - Move REST controllers to infrastructure/filesystem/in/web/rest - Relocate FileController, ProjectController, AuthController, UserController - Controllers now delegate to application services ## Repository Updates - Update all repository imports to use new core model locations - Update savers to reference new model packages - Maintain existing repository functionality ## Test Updates - Update test imports to match new package structure - Ensure all 33 tests continue to pass - Update FileControllerTest and ProjectControllerTest This refactoring improves maintainability, testability, and follows clean architecture best practices while maintaining all existing functionality.
- Add User domain model (POJO) in core/user/model - Add UserServicePort, OidcSyncServicePort, AuthServicePort in application/user/ports/in - Add UserRepositoryPort in application/user/ports/out - Add UserService, OidcUserSyncService, AuthService in application/user/services - Add UserRestAdapter, AuthRestAdapter in infrastructure/user/in/web/rest - Add UserResponse DTO and UserDtoMapper - Add JpaUser entity, UserJpaMapper, UserJpaAdapter in infrastructure/user/out/db/jpa - Add SpringDataUserRepository for JPA operations - Update SecurityConfig to use OidcSyncServicePort interface - Remove old AuthController and UserController (replaced by new adapters) - Include MERGE_STRATEGY.md documentation This implements the ports & adapters (hexagonal) architecture for the user domain, separating the pure domain model from infrastructure concerns.
…- Add UserServiceTest with tests for findById, findByIssuerAndSub, getAuthenticatedUser, and verifyProjectOwnership methods- Add OidcUserSyncServiceTest with tests for new user creation, existing user updates, error handling, and multi-issuer scenarios- Add AuthServiceTest with tests for authentication status, user info retrieval, and logout functionality- Add UserIntegrationTest with tests for /api/me, /api/auth/status, /api/auth/user, /api/auth/logout endpoints and security verification- Add @qualifier to UserRestAdapter to inject correct UserServicePort- Configure OIDC mock with proper client registration for integration testsAll tests pass successfully.
…Phase 6)- Remove application/filesystem/service/AuthService.java (unused)- Remove application/filesystem/service/OidcUserSyncService.java (unused)These services were duplicates of the new hexagonal architectureimplementations in application/user/services/. The new versionsare properly qualified with @service annotations and are the onesactually used by the SecurityConfig and REST adapters.The following files remain as they are still in use byFileController and ProjectController:- security/repository/UserRepository.java- application/filesystem/service/UserService.javaThese can be migrated in a future refactoring task.
The pattern '**/out' was too broad and was matching 'application/user/ports/out/' which contains UserRepositoryPort.java. Changed to only exclude build output directories at specific locations.
## Docker Configuration - Expand .dockerignore with comprehensive exclusions (IDE, logs, OS files, environment files) - Optimize backend/Dockerfile to conditionally install Node.js only when SKIP_FRONTEND is false - Improve Dockerfile build stage by moving ARG definitions and simplifying Gradle commands - Add documentation comments to Dockerfile for dev vs prod build modes
…with-mariadb' and fix frontend API URLs
Implemented port and adapter pattern across filesystem and user domains. Moved security configuration to infrastructure config. Integrated Lombok to reduce boilerplate in domain models. Refactored exceptions to be domain-specific.
- Ignore 'createdOn' property in Project and Directory mappers - Domain models now use Lombok @Getter/@Setter which MapStruct tries to map automatically
- Move User domain object resolution from service layer to REST adapters - Inject UserServicePort into REST adapters to resolve AuthenticatedUser to User - Apply @PreAuthorize("isAuthenticated()") to REST adapter methods - Update ProjectService, DirectoryService, and FileService to accept User domain object - Remove UserServicePort dependency from service implementations - Update corresponding unit and integration tests - Remove unused imports in service classes
…pplication layer - Move SecurityContextHolder access from AuthService to AuthRestAdapter - AuthServicePort now accepts AuthenticatedUser and user info as parameters - AuthRestAdapter extracts authentication from Spring Security context - Remove 'newAuthService' and 'newOidcUserSyncService' bean qualifiers - Update AuthServiceTest to test with domain objects instead of mocking Spring Security This ensures the application layer is framework-agnostic per Clean Architecture.
- Apply Lombok @Getter and @Setter to Project and JpaProject - Apply Lombok @Getter, @Setter, and @NoArgsConstructor to JpaUser - Remove manual getters/setters in these classes to reduce boilerplate - Convert UserJpaMapper from manual implementation to MapStruct interface for consistency
- Convert FileElementDtoMapper to Spring @component to break circular dependency - Rename generic 'domain' parameters to entity-specific names to improve processor compatibility - Simplify @mapping annotations in mappers
furcev32
left a comment
There was a problem hiding this comment.
🚀 Backend Architecture Review - Summary of Findings
Overall, the implementation of the Ports & Adapters (Hexagonal) architecture is very well executed. Below are specific technical recommendations for the files changed in this PR:
1. 🛡️ Global Exception Handling
- File:
GlobalExceptionHandler.java - Recommendation: Add logging in the
handleGeneralError(fallback) method. Currently, unexpected exceptions are caught but the stacktrace is not logged, which will making debugging production issues difficult.
2. 🔐 Security & Configuration
- File:
SecurityConfig.java - Recommendation:
- Solid implementation of PKCE and OIDC.
- CORS Configuration: The allowed origins are currently managed via
frontendUrl. Ensure that production configurations only allow the official domain. - Secrets: Ensure that DB passwords and OIDC client secrets are injected via environment variables (e.g., using the
DotenvLoaderlogic) and never committed to version control.
3. 🏗️ Infrastructure & Persistence
- Files:
JpaFileElement.java,JpaFileElementContainer.java,JpaProject.java, etc. - Recommendation: Standardize the use of Lombok (
@Getter,@Setter,@NoArgsConstructor) in JPA entities to match the domain model's brevity and consistency. - Highlight: The use of recursive CTEs in
SpringDataFileElementContainerRepository.javafor ownership verification is an excellent, performance-oriented solution for deep directory hierarchies.
4. 📂 Mapper Complexity
- Files:
FileElementDtoMapper.java,DirectoryDtoMapper.java - Note: The use of
@Lazyand manualFileElementDtoMapperlogic correctly addresses the recursive nature of the filesystem tree while avoiding circular dependency issues in MapStruct.
5. ✅ Reliability
- Note: The addition of comprehensive integration tests (
ProjectLifecycleIntegrationTest) significantly improves the reliability of the filesystem operations.
furcev32
left a comment
There was a problem hiding this comment.
🔍 Deep Dive Code Review: Merge of Architecture (#57) & Persistence (#32)
I have analyzed the differences between the architectural base and the new persistence logic. Here are my findings per file:
🏛️ Architecture & Services
ProjectService.java/FileService.java:- Logic Change: Successfully integrated the new
Userdomain model. The services now correctly perform ownership checks using theUserobject passed from the REST adapters. - Recommendation: In
FileService.verifyOwnershipByParentId, consider making the error message more descriptive for the user (but keep the log detailed for admins).
- Logic Change: Successfully integrated the new
🔐 Infrastructure & Security
SecurityConfig.java:- Logic Change: Excellent addition of PKCE and the
AuthenticationSuccessHandler. - Observation: You've moved this to the
infrastructure.configpackage, which is the correct place for framework-specific configuration in Clean Architecture. - Note: The CSRF token handling (
CsrfCookieFilter) is well-implemented for SPA compatibility.
- Logic Change: Excellent addition of PKCE and the
🗄️ Persistence Layer (JPA)
JpaUser.java/UserJpaAdapter.java:- Observation: The transition from OIDC claims to a persistent MariaDB user record is seamless.
- Draft Improvement: The
UserJpaMapperis currently manual. Since you are using MapStruct for other mappers, consider converting this one too for consistency.
🌐 Frontend Integration
Profile.tsx:- Logic Change: Great job replacing the mock data with a real API call to
api.get<UserDto>('/api/me'). - UX Note: You added a loading spinner (
Loader2), which significantly improves the perceived performance.
- Logic Change: Great job replacing the mock data with a real API call to
AuthContext.tsx:- Improvement: Changing
window.location.reload()towindow.location.href = '/'is a cleaner way to reset the application state after logout.
- Improvement: Changing
⚠️ Critical Fixes Needed
GlobalExceptionHandler.java:- Line 92: The TODO for logging in
handleGeneralErrorshould be addressed before merging to ensure we don't lose error context in production.
- Line 92: The TODO for logging in
There was a problem hiding this comment.
MODIFIED: FileElement Domain Model
Changes Made:
- Integrated Lombok
@Getter/@Setterannotations
Why These Changes: Reduces boilerplate code while maintaining the same functionality. This is the base class for all file system elements (File, Directory).
- Refactor FileElement from empty interface to type alias of FileElementDto - Remove redundant comment in getElementForFileElement
NicKIT01
left a comment
There was a problem hiding this comment.
Hab nochmal 2 kleinere Sachen angemerkt alles in allem sehr sauber und bei meinen manuellen FrontendTests ist mir auch nichts mehr aufgefallen. Sehr gut!
|
Ich sehe, du hast beim profile tab ziemlich viel entfernt. Was ist denn da jetzt noch wichtiges drinne? Sieht gerade etwas useless aus. |
Stimmt, momentan ist da noch nicht viel zu sehen. Ich wollte aber erst mal das Grundgerüst stehen haben. Es ist quasi der Platzhalter für kommende Erweiterungen (eventuell auch für die nächste Praktikumsgruppe). |
Merge/57 with 32 user persistence
No description provided.