Skip to content

Commit

Permalink
fix: code smells (#1317)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jobulcke
jobulcke authored Jun 18, 2024
1 parent 8848c63 commit 19b63b5
Show file tree
Hide file tree
Showing 15 changed files with 327 additions and 349 deletions.
3 changes: 1 addition & 2 deletions ...a/be/vlaanderen/informatievlaanderen/ldes/server/fragmentation/FragmentDeletionSteps.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static be.vlaanderen.informatievlaanderen.ldes.server.domain.constants.RdfConstants.GENERIC_TREE_RELATION;
Expand All @@ -36,7 +35,7 @@ public Fragment FragmentEntryTransformer(Map<String, String> row) {
row.get("relation").isEmpty() ? new ArrayList<>()
: Arrays.stream(row.get("relation").split(",")).map(treeNode -> new TreeRelation("",
LdesFragmentIdentifier.fromFragmentId(treeNode), "", "", GENERIC_TREE_RELATION))
.collect(Collectors.toList()),
.toList(),
getDeleteTime(row.get("daysUntilDeletion")));
}

Expand Down
9 changes: 4 additions & 5 deletions ...ldes/server/fragmentisers/timebasedhierarchical/services/TimeBasedFragmentFinderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,12 @@ class TimeBasedFragmentFinderTest {
private static final Fragment PARENT = new Fragment(new LdesFragmentIdentifier(VIEW_NAME, List.of()));
private static final FragmentationTimestamp TIME = new FragmentationTimestamp(LocalDateTime.of(2023, 1, 1, 0, 0, 0),
Granularity.DAY);
private TimeBasedConfig config;
private TimeBasedFragmentCreator fragmentCreator;
private TimeBasedFragmentCreator fragmentCreator;
private TimeBasedFragmentFinder fragmentFinder;

@BeforeEach
void setUp() {
config = new TimeBasedConfig(".*", "", Granularity.DAY, false);
TimeBasedConfig config = new TimeBasedConfig(".*", "", Granularity.DAY, false);
fragmentCreator = mock(TimeBasedFragmentCreator.class);
fragmentFinder = new TimeBasedFragmentFinder(fragmentCreator, config);

Expand All @@ -54,8 +53,8 @@ void when_GetLowestIsCalled_Then_ReturnExpectedFragment() {

@Test
void when_GetDefaultIsCalled_Then_ReturnExpectedFragment() {
Fragment actual = fragmentFinder.getDefaultFragment(PARENT);
fragmentFinder.getDefaultFragment(PARENT);

verify(fragmentCreator, times(1)).getOrCreateFragment(PARENT, DEFAULT_BUCKET_STRING, Granularity.YEAR);
verify(fragmentCreator).getOrCreateFragment(PARENT, DEFAULT_BUCKET_STRING, Granularity.YEAR);
}
}
1 change: 0 additions & 1 deletion ...ren/informatievlaanderen/ldes/server/admin/domain/validation/ShaclShapeValidatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

import java.io.File;
import java.io.IOException;
import java.net.URISyntaxException;
import java.nio.charset.StandardCharsets;

import static org.assertj.core.api.Assertions.*;
Expand Down
5 changes: 2 additions & 3 deletions ...ava/be/vlaanderen/informatievlaanderen/ldes/server/admin/rest/config/ResourceRemover.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,14 @@
import org.springframework.stereotype.Component;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

@Component
public class ResourceRemover {
private final ApplicationEventPublisher applicationEventPublisher;
private List<String> usedStreams = new ArrayList<>();
private final List<String> usedStreams = new ArrayList<>();

private ResourceRemover(ApplicationEventPublisher applicationEventPublisher) {
protected ResourceRemover(ApplicationEventPublisher applicationEventPublisher) {
this.applicationEventPublisher = applicationEventPublisher;
}

Expand Down
7 changes: 0 additions & 7 deletions .../vlaanderen/informatievlaanderen/ldes/server/admin/rest/config/SpringIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
import be.vlaanderen.informatievlaanderen.ldes.server.admin.rest.controllers.*;
import be.vlaanderen.informatievlaanderen.ldes.server.domain.converter.PrefixAdderImpl;
import be.vlaanderen.informatievlaanderen.ldes.server.domain.converter.RdfModelConverter;
import be.vlaanderen.informatievlaanderen.ldes.server.domain.events.admin.EventStreamCreatedEvent;
import be.vlaanderen.informatievlaanderen.ldes.server.domain.events.admin.EventStreamDeletedEvent;
import be.vlaanderen.informatievlaanderen.ldes.server.domain.rest.PrefixConstructor;
import io.cucumber.spring.CucumberContextConfiguration;
import org.springframework.beans.factory.annotation.Autowired;
Expand All @@ -22,15 +20,10 @@
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.context.annotation.ComponentScan;
import org.springframework.context.event.EventListener;
import org.springframework.stereotype.Component;
import org.springframework.test.context.ActiveProfiles;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.web.servlet.MockMvc;

import java.util.ArrayList;
import java.util.List;

@SpringBootTest
@AutoConfigureMockMvc
@CucumberContextConfiguration
Expand Down
257 changes: 128 additions & 129 deletions ...ava/be/vlaanderen/informatievlaanderen/ldes/server/compaction/CompactionServiceSteps.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,134 +28,133 @@
@SuppressWarnings("java:S3415")
public class CompactionServiceSteps extends CompactionIntegrationTest {

@DataTableType
public ViewSpecification ViewSpecificationEntryTransformer(Map<String, String> row) {
return new ViewSpecification(
ViewName.fromString(row.get("viewName")),
List.of(), List.of(), Integer.parseInt(row.get("pageSize")));
}

@DataTableType
public FragmentAllocations FragmentAllocationsListEntryTransformer(Map<String, String> row) {
List<MemberAllocation> memberAllocations = new ArrayList<>();
String fragmentIdentifier = row.get("fragmentIdentifier");
for (String memberId : row.get("members").split(",")) {
memberAllocations.add(new MemberAllocation(fragmentIdentifier + "/" + memberId, "mobility-hindrances",
"by-page", fragmentIdentifier, memberId));
}
return new FragmentAllocations(fragmentIdentifier, memberAllocations);
}

@DataTableType
public MemberFragmentations MemberFragmentationsEntryTransformer(Map<String, String> row) {
return new MemberFragmentations(row.get("fragmentId"), Arrays.stream(row.get("memberIds").split(",")).toList());
}

@DataTableType(replaceWithEmptyString = "[blank]")
public Fragment FragmentEntryTransformer(Map<String, String> row) {
return new Fragment(
LdesFragmentIdentifier.fromFragmentId(row.get("fragmentIdentifier")),
Boolean.parseBoolean(row.get("immutable")), Integer.parseInt(row.get("nrOfMembersAdded")),
row.get("relation").isEmpty() ? new ArrayList<>()
: Arrays.stream(row.get("relation").split(",")).map(treeNode -> new TreeRelation("",
LdesFragmentIdentifier.fromFragmentId(treeNode), "", "", GENERIC_TREE_RELATION))
.collect(Collectors.toList()),
null);
}

@Given("a view with the following properties")
public void aViewWithTheFollowingProperties(ViewSpecification viewSpecification) {
applicationEventPublisher.publishEvent(new ViewAddedEvent(viewSpecification));
}

@And("the following Fragments are available")
public void theFollowingFragmentsAreAvailable(List<Fragment> fragments) {
fragments.forEach(fragment -> {
when(fragmentRepository.retrieveFragment(fragment.getFragmentId())).thenReturn(Optional.of(fragment));
when(fragmentRepository.retrieveFragment(fragment.getFragmentId())).thenReturn(Optional.of(fragment));
if (fragment.getFragmentPairs().isEmpty()) {
when(fragmentRepository.retrieveRootFragment(fragment.getViewName().asString()))
.thenReturn(Optional.of(fragment));
}
fragment.getRelations()
.forEach(treeRelation -> when(
fragmentRepository.retrieveFragmentsByOutgoingRelation(treeRelation.treeNode()))
.thenReturn(List.of(fragment)));
});
}

@And("the following allocations are present")
public void theFollowingAllocationsArePresent(List<FragmentAllocations> fragmentAllocations) {
when(allocationRepository.getPossibleCompactionCandidates(any(ViewName.class), anyInt()))
.thenAnswer(i ->
getAllocationAggregates(fragmentAllocations,
i.getArgument(0, ViewName.class),
i.getArgument(1, Integer.class))
);

when(allocationRepository.getMemberAllocationIdsByFragmentIds(ArgumentMatchers.any()))
.thenAnswer(x -> {
Set<String> requested = x.getArgument(0);
return fragmentAllocations.stream()
.filter(fragmentAllocation -> requested.contains(fragmentAllocation.fragmentId))
.flatMap(fragmentAllocations1 -> fragmentAllocations1.memberAllocations.stream()
.map(MemberAllocation::memberId)).toList();
});
}

private Stream<CompactionCandidate> getAllocationAggregates(List<FragmentAllocations> fragmentAllocations, ViewName viewName, Integer viewCapacity) {
return fragmentAllocations.stream()
.filter(fragmentAllocation -> {
var fragmentId = LdesFragmentIdentifier.fromFragmentId(fragmentAllocation.fragmentId);
return fragmentId.getViewName().equals(viewName);
})
.map(fragmentAllocation -> new CompactionCandidate(fragmentAllocation.fragmentId, fragmentAllocation.memberAllocations.size()));
}

@And("verify creation of the following fragments")
public void verifyCreationOfTheFollowingFragments(List<String> createdFragments) {
createdFragments.forEach(createdFragment -> verify(fragmentRepository)
.saveFragment(new Fragment(LdesFragmentIdentifier.fromFragmentId(createdFragment))));
}

@And("verify update of predecessor relations")
public void verifyUpdateOfPredecessorRelations(List<String> predecessorFragments) {
predecessorFragments.forEach(predecessorFragment -> {
verify(fragmentRepository)
.saveFragment(new Fragment(LdesFragmentIdentifier.fromFragmentId(predecessorFragment)));

List<TreeRelation> treeRelations = fragmentRepository
.retrieveFragment(LdesFragmentIdentifier.fromFragmentId(predecessorFragment))
.orElseThrow()
.getRelations();
assertThat(treeRelations)
.map(TreeRelation::treeNode)
.map(LdesFragmentIdentifier::asDecodedFragmentId)
.filteredOn(identifier -> !identifier.contains("dummy"))
.hasSize(1);
});
}

@And("verify fragmentation of members")
public void verifyFragmentationOfMembers(List<MemberFragmentations> memberFragmentations) {
verify(eventConsumer, times(memberFragmentations.size())).consumeEvent(any(BulkMemberAllocatedEvent.class));
memberFragmentations.forEach(memberFragmentation -> {
verify(fragmentRepository).incrementNrOfMembersAdded(LdesFragmentIdentifier.fromFragmentId(memberFragmentation.fragmentId), memberFragmentation.memberIds.size());
});
}

@Then("wait for {int} seconds until compaction has executed at least once")
public void waitForSecondsUntilCompactionHasExecutedAtLeastOnce(int secondsToWait) {
await()
.timeout(secondsToWait + 1, SECONDS)
.pollDelay(secondsToWait, SECONDS)
.untilAsserted(() -> assertThat(true).isTrue());
}

public record FragmentAllocations(String fragmentId, List<MemberAllocation> memberAllocations) {
}

public record MemberFragmentations(String fragmentId, List<String> memberIds) {
}
@DataTableType
public ViewSpecification ViewSpecificationEntryTransformer(Map<String, String> row) {
return new ViewSpecification(
ViewName.fromString(row.get("viewName")),
List.of(), List.of(), Integer.parseInt(row.get("pageSize")));
}

@DataTableType
public FragmentAllocations FragmentAllocationsListEntryTransformer(Map<String, String> row) {
List<MemberAllocation> memberAllocations = new ArrayList<>();
String fragmentIdentifier = row.get("fragmentIdentifier");
for (String memberId : row.get("members").split(",")) {
memberAllocations.add(new MemberAllocation(fragmentIdentifier + "/" + memberId, "mobility-hindrances",
"by-page", fragmentIdentifier, memberId));
}
return new FragmentAllocations(fragmentIdentifier, memberAllocations);
}

@DataTableType
public MemberFragmentations MemberFragmentationsEntryTransformer(Map<String, String> row) {
return new MemberFragmentations(row.get("fragmentId"), Arrays.stream(row.get("memberIds").split(",")).toList());
}

@DataTableType(replaceWithEmptyString = "[blank]")
public Fragment FragmentEntryTransformer(Map<String, String> row) {
final ArrayList<TreeRelation> relations = row.get("relation").isEmpty() ? new ArrayList<>()
: Arrays.stream(row.get("relation").split(",")).map(treeNode -> new TreeRelation("",
LdesFragmentIdentifier.fromFragmentId(treeNode), "", "", GENERIC_TREE_RELATION))
.collect(Collectors.toCollection(ArrayList::new));
return new Fragment(
LdesFragmentIdentifier.fromFragmentId(row.get("fragmentIdentifier")),
Boolean.parseBoolean(row.get("immutable")), Integer.parseInt(row.get("nrOfMembersAdded")),
relations,
null);
}

@Given("a view with the following properties")
public void aViewWithTheFollowingProperties(ViewSpecification viewSpecification) {
applicationEventPublisher.publishEvent(new ViewAddedEvent(viewSpecification));
}

@And("the following Fragments are available")
public void theFollowingFragmentsAreAvailable(List<Fragment> fragments) {
fragments.forEach(fragment -> {
when(fragmentRepository.retrieveFragment(fragment.getFragmentId())).thenReturn(Optional.of(fragment));
when(fragmentRepository.retrieveFragment(fragment.getFragmentId())).thenReturn(Optional.of(fragment));
if (fragment.getFragmentPairs().isEmpty()) {
when(fragmentRepository.retrieveRootFragment(fragment.getViewName().asString()))
.thenReturn(Optional.of(fragment));
}
fragment.getRelations()
.forEach(treeRelation -> when(
fragmentRepository.retrieveFragmentsByOutgoingRelation(treeRelation.treeNode()))
.thenReturn(List.of(fragment)));
});
}

@And("the following allocations are present")
public void theFollowingAllocationsArePresent(List<FragmentAllocations> fragmentAllocations) {
when(allocationRepository.getPossibleCompactionCandidates(any(ViewName.class), anyInt()))
.thenAnswer(i ->
getAllocationAggregates(fragmentAllocations, i.getArgument(0, ViewName.class))
);

when(allocationRepository.getMemberAllocationIdsByFragmentIds(ArgumentMatchers.any()))
.thenAnswer(x -> {
Set<String> requested = x.getArgument(0);
return fragmentAllocations.stream()
.filter(fragmentAllocation -> requested.contains(fragmentAllocation.fragmentId))
.flatMap(fragmentAllocations1 -> fragmentAllocations1.memberAllocations.stream()
.map(MemberAllocation::memberId)).toList();
});
}

private Stream<CompactionCandidate> getAllocationAggregates(List<FragmentAllocations> fragmentAllocations, ViewName viewName) {
return fragmentAllocations.stream()
.filter(fragmentAllocation -> {
var fragmentId = LdesFragmentIdentifier.fromFragmentId(fragmentAllocation.fragmentId);
return fragmentId.getViewName().equals(viewName);
})
.map(fragmentAllocation -> new CompactionCandidate(fragmentAllocation.fragmentId, fragmentAllocation.memberAllocations.size()));
}

@And("verify creation of the following fragments")
public void verifyCreationOfTheFollowingFragments(List<String> createdFragments) {
createdFragments.forEach(createdFragment -> verify(fragmentRepository)
.saveFragment(new Fragment(LdesFragmentIdentifier.fromFragmentId(createdFragment))));
}

@And("verify update of predecessor relations")
public void verifyUpdateOfPredecessorRelations(List<String> predecessorFragments) {
predecessorFragments.forEach(predecessorFragment -> {
verify(fragmentRepository)
.saveFragment(new Fragment(LdesFragmentIdentifier.fromFragmentId(predecessorFragment)));

List<TreeRelation> treeRelations = fragmentRepository
.retrieveFragment(LdesFragmentIdentifier.fromFragmentId(predecessorFragment))
.orElseThrow()
.getRelations();
assertThat(treeRelations)
.map(TreeRelation::treeNode)
.map(LdesFragmentIdentifier::asDecodedFragmentId)
.filteredOn(identifier -> !identifier.contains("dummy"))
.hasSize(1);
});
}

@And("verify fragmentation of members")
public void verifyFragmentationOfMembers(List<MemberFragmentations> memberFragmentations) {
verify(eventConsumer, times(memberFragmentations.size())).consumeEvent(any(BulkMemberAllocatedEvent.class));
memberFragmentations.forEach(memberFragmentation -> {
verify(fragmentRepository).incrementNrOfMembersAdded(LdesFragmentIdentifier.fromFragmentId(memberFragmentation.fragmentId), memberFragmentation.memberIds.size());
});
}

@Then("wait for {int} seconds until compaction has executed at least once")
public void waitForSecondsUntilCompactionHasExecutedAtLeastOnce(int secondsToWait) {
await()
.timeout(secondsToWait + 1, SECONDS)
.pollDelay(secondsToWait, SECONDS)
.untilAsserted(() -> assertThat(true).isTrue());
}

public record FragmentAllocations(String fragmentId, List<MemberAllocation> memberAllocations) {
}

public record MemberFragmentations(String fragmentId, List<String> memberIds) {
}

}
6 changes: 3 additions & 3 deletions ...ava/be/vlaanderen/informatievlaanderen/ldes/server/domain/rest/PrefixConstructorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class PrefixConstructorTest {
private PrefixConstructor prefixConstructor;

@Test
void when_NotUsingRelativeUrls_Then_GetHostname() throws Exception {
void when_NotUsingRelativeUrls_Then_GetHostname() {
prefixConstructor = new PrefixConstructor(hostname, false);
MockHttpServletRequest request = new MockHttpServletRequest();
RequestContextHolder.setRequestAttributes(new ServletRequestAttributes(request));
Expand All @@ -32,7 +32,7 @@ void when_NotUsingRelativeUrls_Then_GetHostname() throws Exception {
}
@ParameterizedTest(name = "Request with URI {0} returns {1}")
@ArgumentsSource(RequestUriArgumentsProvider.class)
void when_UsingRelativeUrls_Then_GetCorrectPrefix(String requestUri, String expected) throws Exception {
void when_UsingRelativeUrls_Then_GetCorrectPrefix(String requestUri, String expected) {
prefixConstructor = new PrefixConstructor(hostname, true);
MockHttpServletRequest request = new MockHttpServletRequest();
RequestContextHolder.setRequestAttributes(new ServletRequestAttributes(request));
Expand All @@ -53,5 +53,5 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext extensionCo
Arguments.of("", ""),
Arguments.of("test/testing/testing/testing", "../../../.."));
}
};
}
}
Loading

0 comments on commit 19b63b5

Please sign in to comment.