Skip to content

Commit

Permalink
feat(impl): [#199] Harden test and clean method signature
Browse files Browse the repository at this point in the history
 1) harden test PolicyStoreServiceTest#whenUpdate by asserting that the creation timestamp is not changed and that the validUntil timestamp is changed as expected

 2) clean method signature by moving loop from controller to the service method
  • Loading branch information
dsmf committed Mar 13, 2024
1 parent dcdd24e commit 58a3212
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ public class PolicyStoreController {
ref = "#/components/examples/error-response-403"))
}),
})

@PostMapping("/policies")
@ResponseStatus(HttpStatus.CREATED)
@PreAuthorize("hasAuthority('" + IrsRoles.ADMIN_IRS + "')")
Expand Down Expand Up @@ -237,6 +238,7 @@ public void deleteAllowedPolicy(@PathVariable("policyId") final String policyId)
@ResponseStatus(HttpStatus.OK)
@PreAuthorize("hasAuthority('" + IrsRoles.ADMIN_IRS + "')")
public void updateAllowedPolicy(final @Valid @RequestBody UpdatePolicyRequest request) {
request.policiesIds().forEach(policyId -> service.updatePolicy(policyId, request));
service.updatePolicies(request);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,13 @@ public void deletePolicy(final String policyId) {

}

public void updatePolicy(final String policyId, final UpdatePolicyRequest request) {
public void updatePolicies(final UpdatePolicyRequest request) {
for (final String policyId : request.policiesIds()) {
updatePolicy(policyId, request.validUntil(), request.businessPartnerNumbers());
}
}

private void updatePolicy(final String policyId, final OffsetDateTime validUntil, final List<String> bpns) {
try {
log.info("Updating policy with id {}", policyId);
final List<String> bpnsContainingPolicyId = getAllStoredPolicies().entrySet()
Expand All @@ -156,18 +162,18 @@ public void updatePolicy(final String policyId, final UpdatePolicyRequest reques
.toList();

final Policy policyToUpdate = getStoredPolicies(bpnsContainingPolicyId).stream()
.filter(policy -> policy.getPolicyId()
.equals(policyId))
.findAny()
.orElseThrow(
() -> new PolicyStoreException(
"Policy with id '"
+ policyId
+ "' doesn't exists!"));

policyToUpdate.update(request.validUntil());
.filter(policy -> policy.getPolicyId()
.equals(policyId))
.findAny()
.orElseThrow(
() -> new PolicyStoreException(
"Policy with id '"
+ policyId
+ "' doesn't exists!"));

policyToUpdate.update(validUntil);
bpnsContainingPolicyId.forEach(bpn -> persistence.delete(bpn, policyId));
persistence.save(request.businessPartnerNumbers(), policyToUpdate);
persistence.save(bpns, policyToUpdate);
} catch (final PolicyStoreException e) {
throw new ResponseStatusException(HttpStatus.NOT_FOUND, e.getMessage(), e);
}
Expand All @@ -176,7 +182,11 @@ public void updatePolicy(final String policyId, final UpdatePolicyRequest reques
@Override
public List<AcceptedPolicy> getAcceptedPolicies(final List<String> bpns) {
if (bpns == null) {
return getAllStoredPolicies().values().stream().flatMap(Collection::stream).map(this::toAcceptedPolicy).toList();
return getAllStoredPolicies().values()
.stream()
.flatMap(Collection::stream)
.map(this::toAcceptedPolicy)
.toList();
}

final ArrayList<Policy> policies = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ void updateAllowedPolicy() {
testee.updateAllowedPolicy(request);

// assert
verify(service).updatePolicy(policyId, request);
verify(service).updatePolicies(request);
}

private List<Permission> createPermissions() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,38 +205,52 @@ void whenRegisterPolicyWithMissingPermissionsShouldThrowException() {

// act
// assert
assertThrows(ResponseStatusException.class, () -> testee.registerPolicy(policy,
null));
assertThrows(ResponseStatusException.class, () -> testee.registerPolicy(policy, null));
}

@Test
void whenRegisterPolicyWithMissingConstraintShouldThrowException() {
// arrange
final Policy policy = Policy.builder()
.permissions(List.of(
Permission.builder().constraint(new Constraints(emptyList(), emptyList())).build(),
Permission.builder().build()
))
.permissions(List.of(Permission.builder()
.constraint(
new Constraints(emptyList(), emptyList()))
.build(), Permission.builder().build()))
.build();

// act
// assert
assertThrows(ResponseStatusException.class, () -> testee.registerPolicy(policy,
null));
assertThrows(ResponseStatusException.class, () -> testee.registerPolicy(policy, null));
}

@Test
void whenUpdate() {
// arrange
final OffsetDateTime now = OffsetDateTime.now(clock);
when(persistence.readAll()).thenReturn(Map.of("bpn2", List.of(new Policy("testId", now, now.plusMinutes(1), emptyList()))));
when(persistence.readAll(any())).thenReturn(List.of(new Policy("testId", now, now.plusMinutes(1), emptyList())));
final String policyId = "testId";

final String originalBpn = "bpn2";
final String expectedBpn = "bpn1";

final OffsetDateTime createdOn = OffsetDateTime.now(clock).minusDays(10);
final OffsetDateTime originalValidUntil = createdOn.plusMinutes(1);
final OffsetDateTime expectedValidUntil = createdOn.plusDays(3);

final List<Permission> permissions = emptyList();

final Policy testPolicy = new Policy(policyId, createdOn, originalValidUntil, permissions);
when(persistence.readAll()).thenReturn(Map.of(originalBpn, List.of(testPolicy)));
when(persistence.readAll(originalBpn)).thenReturn(
List.of(new Policy(policyId, createdOn, originalValidUntil, permissions)));

// act
testee.updatePolicy("testId", new UpdatePolicyRequest(OffsetDateTime.now(), List.of("bpn1"), List.of("policyId")));
testee.updatePolicies(new UpdatePolicyRequest(expectedValidUntil, List.of(expectedBpn), List.of(policyId)));

// assert
verify(persistence).delete("bpn2", "testId");
verify(persistence).save(eq(List.of("bpn1")), any());
verify(persistence).delete(originalBpn, policyId);

final var policyCaptor = ArgumentCaptor.forClass(Policy.class);
verify(persistence).save(eq(List.of(expectedBpn)), policyCaptor.capture());
assertThat(policyCaptor.getValue().getCreatedOn()).isEqualTo(createdOn);
assertThat(policyCaptor.getValue().getValidUntil()).isEqualTo(expectedValidUntil);
}
}

0 comments on commit 58a3212

Please sign in to comment.