191 lesen und schreiben der stimmzettelumschläge #663
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces a comprehensive implementation for managing ballot envelope counts in the election result reporting system. This involves creating a new domain entity The implementation follows a layered architecture with clear separation of concerns. It includes database migration scripts for both H2 and Oracle databases, Keycloak role and authority configurations, and extensive test coverage across different layers. The changes enable functionality to retrieve and update ballot envelope counts for specific elections and districts, with robust validation and security mechanisms in place. Assessment against linked issues
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (21)
wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeModel.java (2)
7-10: Document the purpose of anzahlWaehler2The purpose of the secondary voter count field
anzahlWaehler2is not immediately clear. Please add documentation to explain its specific use case in the election process.
7-10: Consider adding additional validation constraintsTo ensure data integrity in the election process, consider adding these validations:
- Range validation for voter counts (
@PositiveOrZero)- Temporal validation for
urneneroeffnungsUhrzeitto prevent future datesExample implementation:
public record StimmzettelumschlaegeModel(@NotNull BezirkUndWahlID bezirkUndWahlID, + @PastOrPresent LocalDateTime urneneroeffnungsUhrzeit, + @NotNull @PositiveOrZero long anzahlWaehler, + @PositiveOrZero long anzahlWaehler2) {wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeModelMapperTest.java (1)
16-39: Consider improving variable naming consistency and test data documentation.While the test structure and coverage are good, consider these improvements:
- The variable name
wahlscheineEntity(ballot papers entity) doesn't match the actual typeStimmzettelumschlaege(ballot envelopes)- The test values (47, 11) would benefit from comments explaining their significance
Consider this improvement:
- val wahlscheineEntity = new Stimmzettelumschlaege(new BezirkUndWahlID(wahlID, wahlbezirkID), urneneroeffnungsUhrzeit, anzahlWaehler, (long) anzahlWaehler2); + // Example values: 47 regular voters, 11 special voters (e.g., postal votes) + val stimmzettelumschlaege = new Stimmzettelumschlaege(new BezirkUndWahlID(wahlID, wahlbezirkID), urneneroeffnungsUhrzeit, anzahlWaehler, (long) anzahlWaehler2);wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeModelMapper.java (1)
9-11: Add JavaDoc and consider validation annotations.The mapping methods would benefit from clear documentation and null validation:
+ /** + * Converts a ballot envelope entity to its model representation. + * @param entity the ballot envelope entity to convert + * @return the corresponding model + */ + @NonNull StimmzettelumschlaegeModel toModel(Stimmzettelumschlaege entity); + /** + * Converts a ballot envelope model to its entity representation. + * @param model the ballot envelope model to convert + * @return the corresponding entity + */ + @NonNull Stimmzettelumschlaege toEntity(StimmzettelumschlaegeModel model);wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/domain/stimmzettelumschlaege/StimmzettelumschlaegeRepository.java (1)
39-41: Bulk cache eviction
Evicting all entries on bulk deletion is correct if the domain logic requires discarding all cache entries. Just ensure that this is not too broad, especially if the cache is shared.Also applies to: 44-46
wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeService.java (1)
47-53: Exception handling in save operation
Catching a genericExceptionensures all errors are logged, but consider restricting to more specific exceptions if you want different handling for data integrity issues vs. transient errors.wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/rest/stimmzettelumschlaege/StimmzettelumschlaegeController.java (1)
74-79: Idempotency considerations for POST
When re-sending a previously posted request, evaluate whether the resulting data overwrites or merges. Currently, it appears to simply overwrite. If partial updates are required in the future, consider a PATCH endpoint.wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/domain/stimmzettelumschlaege/Stimmzettelumschlaege.java (1)
12-16: Add JPA auditing and optimistic lockingConsider adding:
- Version field for optimistic locking to prevent concurrent modifications
- Audit fields to track creation and modification timestamps
@Entity @Data @NoArgsConstructor @AllArgsConstructor +@EntityListeners(AuditingEntityListener.class) public class Stimmzettelumschlaege {Add the following fields:
@Version private Long version; @CreatedDate private LocalDateTime createdAt; @LastModifiedDate private LocalDateTime lastModifiedAt;wls-ergebnismeldung-service/src/test/resources/stimmzettelumschlaege.http (1)
29-37: Add test cases for validation boundariesThe test payload should include additional test cases to verify validation constraints:
- Negative voter counts
- Future dates
- Missing required fields
Consider adding test cases with the following scenarios:
### Test negative voter count { ... "anzahlWaehler": -1 } ### Test future date { ... "urneneroeffnungsUhrzeit": "2025-12-18T14:52:41.263Z" } ### Test missing required field { ... "bezirkUndWahlID": null }wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/rest/stimmzettelumschlaege/StimmzettelumschlaegeDTOMapperTest.java (2)
24-38: Consider adding edge case tests for ToDTO conversion.While the basic conversion test is good, consider adding tests for:
- Boundary values (e.g., 0, Integer.MAX_VALUE for anzahlWaehler)
- Different datetime scenarios (e.g., past dates, midnight)
Example test:
@Test void should_handleBoundaryValues_when_givenModel() { val model = new StimmzettelumschlaegeModel( new BezirkUndWahlID("wahlID", "wahlbezirkID"), LocalDateTime.of(2024, 1, 1, 0, 0), // midnight 0, // minimum voters Integer.MAX_VALUE // maximum voters ); val result = unitUnderTest.toDTO(model); Assertions.assertThat(result.getAnzahlWaehler()).isZero(); Assertions.assertThat(result.getAnzahlWaehler2()).isEqualTo(Integer.MAX_VALUE); }
49-64: Consider adding edge case tests for ToModel conversion.Similar to the ToDTO tests, consider adding boundary value tests for the ToModel conversion to ensure symmetrical behavior.
docs/src/services/ergebnismeldung-service/index.md (1)
61-64: Consider minor grammatical improvements.The new section about ballot envelope counting is informative, but could benefit from minor grammatical refinements.
Consider this revision:
-Bei der Briefwahl zählt der Wahlvorstand die Stimmzettelumschlaege. -Über das Schreiben und Lesen der Stimmzettelumschlaege kann die aktuelle Anzahl eines Wahlbezirkes für eine Wahl an WLS -übermittelt bzw. ausgelesen werden. +Bei der Briefwahl zählt der Wahlvorstand die Stimmzettelumschläge. +Über das Schreiben und Lesen der Stimmzettelumschläge kann die aktuelle Anzahl der Umschläge eines Wahlbezirkes für eine Wahl an WLS +übermittelt bzw. ausgelesen werden.wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeValidatorTest.java (1)
68-68: Consider adding boundary tests for voter counts.The test creates a
StimmzettelumschlaegeModelwith zero values, but it might be worth adding tests for negative numbers or extremely large values.+ @ParameterizedTest + @ValueSource(ints = {-1, Integer.MIN_VALUE, Integer.MAX_VALUE}) + void should_validateVoterCounts(int count) { + val model = new StimmzettelumschlaegeModel(null, null, count, count); + Assertions.assertThatException() + .isThrownBy(() -> unitUnderTest.validStimmzettelumschlaegeOrThrow(model)); + }wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeServiceSecurityTest.java (2)
134-140: Consider parameterizing test data creation.The
createStimmzettelumschlaegeModelmethod uses hardcoded values. Consider extracting these as constants or using a test data builder pattern for better maintainability.+ private static final int DEFAULT_ANZAHL_WAEHLER = 47; + private static final int DEFAULT_ANZAHL_WAEHLER_2 = 11; + private StimmzettelumschlaegeModel createStimmzettelumschlaegeModel(BezirkUndWahlID id) { val urneneroeffnungsUhrzeit = LocalDateTime.now(); - val anzahlWaehler = 47; - val anzahlWaehler2 = 11; + val anzahlWaehler = DEFAULT_ANZAHL_WAEHLER; + val anzahlWaehler2 = DEFAULT_ANZAHL_WAEHLER_2; return new StimmzettelumschlaegeModel(id, urneneroeffnungsUhrzeit, anzahlWaehler, anzahlWaehler2); }
82-93: Consider adding concurrent access test.While the basic security tests are good, consider adding a test for concurrent access scenarios to ensure thread safety of the permission checks.
+ @Test + void should_handleConcurrentAccess() { + SecurityUtils.runWith(Authorities.ALL_AUTHORITIES_SET_STIMMZETTELUMSCHLAEGE); + val wahlbezirkID = "wahlbezirkID"; + val id = new BezirkUndWahlID("wahlID", wahlbezirkID); + + Mockito.when(bezirkIDPermissionEvaluator.tokenUserBezirkIdMatches(eq(wahlbezirkID), notNull())) + .thenReturn(true); + + // Create multiple threads to simulate concurrent access + val executor = Executors.newFixedThreadPool(3); + try { + List<Future<?>> futures = IntStream.range(0, 3) + .mapToObj(i -> executor.submit(() -> { + val model = createStimmzettelumschlaegeModel(id); + unitUnderTest.setStimmzettelumschlaege(id, model); + return null; + })) + .collect(Collectors.toList()); + + futures.forEach(f -> Assertions.assertThatNoException().isThrownBy(f::get)); + } finally { + executor.shutdown(); + } + }wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeServiceTest.java (3)
146-148: Consider enhancing the helper method with more realistic test data.The
createStimmzettelumschlaegeModelhelper method uses default values (null, 0, 0) which might not represent realistic scenarios. Consider parameterizing the method to create more meaningful test data.- private StimmzettelumschlaegeModel createStimmzettelumschlaegeModel(final BezirkUndWahlID id) { - return new StimmzettelumschlaegeModel(id, null, 0, 0); + private StimmzettelumschlaegeModel createStimmzettelumschlaegeModel( + final BezirkUndWahlID id, + final LocalDateTime urneneroeffnungsUhrzeit, + final int anzahlWaehler, + final long anzahlWaehler2) { + return new StimmzettelumschlaegeModel( + id, + urneneroeffnungsUhrzeit != null ? urneneroeffnungsUhrzeit : LocalDateTime.now(), + anzahlWaehler, + anzahlWaehler2); + }
134-137: Enhance exception message for better debugging.The mock exception message "saving failed" could be more descriptive to help with debugging test failures.
- val mockedRepositorySaveException = new RuntimeException("saving failed"); + val mockedRepositorySaveException = new RuntimeException("Failed to save StimmzettelumschlaegeModel to repository");
84-144: Add test cases for edge scenarios.The test suite would benefit from additional test cases covering edge scenarios:
- Negative voter counts
- Maximum allowed voter counts
- Validation of urneneroeffnungsUhrzeit (urn opening time) constraints
wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/rest/stimmzettelumschlaege/StimmzettelumschlaegeControllerIntegrationTest.java (2)
195-197: Make URI builder more robust.The current URI builder method could be enhanced to handle special characters and ensure proper URL encoding.
private String buildStimmzettelumschlaegeURI(final String wahlID, final String wahlbezirkID) { - return "/businessActions/stimmzettelumschlaege/" + wahlID + "/" + wahlbezirkID; + return String.format("/businessActions/stimmzettelumschlaege/%s/%s", + URLEncoder.encode(wahlID, StandardCharsets.UTF_8), + URLEncoder.encode(wahlbezirkID, StandardCharsets.UTF_8)); }
116-193: Add test for concurrent modifications.The
PostStimmzettelumschlaegetest class should include a test case for concurrent modifications to ensure proper handling of race conditions.Example test to add:
@Test void should_handleConcurrentModifications() throws Exception { val wahlID = "wahlID"; val wahlbezirkID = "wahlbezirkID"; val requestBody = new StimmzettelumschlaegeDTO(/* ... */); // Simulate concurrent requests CompletableFuture<MockHttpServletResponse> future1 = CompletableFuture.supplyAsync(() -> mockMvc.perform(/* request 1 */).andReturn().getResponse() ); CompletableFuture<MockHttpServletResponse> future2 = CompletableFuture.supplyAsync(() -> mockMvc.perform(/* request 2 */).andReturn().getResponse() ); // Assert that one request succeeds and the other fails appropriately CompletableFuture.allOf(future1, future2).join(); // Add assertions }wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/rest/stimmzettelumschlaege/StimmzettelumschlaegeControllerTest.java (1)
20-31: Enhance test readability with @DisplayName annotations.Consider adding @DisplayName annotations to improve test readability and documentation. Also, consider extracting test data into constants.
Example improvement:
@ExtendWith(MockitoExtension.class) +@DisplayName("StimmzettelumschlaegeController Tests") class StimmzettelumschlaegeControllerTest { + private static final String TEST_WAHL_ID = "wahlID"; + private static final String TEST_WAHLBEZIRK_ID = "wahlbezirkID";
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
docs/src/services/ergebnismeldung-service/index.md(1 hunks)stack/keycloak/migration/add-authorities-ergebnismeldung-stimmzettelumschlaege.yml(1 hunks)stack/keycloak/migration/keycloak-changelog.yml(1 hunks)wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/domain/stimmzettelumschlaege/Stimmzettelumschlaege.java(1 hunks)wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/domain/stimmzettelumschlaege/StimmzettelumschlaegeRepository.java(1 hunks)wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/exception/ExceptionConstants.java(2 hunks)wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/rest/stimmzettelumschlaege/StimmzettelumschlaegeController.java(1 hunks)wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/rest/stimmzettelumschlaege/StimmzettelumschlaegeDTO.java(1 hunks)wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/rest/stimmzettelumschlaege/StimmzettelumschlaegeDTOMapper.java(1 hunks)wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeModel.java(1 hunks)wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeModelMapper.java(1 hunks)wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeService.java(1 hunks)wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeValidator.java(1 hunks)wls-ergebnismeldung-service/src/main/resources/db/migrations/h2/V3_0__createTableStimmzettelumschlaege.sql(1 hunks)wls-ergebnismeldung-service/src/main/resources/db/migrations/oracle/V3_0__createTableStimmzettelumschlaege.sql(1 hunks)wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/rest/stimmzettelumschlaege/StimmzettelumschlaegeControllerIntegrationTest.java(1 hunks)wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/rest/stimmzettelumschlaege/StimmzettelumschlaegeControllerTest.java(1 hunks)wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/rest/stimmzettelumschlaege/StimmzettelumschlaegeDTOMapperTest.java(1 hunks)wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeModelMapperTest.java(1 hunks)wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeServiceSecurityTest.java(1 hunks)wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeServiceTest.java(1 hunks)wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeValidatorTest.java(1 hunks)wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/utils/Authorities.java(3 hunks)wls-ergebnismeldung-service/src/test/resources/stimmzettelumschlaege.http(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- wls-ergebnismeldung-service/src/main/resources/db/migrations/oracle/V3_0__createTableStimmzettelumschlaege.sql
- wls-ergebnismeldung-service/src/main/resources/db/migrations/h2/V3_0__createTableStimmzettelumschlaege.sql
- wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/exception/ExceptionConstants.java
🧰 Additional context used
📓 Learnings (1)
wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/rest/stimmzettelumschlaege/StimmzettelumschlaegeControllerIntegrationTest.java (1)
Learnt from: MrSebastian
PR: it-at-m/Wahllokalsystem#621
File: wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/rest/status/StatusControllerIntegrationTest.java:38-44
Timestamp: 2024-12-06T14:52:05.607Z
Learning: In `StatusControllerIntegrationTest`, integration tests are intended to verify the integration of all components and do not include security tests.
🔇 Additional comments (26)
wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeModel.java (1)
1-11: LGTM! The overall structure is well-designed
The record implementation is clean and uses appropriate types for each field. The use of Jakarta validation for required fields is good practice.
wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeModelMapperTest.java (3)
1-11: LGTM! Well-organized imports and package structure.
The imports are clean, necessary, and well-organized. Good use of lombok.val for concise test code.
12-14: LGTM! Proper mapper initialization.
Good practice using MapStruct's factory method for mapper creation and maintaining it as a final field.
41-63: Verify the int/long type conversion handling.
The explicit casting (long) anzahlWaehler2 in the entity creation suggests a type mismatch between the model (using int) and entity (using long). This could lead to potential issues with large numbers.
Let's verify the type definitions in both classes:
wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeModelMapper.java (2)
1-4: LGTM! Clean package structure and imports.
The package naming and import statements follow best practices with a clear domain-driven structure.
1-12: Verify integration with related components.
Let's ensure the mapper is properly integrated with its dependencies:
✅ Verification successful
Let me gather more information about the model class and service usage to complete the verification.
Let me check one more aspect to verify the model class definition since it wasn't visible in the previous results.
Let me check the content of the model class since we found its location.
Integration verified successfully
The mapper is properly integrated with all related components:
- Model and entity classes are well-defined with matching fields (BezirkUndWahlID, urneneroeffnungsUhrzeit, anzahlWaehler, anzahlWaehler2)
- Mapper is correctly used in the service layer for entity-model conversions
- Comprehensive test coverage exists including:
- Unit tests for the mapper (StimmzettelumschlaegeModelMapperTest)
- Integration tests verifying the complete flow
- Service layer tests validating the mapping operations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for the existence and structure of related classes
echo "Checking for related model and entity classes..."
rg -l "class Stimmzettelumschlaege"
rg -l "class StimmzettelumschlaegeModel"
# Check for proper usage in service layer
echo "Checking mapper usage in service layer..."
rg -l "StimmzettelumschlaegeModelMapper"
# Verify test coverage
echo "Checking for mapper tests..."
fd -g "*StimmzettelumschlaegeModelMapperTest*"
Length of output: 3428
Script:
#!/bin/bash
# Check the model class definition
echo "Checking StimmzettelumschlaegeModel definition..."
rg -A 10 "class StimmzettelumschlaegeModel"
# Check how the mapper is used in the service
echo "Checking mapper usage in service..."
rg -A 5 "StimmzettelumschlaegeModelMapper" wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeService.java
# Check test implementation
echo "Checking mapper test implementation..."
rg -A 10 "class StimmzettelumschlaegeModelMapperTest" wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeModelMapperTest.java
Length of output: 3730
Script:
#!/bin/bash
# Find and check the model class definition
echo "Searching for StimmzettelumschlaegeModel file..."
fd StimmzettelumschlaegeModel.java
# If found, check its content
echo "Checking model class content..."
rg -A 10 "class StimmzettelumschlaegeModel" --type java
# Check entity class structure
echo "Checking entity class structure..."
rg -A 10 "class Stimmzettelumschlaege" wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/domain/stimmzettelumschlaege/Stimmzettelumschlaege.java
Length of output: 3149
Script:
#!/bin/bash
# Check the model class content
echo "Checking StimmzettelumschlaegeModel content..."
cat wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeModel.java
# Check for any other direct usages of the mapper
echo "Checking for other mapper usages..."
rg -B 3 -A 3 "stimmzettelumschlaegeModelMapper\." --type java
Length of output: 12490
wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/domain/stimmzettelumschlaege/StimmzettelumschlaegeRepository.java (4)
20-21: Cache key usage
Using the entire BezirkUndWahlID as the cache key is reasonable, but ensure its hashCode() and equals() methods are well-defined and stable to avoid cache inconsistencies.
23-26: Security and caching synergy
Good use of @CachePut in conjunction with the Ergebnismeldung_WRITE_Stimmzettelumschlaege authority. This ensures the updated data is immediately reflected in cache for subsequent reads.
28-31: Deletion semantics
Deleting a record by ID clears the cache only for that specific entry. Check if the domain logic expects dependent or related entities to be handled too (cascading deletions), or if partial invalidation might miss required updates.
11-12: Ensure consistent usage of READ authority in other layers
While the repository is annotated with READ authority, confirm that the service and controller layers align with the same authority constraints to avoid potential mismatches.
✅ Verification successful
Let me search for the service and controller layer security annotations to verify the authority constraints.
Let me search for the controller layer security annotations as we haven't found them yet.
Let me search for the endpoint mappings and security annotations in the controller class.
Let me check the service layer methods that are being called from the controller to complete our verification.
Based on the gathered information, I can now provide the final response:
Authority constraints are properly aligned across layers
The security model shows consistent and properly layered authority checks:
- Repository layer uses
Ergebnismeldung_READ_Stimmzettelumschlaegefor read operations - Service layer uses business-action authorities:
Ergebnismeldung_BUSINESSACTION_GetStimmzettelumschlaegefor readsErgebnismeldung_BUSINESSACTION_PostStimmzettelumschlaegefor writes
- Controller layer inherits the service layer's security constraints through method invocations
The authority hierarchy is well-structured with repository-level READ permissions being a lower-level concern, while the service layer handles business-action authorities, which is a good security practice.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searching for usage of StimmzettelumschlaegeRepository and verifying read constraints.
rg -A 5 "StimmzettelumschlaegeRepository"
Length of output: 12478
Script:
#!/bin/bash
# Search for StimmzettelumschlaegeService and StimmzettelumschlaegeController classes to check their security annotations
ast-grep --pattern $'@PreAuthorize($$$)
class StimmzettelumschlaegeService {
$$$
}'
ast-grep --pattern $'@PreAuthorize($$$)
class StimmzettelumschlaegeController {
$$$
}'
# Also search for any methods with PreAuthorize annotations in these classes
rg -A 2 "@PreAuthorize.*Stimmzettelumschlaege"
Length of output: 4773
Script:
#!/bin/bash
# Search for StimmzettelumschlaegeController class content
rg -A 10 "class StimmzettelumschlaegeController"
# Search for any endpoints related to Stimmzettelumschlaege
rg -A 2 "@(GetMapping|PostMapping|PutMapping|DeleteMapping).*Stimmzettelumschlaege"
Length of output: 6933
Script:
#!/bin/bash
# Search for the content of StimmzettelumschlaegeController.java
rg -A 5 "@(GetMapping|PostMapping|PutMapping|DeleteMapping|PreAuthorize).*" wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/rest/stimmzettelumschlaege/StimmzettelumschlaegeController.java
Length of output: 1187
Script:
#!/bin/bash
# Search for the methods in StimmzettelumschlaegeService
rg -A 5 "@PreAuthorize.*|public.*Stimmzettelumschlaege.*" wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeService.java
Length of output: 1738
wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeService.java (3)
25-26: Authority consistency
The get operation is secured with a distinct authority (Ergebnismeldung_BUSINESSACTION_GetStimmzettelumschlaege) from the repository read authority. Double-check that this authority is accounted for in Keycloak and that users have both if needed.
29-34: Validation approach
Excellent usage of a validator and a specialized exception for incomplete parameters. This helps maintain clarity and a consistent error-handling strategy.
36-39: Parameter-based permission check
The custom evaluator method @bezirkIdPermisionEvaluator.tokenUserBezirkIdMatches(...) is a neat approach for context-specific permission checks. However, confirm that the parameter name #param is correct and consistent with your evaluator's logic.
wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/rest/stimmzettelumschlaege/StimmzettelumschlaegeController.java (2)
30-40: Detailed Swagger documentation
The well-structured Swagger annotations and multiple response scenarios make the API usage clearer to clients. This can significantly reduce the overhead of external integration.
51-56: Check for negative or invalid district/election values
The GET endpoint relies on the service validator, but if there's any additional domain logic (e.g., verifying wahlID format), ensure it’s tested.
wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/rest/stimmzettelumschlaege/StimmzettelumschlaegeDTOMapper.java (1)
6-12: Mapper coverage
Both directions of mapping (DTO↔Model) are covered. Confirm that optional or nested fields in the model have correct default handling. If more fields are added in the future, ensure tests are kept in sync with these methods.
wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeValidator.java (1)
17-22: LGTM! Well-structured validation method.
The validation logic for BezirkUndWahlID is comprehensive, checking both null and blank conditions. Good use of StringUtils for robust string validation.
stack/keycloak/migration/keycloak-changelog.yml (1)
58-58: LGTM! Addition of ballot envelope authorities migration file.
The new migration file path follows the established naming convention and integrates well with the existing authority definitions.
docs/src/services/ergebnismeldung-service/index.md (1)
57-60: LGTM! Clear description of ballot placement process.
The documentation clearly explains the process of recording votes with ballot cards at the polling station.
wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/utils/Authorities.java (3)
16-17: LGTM! Service authority constants follow established naming patterns.
The new service authority constants maintain consistency with existing patterns and clearly indicate their purpose for getting and setting ballot envelope data.
31-33: LGTM! Repository authority constants follow CRUD pattern.
The repository authority constants correctly implement the standard CRUD operations (READ, DELETE, WRITE) pattern used throughout the codebase.
74-82: LGTM! Authority arrays properly group related permissions.
The authority arrays correctly group the service and repository permissions needed for get/set operations, following the established pattern in the codebase.
wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeValidatorTest.java (2)
1-30: LGTM! Well-structured test class setup.
The test class follows best practices with:
- Proper use of MockitoExtension
- Clear dependency injection
- Good organization using @nested classes
31-61: LGTM! Comprehensive validation testing for BezirkUndWahlId.
Excellent test coverage with:
- Positive case testing
- Parameterized testing for all invalid scenarios
- Clear test naming convention
wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeServiceSecurityTest.java (1)
32-49: LGTM! Proper test setup with security considerations.
Good practices observed:
- Using dedicated test profile
- Proper cleanup in @beforeeach
- Mocking of BezirkIDPermissionEvaluator
wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/rest/stimmzettelumschlaege/StimmzettelumschlaegeControllerTest.java (1)
1-86: Overall implementation approved with suggested improvements.
The test implementation provides good basic coverage of the controller's functionality. While there are no critical issues, implementing the suggested improvements would enhance the test suite's robustness and maintainability.
...m/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeModelMapper.java
Show resolved
Hide resolved
...llokalsystem/ergebnismeldungservice/rest/stimmzettelumschlaege/StimmzettelumschlaegeDTO.java
Show resolved
Hide resolved
...hllokalsystem/ergebnismeldungservice/domain/stimmzettelumschlaege/Stimmzettelumschlaege.java
Show resolved
Hide resolved
wls-ergebnismeldung-service/src/test/resources/stimmzettelumschlaege.http
Show resolved
Hide resolved
...tem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeValidator.java
Show resolved
Hide resolved
stack/keycloak/migration/add-authorities-ergebnismeldung-stimmzettelumschlaege.yml
Show resolved
Hide resolved
...eldungservice/rest/stimmzettelumschlaege/StimmzettelumschlaegeControllerIntegrationTest.java
Outdated
Show resolved
Hide resolved
...m/ergebnismeldungservice/rest/stimmzettelumschlaege/StimmzettelumschlaegeControllerTest.java
Outdated
Show resolved
Hide resolved
...m/ergebnismeldungservice/rest/stimmzettelumschlaege/StimmzettelumschlaegeControllerTest.java
Outdated
Show resolved
Hide resolved
…lokalsystem/ergebnismeldungservice/rest/stimmzettelumschlaege/StimmzettelumschlaegeControllerIntegrationTest.java Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…mzettelumschläge' into 191-lesen-und-schreiben-der-stimmzettelumschläge
dragonfly28
requested review from
MrSebastian and
vjohnslhm
and removed request for
vjohnslhm
...ava/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/exception/ExceptionConstants.java
Show resolved
Hide resolved
...tem/ergebnismeldungservice/service/stimmzettelumschlaege/StimmzettelumschlaegeValidator.java
Show resolved
Hide resolved
...eldungservice/rest/stimmzettelumschlaege/StimmzettelumschlaegeControllerIntegrationTest.java
Outdated
Show resolved
Hide resolved
...eldungservice/rest/stimmzettelumschlaege/StimmzettelumschlaegeControllerIntegrationTest.java
Outdated
Show resolved
Hide resolved
|
StimmzettelumschlaegeControllerIntegrationTest muss komplett mit Security laufen wegen des Checks auf die wahlbezirksArt im Authentication-Token des Service Verwendung von @WithMockUserAsJwt, siehe StimmzettelumschlaegeServiceSecurityTest |
Beschreibung:
siehe Issue
Backend
Closes #191
Summary by CodeRabbit
New Features
Bug Fixes
Tests