Skip to content

Commit 4893d66

Browse files
authored
Fix FDP v2 index API (#637)
* replace `modificationTime`, `registrationTime`, and `astRetrievalTime` by `updatedAt`, `createdAt`, and `lastRetrievalAt`, respectively. * replace setCreatedAt by setLastRetrievalAt before save * add tests to reproduce issues #633, #643, #644 * set lastRetrievalAt for newIndexEntry test fixture * sort by 'createdAt' instead of 'created' (fixes #643) * add parameterized types in entry/Detail_GET * add IndexEntry.events mapped to IndexEvent.relatedTo with cascade all (fixes #644) * implement sort parameter validation for /index/entries endpoint based on an explicit list of allowed fields IndexEntryController.sortFields * update changelog
1 parent fc751c4 commit 4893d66

File tree

13 files changed

+247
-10
lines changed

13 files changed

+247
-10
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
1717
- Separate and reuse github workflows by @dennisvang in #617
1818
- Prevent duplicate workflow runs by @dennisvang in #628
1919
- Clean up Dockerfile by @dennisvang in #626
20+
- Fix index API by @dennisvang in #637 (backward incompatible)
2021

2122
## [1.17.2]
2223

src/main/java/org/fairdatapoint/api/controller/exception/ExceptionControllerAdvice.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,5 +242,4 @@ private ErrorDTO handleInvalidSparqlQuery(Exception exception) {
242242
);
243243
return new ErrorDTO(HttpStatus.BAD_REQUEST, message, details);
244244
}
245-
246245
}

src/main/java/org/fairdatapoint/api/controller/index/IndexEntryController.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import org.springframework.http.MediaType;
4545
import org.springframework.security.access.prepost.PreAuthorize;
4646
import org.springframework.web.bind.annotation.*;
47+
import org.springframework.web.server.ResponseStatusException;
4748

4849
import java.util.List;
4950
import java.util.Optional;
@@ -54,6 +55,9 @@
5455
@RequestMapping("/index/entries")
5556
@RequiredArgsConstructor
5657
public class IndexEntryController {
58+
private static final List<String> SORT_FIELDS = List.of(
59+
"clientUrl", "createdAt", "lastRetrievalAt", "permit", "status", "updatedAt"
60+
);
5761

5862
private final IndexEntryService service;
5963

@@ -71,6 +75,14 @@ public Page<IndexEntryDTO> getEntriesPage(
7175
@RequestParam(required = false, defaultValue = "") String state,
7276
@RequestParam(required = false, defaultValue = "accepted") String permit
7377
) {
78+
// validate sort query parameters
79+
// todo: implement validator for Pageable if used more often
80+
pageable.getSort().stream().forEach(order -> {
81+
if (!SORT_FIELDS.contains(order.getProperty())) {
82+
throw new ResponseStatusException(
83+
HttpStatus.UNPROCESSABLE_ENTITY, "Invalid sort parameter: " + order.getProperty());
84+
}
85+
});
7486
return service.getEntriesPageDTOs(pageable, state, permit);
7587
}
7688

src/main/java/org/fairdatapoint/api/dto/index/entry/IndexEntryDTO.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ public class IndexEntryDTO {
5252
private IndexEntryPermit permit;
5353

5454
@NotNull
55-
private String registrationTime;
55+
private String createdAt;
5656

5757
@NotNull
58-
private String modificationTime;
58+
private String updatedAt;
5959
}

src/main/java/org/fairdatapoint/api/dto/index/entry/IndexEntryDetailDTO.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,12 @@ public class IndexEntryDetailDTO {
6161
private List<EventDTO> events;
6262

6363
@NotNull
64-
private String registrationTime;
64+
private String createdAt;
6565

6666
@NotNull
67-
private String modificationTime;
67+
private String updatedAt;
6868

6969
@NotNull
70-
private String lastRetrievalTime;
70+
private String lastRetrievalAt;
7171

7272
}

src/main/java/org/fairdatapoint/entity/index/entry/IndexEntry.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,17 @@
2828
import lombok.*;
2929
import lombok.experimental.SuperBuilder;
3030
import org.fairdatapoint.entity.base.BaseEntity;
31+
import org.fairdatapoint.entity.index.event.IndexEvent;
3132
import org.hibernate.annotations.JdbcType;
3233
import org.hibernate.annotations.Type;
3334
import org.hibernate.dialect.PostgreSQLEnumJdbcType;
3435

3536
import java.time.Duration;
3637
import java.time.Instant;
3738
import java.util.HashMap;
39+
import java.util.HashSet;
3840
import java.util.Map;
41+
import java.util.Set;
3942

4043
@Entity(name = "IndexEntry")
4144
@Table(name = "index_entry")
@@ -79,6 +82,9 @@ public class IndexEntry extends BaseEntity {
7982
@Column(name = "metadata", columnDefinition = "jsonb", nullable = false)
8083
private Map<String, String> metadata = new HashMap<>();
8184

85+
@OneToMany(mappedBy = "relatedTo", orphanRemoval = true, cascade = CascadeType.ALL, fetch = FetchType.LAZY)
86+
private Set<IndexEvent> events = new HashSet<>();
87+
8288
public Duration getLastRetrievalAgo() {
8389
if (getLastRetrievalAt() == null) {
8490
return null;

src/main/java/org/fairdatapoint/service/index/entry/IndexEntryMapper.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ public IndexEntryDTO toDTO(IndexEntry indexEntry, Instant validThreshold) {
5050
indexEntry.getLastRetrievalAt(),
5151
validThreshold),
5252
indexEntry.getPermit(),
53+
// convert to ISO 8601
5354
indexEntry.getCreatedAt().toString(),
5455
indexEntry.getUpdatedAt().toString()
5556
);
@@ -69,18 +70,19 @@ public IndexEntryDetailDTO toDetailDTO(
6970
StreamSupport.stream(events.spliterator(), false)
7071
.map(eventMapper::toDTO)
7172
.toList(),
73+
// convert to ISO 8601
7274
indexEntry.getCreatedAt().toString(),
7375
indexEntry.getUpdatedAt().toString(),
7476
indexEntry.getLastRetrievalAt().toString()
7577
);
7678
}
7779

7880
public IndexEntryStateDTO toStateDTO(
79-
IndexEntryState state, Instant lastRetrievalTime, Instant validThreshold
81+
IndexEntryState state, Instant lastRetrievalAt, Instant validThreshold
8082
) {
8183
return switch (state) {
8284
case UNKNOWN -> IndexEntryStateDTO.UNKNOWN;
83-
case VALID -> lastRetrievalTime.isAfter(validThreshold)
85+
case VALID -> lastRetrievalAt.isAfter(validThreshold)
8486
? IndexEntryStateDTO.ACTIVE
8587
: IndexEntryStateDTO.INACTIVE;
8688
case INVALID -> IndexEntryStateDTO.INVALID;

src/main/java/org/fairdatapoint/service/index/entry/IndexEntryService.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ private Iterable<IndexEvent> getEvents(UUID indexEntryUuid) {
190190

191191
public Iterable<IndexEvent> getEvents(IndexEntry indexEntry) {
192192
return eventRepository.getAllByRelatedTo(indexEntry,
193-
PageRequest.of(0, PAGE_SIZE, Sort.by(Sort.Direction.DESC, "created")));
193+
PageRequest.of(0, PAGE_SIZE, Sort.by(Sort.Direction.DESC, "createdAt")));
194194
}
195195

196196
@RequiredEnabledIndexFeature

src/main/java/org/fairdatapoint/service/index/event/EventService.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@ private void processMetadataRetrieval(IndexEvent event) {
177177
event.getPayload().getMetadataRetrieval().setMetadata(metadata.get());
178178
event.getRelatedTo().setCurrentMetadata(metadata.get());
179179
event.getRelatedTo().setState(IndexEntryState.VALID);
180+
event.getRelatedTo().setLastRetrievalAt(Instant.now());
180181
log.info("Storing metadata for {}", clientUrl);
181182
indexEntryRepository.save(event.getRelatedTo());
182183
}
@@ -201,7 +202,6 @@ private void processMetadataRetrieval(IndexEvent event) {
201202
log.info("Rate limit reached for {} (skipping metadata retrieval)", clientUrl);
202203
event.getPayload().getMetadataRetrieval().setError("Rate limit reached (skipping)");
203204
}
204-
event.getRelatedTo().setCreatedAt(Instant.now());
205205
event.finish();
206206
final IndexEvent newEvent = eventRepository.save(event);
207207
indexEntryRepository.save(newEvent.getRelatedTo());
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
/**
2+
* The MIT License
3+
* Copyright © 2016-2024 FAIR Data Team
4+
*
5+
* Permission is hereby granted, free of charge, to any person obtaining a copy
6+
* of this software and associated documentation files (the "Software"), to deal
7+
* in the Software without restriction, including without limitation the rights
8+
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
9+
* copies of the Software, and to permit persons to whom the Software is
10+
* furnished to do so, subject to the following conditions:
11+
*
12+
* The above copyright notice and this permission notice shall be included in
13+
* all copies or substantial portions of the Software.
14+
*
15+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
16+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
17+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
18+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
19+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
20+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
21+
* THE SOFTWARE.
22+
*/
23+
package org.fairdatapoint.acceptance.index.entry;
24+
25+
import org.fairdatapoint.WebIntegrationTest;
26+
import org.fairdatapoint.api.dto.index.entry.IndexEntryDTO;
27+
import org.fairdatapoint.database.db.repository.IndexEntryRepository;
28+
import org.fairdatapoint.database.db.repository.IndexEventRepository;
29+
import org.fairdatapoint.entity.index.entry.IndexEntry;
30+
import org.fairdatapoint.entity.index.event.IndexEvent;
31+
import org.fairdatapoint.entity.index.event.IndexEventPayload;
32+
import org.fairdatapoint.entity.index.event.payload.AdminTrigger;
33+
import org.fairdatapoint.utils.CustomPageImpl;
34+
import org.fairdatapoint.utils.TestIndexEntryFixtures;
35+
import org.junit.jupiter.api.DisplayName;
36+
import org.junit.jupiter.api.Test;
37+
import org.springframework.beans.factory.annotation.Autowired;
38+
import org.springframework.core.ParameterizedTypeReference;
39+
import org.springframework.http.HttpHeaders;
40+
import org.springframework.http.HttpStatus;
41+
import org.springframework.http.RequestEntity;
42+
import org.springframework.http.ResponseEntity;
43+
44+
import java.net.URI;
45+
import java.util.UUID;
46+
47+
import static org.hamcrest.MatcherAssert.assertThat;
48+
import static org.hamcrest.core.Is.is;
49+
import static org.hamcrest.core.IsEqual.equalTo;
50+
51+
@DisplayName("DELETE /index/entries/{uuid}")
52+
public class Detail_DELETE extends WebIntegrationTest {
53+
@Autowired
54+
private IndexEntryRepository indexEntryRepository;
55+
56+
@Autowired
57+
private IndexEventRepository indexEventRepository;
58+
59+
private final ParameterizedTypeReference<CustomPageImpl<IndexEntryDTO>> responseType =
60+
new ParameterizedTypeReference<>() {};
61+
62+
private URI url(UUID uuid) {
63+
return URI.create("/index/entries/" + uuid);
64+
}
65+
66+
@Test
67+
@DisplayName("HTTP 204: delete detail with event")
68+
public void res204_deleteDetailWithEvent() {
69+
// test for issue #644
70+
71+
// GIVEN: prepare data
72+
indexEntryRepository.deleteAll();
73+
IndexEntry entry = TestIndexEntryFixtures.entryExample();
74+
indexEntryRepository.save(entry);
75+
indexEventRepository.deleteAll();
76+
// minimal event related to entry
77+
// todo: create TestIndexEventFixtures utility if used more often
78+
IndexEvent event = new IndexEvent(0, new AdminTrigger());
79+
event.setRelatedTo(entry);
80+
indexEventRepository.save(event);
81+
82+
// AND: prepare request
83+
RequestEntity<Void> request = RequestEntity
84+
.delete(url(entry.getUuid()))
85+
.header(HttpHeaders.AUTHORIZATION, ADMIN_TOKEN)
86+
.build();
87+
88+
// WHEN
89+
ResponseEntity<CustomPageImpl<IndexEntryDTO>> result = client.exchange(request, responseType);
90+
91+
// THEN
92+
assertThat("Correct response code", result.getStatusCode(), is(equalTo(HttpStatus.NO_CONTENT)));
93+
}
94+
}
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
/**
2+
* The MIT License
3+
* Copyright © 2016-2024 FAIR Data Team
4+
*
5+
* Permission is hereby granted, free of charge, to any person obtaining a copy
6+
* of this software and associated documentation files (the "Software"), to deal
7+
* in the Software without restriction, including without limitation the rights
8+
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
9+
* copies of the Software, and to permit persons to whom the Software is
10+
* furnished to do so, subject to the following conditions:
11+
*
12+
* The above copyright notice and this permission notice shall be included in
13+
* all copies or substantial portions of the Software.
14+
*
15+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
16+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
17+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
18+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
19+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
20+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
21+
* THE SOFTWARE.
22+
*/
23+
package org.fairdatapoint.acceptance.index.entry;
24+
25+
import org.fairdatapoint.WebIntegrationTest;
26+
import org.fairdatapoint.api.dto.index.entry.IndexEntryDTO;
27+
import org.fairdatapoint.database.db.repository.IndexEntryRepository;
28+
import org.fairdatapoint.entity.index.entry.IndexEntry;
29+
import org.fairdatapoint.utils.CustomPageImpl;
30+
import org.fairdatapoint.utils.TestIndexEntryFixtures;
31+
import org.junit.jupiter.api.DisplayName;
32+
import org.junit.jupiter.api.Test;
33+
import org.springframework.beans.factory.annotation.Autowired;
34+
import org.springframework.core.ParameterizedTypeReference;
35+
import org.springframework.http.HttpStatus;
36+
import org.springframework.http.MediaType;
37+
import org.springframework.http.RequestEntity;
38+
import org.springframework.http.ResponseEntity;
39+
40+
import java.net.URI;
41+
import java.util.UUID;
42+
43+
import static org.hamcrest.MatcherAssert.assertThat;
44+
import static org.hamcrest.core.Is.is;
45+
import static org.hamcrest.core.IsEqual.equalTo;
46+
47+
@DisplayName("GET /index/entries/{uuid}")
48+
public class Detail_GET extends WebIntegrationTest {
49+
@Autowired
50+
private IndexEntryRepository indexEntryRepository;
51+
52+
private final ParameterizedTypeReference<CustomPageImpl<IndexEntryDTO>> responseType =
53+
new ParameterizedTypeReference<>() {};
54+
55+
private URI url(UUID uuid) {
56+
return URI.create("/index/entries/" + uuid);
57+
}
58+
59+
@Test
60+
@DisplayName("HTTP 200: get detail")
61+
public void res200_getDetail() {
62+
// test for issue #643
63+
64+
// GIVEN: prepare data
65+
indexEntryRepository.deleteAll();
66+
IndexEntry entry = TestIndexEntryFixtures.entryExample();
67+
indexEntryRepository.save(entry);
68+
69+
// AND: prepare request
70+
RequestEntity<Void> request = RequestEntity
71+
.get(url(entry.getUuid()))
72+
.accept(MediaType.APPLICATION_JSON)
73+
.build();
74+
75+
// WHEN
76+
ResponseEntity<CustomPageImpl<IndexEntryDTO>> result = client.exchange(request, responseType);
77+
78+
// THEN
79+
assertThat("Correct response code is received", result.getStatusCode(), is(equalTo(HttpStatus.OK)));
80+
}
81+
}

src/test/java/org/fairdatapoint/acceptance/index/entry/List_GET.java

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@
3131
import org.fairdatapoint.utils.TestIndexEntryFixtures;
3232
import org.junit.jupiter.api.DisplayName;
3333
import org.junit.jupiter.api.Test;
34+
import org.junit.jupiter.params.ParameterizedTest;
35+
import org.junit.jupiter.params.provider.CsvSource;
3436
import org.springframework.beans.factory.annotation.Autowired;
3537
import org.springframework.core.ParameterizedTypeReference;
3638
import org.springframework.http.*;
@@ -77,6 +79,12 @@ private URI urlWithPermit(String permitQuery) {
7779
.build().toUri();
7880
}
7981

82+
private URI urlWithSorting(String sortQuery) {
83+
return UriComponentsBuilder.fromUri(url())
84+
.queryParam("sort", sortQuery)
85+
.build().toUri();
86+
}
87+
8088
@Test
8189
@DisplayName("HTTP 200: page empty")
8290
public void res200_pageEmpty() {
@@ -163,6 +171,39 @@ public void res200_pageFew() {
163171
}
164172
}
165173

174+
@ParameterizedTest
175+
@CsvSource({
176+
"'updatedAt,desc', OK", // existing field
177+
"'modificationTime,desc', UNPROCESSABLE_ENTITY" // non-existent field
178+
})
179+
@DisplayName("HTTP 200: page few (sorted)")
180+
public void res200_pageFewSorted(String sortQuery, String expectedStatus) {
181+
// Test for issue #633
182+
//
183+
// https://www.rfc-editor.org/rfc/rfc9110.html
184+
//
185+
// todo: The res200_ naming convention is a bit restrictive and does not cover
186+
// multiple cases. Should we rename to regression tests or something?
187+
188+
// GIVEN: prepare data
189+
indexEntryRepository.deleteAll();
190+
List<IndexEntry> entries = TestIndexEntryFixtures.entriesFew();
191+
indexEntryRepository.saveAll(entries);
192+
193+
// AND: prepare request
194+
RequestEntity<?> request = RequestEntity
195+
.get(urlWithSorting(sortQuery))
196+
.accept(MediaType.APPLICATION_JSON)
197+
.build();
198+
199+
// WHEN
200+
ResponseEntity<CustomPageImpl<IndexEntryDTO>> result = client.exchange(request, responseType);
201+
202+
// THEN
203+
assertThat("Correct response code is received",
204+
result.getStatusCode(), is(equalTo(HttpStatus.valueOf(expectedStatus))));
205+
}
206+
166207
@Test
167208
@DisplayName("HTTP 200: page many (middle)")
168209
public void res200_pageManyMiddle() {

0 commit comments

Comments
 (0)