Skip to content

Commit

Permalink
Adopt status codes due to issue kit-data-manager#403.
Browse files Browse the repository at this point in the history
  • Loading branch information
@VolkerHartmann
VolkerHartmann committed Dec 6, 2023
1 parent b7c613b commit 5112c38
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 29 deletions.
21 changes: 14 additions & 7 deletions src/main/java/edu/kit/datamanager/metastore2/util/MetadataRecordUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -791,9 +791,16 @@ public static MetadataRecord getRecordByIdAndVersion(MetastoreConfiguration meta
String recordId, Long version, boolean supportEtag) throws ResourceNotFoundException {
//if security enabled, check permission -> if not matching, return HTTP UNAUTHORIZED or FORBIDDEN
long nano = System.nanoTime() / 1000000;
long nano2;
MetadataRecord result = null;
Page<DataResource> dataResource = metastoreProperties.getDataResourceService().findAllVersions(recordId, null);
long nano2 = System.nanoTime() / 1000000;
Page<DataResource> dataResource;
try {
dataResource = metastoreProperties.getDataResourceService().findAllVersions(recordId, null);
} catch (ResourceNotFoundException ex) {
ex.setDetail("Metadata document with ID '" + recordId + "' doesn't exist!");
throw ex;
}
nano2 = System.nanoTime() / 1000000;
Stream<DataResource> stream = dataResource.get();
if (version != null) {
stream = stream.filter(resource -> Long.parseLong(resource.getVersion()) == version);
Expand All @@ -802,9 +809,9 @@ public static MetadataRecord getRecordByIdAndVersion(MetastoreConfiguration meta
if (findFirst.isPresent()) {
result = migrateToMetadataRecord(metastoreProperties, findFirst.get(), supportEtag);
} else {
String message = String.format("ID '%s' or version '%d' doesn't exist!", recordId, version);
String message = String.format("Version '%d' of ID '%s' doesn't exist!", version, recordId);
LOG.error(message);
throw new BadArgumentException(message);
throw new ResourceNotFoundException(message);
}
long nano3 = System.nanoTime() / 1000000;
LOG.info("getRecordByIdAndVersion {}, {}, {}", nano, (nano2 - nano), (nano3 - nano));
Expand Down Expand Up @@ -879,7 +886,7 @@ public static Set<AclEntry> mergeAcl(Set<AclEntry> managed, Set<AclEntry> provid
LOG.trace("Updating record acl from {} to {}.", managed, provided);
managed = provided;
} else {
LOG.trace("Provided ACL is still the same -> Continue using old one.");
LOG.trace("Provided ACL is still the same -> Continue using old one.");
}
} else {
LOG.trace("Provided ACL is empty -> Continue using old one.");
Expand Down Expand Up @@ -909,8 +916,8 @@ public static <T> T mergeEntry(String description, T managed, T provided) {
* @return Merged record
*/
public static <T> T mergeEntry(String description, T managed, T provided, boolean overwriteWithNull) {
if ((provided != null && !provided.equals(managed)) ||
overwriteWithNull) {
if ((provided != null && !provided.equals(managed))
|| overwriteWithNull) {
LOG.trace(description + " from '{}' to '{}'", managed, provided);
managed = provided;
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/edu/kit/datamanager/metastore2/web/impl/MetadataControllerImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ public ResponseEntity<List<MetadataRecord>> getRecords(
currentSchemaRecord.setSchemaDocumentUri(schemaIdentifier.getIdentifier());
}
allRelatedIdentifiersSchema.add(currentSchemaRecord.getSchemaDocumentUri());
} catch (ResourceNotFoundException rnfe) {
} catch (Exception rnfe) {
// schemaID not found set version to 1
currentSchemaRecord = new MetadataSchemaRecord();
currentSchemaRecord.setSchemaVersion(1l);
Expand Down
87 changes: 66 additions & 21 deletions src/test/java/edu/kit/datamanager/metastore2/test/MetadataControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -810,13 +810,24 @@ public void testGetRecordByIdWithVersion() throws Exception {
@Test
public void testGetRecordByIdWithInvalidId() throws Exception {
String metadataRecordId = createDCMetadataRecord();
this.mockMvc.perform(get("/api/v1/metadata/cd").header("Accept", MetadataRecord.METADATA_RECORD_MEDIA_TYPE)).andDo(print()).andExpect(status().isNotFound()).andReturn();
MvcResult result = this.mockMvc.perform(get("/api/v1/metadata/cd").
header("Accept", MetadataRecord.METADATA_RECORD_MEDIA_TYPE)).
andDo(print()).
andExpect(status().isNotFound()).
andReturn();
Assert.assertTrue("Try to access invalid id!", result.getResponse().getContentAsString().contains("Metadata document with ID 'cd' doesn't exist!"));
}

@Test
public void testGetRecordByIdWithInvalidVersion() throws Exception {
String metadataRecordId = createDCMetadataRecord();
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "13").header("Accept", MetadataRecord.METADATA_RECORD_MEDIA_TYPE)).andDo(print()).andExpect(status().is4xxClientError()).andReturn();
MvcResult result = this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).
param("version", "13").
header("Accept", MetadataRecord.METADATA_RECORD_MEDIA_TYPE)).
andDo(print()).
andExpect(status().isNotFound()).
andReturn();
Assert.assertTrue("Try to access invalid version!", result.getResponse().getContentAsString().contains("Version '13' of ID '" + metadataRecordId + "' doesn't exist!"));
}

@Test
Expand All @@ -835,8 +846,12 @@ public void testFindRecordsByInvalidSchemaId() throws Exception {
MvcResult res = this.mockMvc.perform(get("/api/v1/metadata").
param("schemaId", "anyinvalidschemaid")).
andDo(print()).
andExpect(status().is4xxClientError()).
andExpect(status().isOk()).
andReturn();
ObjectMapper map = new ObjectMapper();
MetadataRecord[] result = map.readValue(res.getResponse().getContentAsString(), MetadataRecord[].class);

Assert.assertEquals(0, result.length);
}

@Test
Expand Down Expand Up @@ -893,15 +908,15 @@ public void testFindRecordsBySchemaIdANDResourceId() throws Exception {
String metadataRecordId2 = createDCMetadataRecordWithRelatedResource(relatedResource2, SCHEMA_ID);
String metadataRecordIdv2 = createDCMetadataRecordWithRelatedResource(relatedResource, secondSchemaId);
String metadataRecordId2v2 = createDCMetadataRecordWithRelatedResource(relatedResource2, secondSchemaId);

// Looking for first schema
MvcResult res = this.mockMvc.perform(get("/api/v1/metadata").
param("schemaId", SCHEMA_ID)).
andDo(print()).
andExpect(status().isOk()).
andReturn();
ObjectMapper map = new ObjectMapper();
MetadataRecord[] result = map.readValue(res.getResponse().getContentAsString(), MetadataRecord[].class);

// Looking for second schema
Assert.assertEquals(2, result.length);
res = this.mockMvc.perform(get("/api/v1/metadata").
param("schemaId", secondSchemaId)).
Expand All @@ -910,7 +925,7 @@ public void testFindRecordsBySchemaIdANDResourceId() throws Exception {
andReturn();
result = map.readValue(res.getResponse().getContentAsString(), MetadataRecord[].class);
Assert.assertEquals(2, result.length);

// Looking for first AND second schema
res = this.mockMvc.perform(get("/api/v1/metadata").
param("schemaId", SCHEMA_ID).
param("schemaId", secondSchemaId)).
Expand All @@ -919,6 +934,28 @@ public void testFindRecordsBySchemaIdANDResourceId() throws Exception {
andReturn();
result = map.readValue(res.getResponse().getContentAsString(), MetadataRecord[].class);
Assert.assertEquals(4, result.length);
// Looking for first, second AND invalid schema
res = this.mockMvc.perform(get("/api/v1/metadata").
param("schemaId", SCHEMA_ID).
param("schemaId", secondSchemaId).
param("schemaId", "invalidschemaid")).
andDo(print()).
andExpect(status().isOk()).
andReturn();
result = map.readValue(res.getResponse().getContentAsString(), MetadataRecord[].class);
Assert.assertEquals(4, result.length);
// Looking for first, second AND invalid schema AND resource1 and resource2
res = this.mockMvc.perform(get("/api/v1/metadata").
param("schemaId", SCHEMA_ID).
param("schemaId", secondSchemaId).
param("schemaId", "invalidschemaid").
param("resourceId", relatedResource).
param("resourceId", relatedResource2)).
andDo(print()).
andExpect(status().isOk()).
andReturn();
result = map.readValue(res.getResponse().getContentAsString(), MetadataRecord[].class);
Assert.assertEquals(4, result.length);

res = this.mockMvc.perform(get("/api/v1/metadata").
param("schemaId", SCHEMA_ID).
Expand Down Expand Up @@ -1009,7 +1046,15 @@ public void testFindRecordsByInvalidUploadDate() throws Exception {
@Test
public void testFindRecordsByUnknownParameter() throws Exception {
String metadataRecordId = createDCMetadataRecord();
this.mockMvc.perform(get("/api/v1/metadata").param("schemaId", "cd")).andDo(print()).andExpect(status().isUnprocessableEntity()).andReturn();
MvcResult res = this.mockMvc.perform(get("/api/v1/metadata").
param("schemaId", "cd")).
andDo(print()).
andExpect(status().isOk()).
andReturn();
ObjectMapper map = new ObjectMapper();
MetadataRecord[] result = map.readValue(res.getResponse().getContentAsString(), MetadataRecord[].class);

Assert.assertEquals(0, result.length);
}

@Test
Expand Down Expand Up @@ -1563,38 +1608,38 @@ public void testIssue52() throws Exception {
.andReturn();
Assert.assertTrue("Reference to " + RELATED_RESOURCE_STRING + " is not available", result.getResponse().getContentAsString().contains("\"" + RELATED_RESOURCE_STRING + "\""));
// check for higher versions which should be not available
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "2")).andDo(print()).andExpect(status().isBadRequest());
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "3")).andDo(print()).andExpect(status().isBadRequest());
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "4")).andDo(print()).andExpect(status().isBadRequest());
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "2")).andDo(print()).andExpect(status().isNotFound());
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "3")).andDo(print()).andExpect(status().isNotFound());
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "4")).andDo(print()).andExpect(status().isNotFound());

version++;
metadataRecordId = ingestNewMetadataRecord(metadataRecordId, version);
// Read all versions (should be still one version)
result = this.mockMvc.perform(get("/api/v1/metadata").param("id", metadataRecordId).header(HttpHeaders.ACCEPT, "application/json")).andDo(print()).andExpect(status().isOk()).andExpect(MockMvcResultMatchers.jsonPath("$", Matchers.hasSize((int) 1))).andReturn();
Assert.assertTrue("Reference to " + RELATED_RESOURCE_STRING + version + " is not available", result.getResponse().getContentAsString().contains("\"" + RELATED_RESOURCE_STRING + version + "\""));
// check for higher versions which should be not available
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "2")).andDo(print()).andExpect(status().isBadRequest());
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "3")).andDo(print()).andExpect(status().isBadRequest());
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "4")).andDo(print()).andExpect(status().isBadRequest());
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "2")).andDo(print()).andExpect(status().isNotFound());
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "3")).andDo(print()).andExpect(status().isNotFound());
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "4")).andDo(print()).andExpect(status().isNotFound());

version++;
metadataRecordId = ingestNewMetadataRecord(metadataRecordId, version);
// Read all versions (should be still one version)
result = this.mockMvc.perform(get("/api/v1/metadata").param("id", metadataRecordId).header(HttpHeaders.ACCEPT, "application/json")).andDo(print()).andExpect(status().isOk()).andExpect(MockMvcResultMatchers.jsonPath("$", Matchers.hasSize((int) 1))).andReturn();
Assert.assertTrue("Reference to " + RELATED_RESOURCE_STRING + version + " is not available", result.getResponse().getContentAsString().contains("\"" + RELATED_RESOURCE_STRING + version + "\""));
// check for higher versions which should be not available
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "2")).andDo(print()).andExpect(status().isBadRequest());
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "3")).andDo(print()).andExpect(status().isBadRequest());
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "4")).andDo(print()).andExpect(status().isBadRequest());
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "2")).andDo(print()).andExpect(status().isNotFound());
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "3")).andDo(print()).andExpect(status().isNotFound());
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "4")).andDo(print()).andExpect(status().isNotFound());

metadataRecordId = ingestMetadataRecordWithVersion(metadataRecordId, version);
// Read all versions (should be still one version)
result = this.mockMvc.perform(get("/api/v1/metadata").param("id", metadataRecordId).header(HttpHeaders.ACCEPT, "application/json")).andDo(print()).andExpect(status().isOk()).andExpect(MockMvcResultMatchers.jsonPath("$", Matchers.hasSize((int) 2))).andReturn();
Assert.assertTrue("Reference to " + RELATED_RESOURCE_STRING + version + " is not available", result.getResponse().getContentAsString().contains("\"" + RELATED_RESOURCE_STRING + version + "\""));
// check for higher versions which should be not available (if version > 2)
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "2")).andDo(print()).andExpect(status().isOk());
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "3")).andDo(print()).andExpect(status().isBadRequest());
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "4")).andDo(print()).andExpect(status().isBadRequest());
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "3")).andDo(print()).andExpect(status().isNotFound());
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "4")).andDo(print()).andExpect(status().isNotFound());

version++;
metadataRecordId = ingestNewMetadataRecord(metadataRecordId, version);
Expand All @@ -1614,8 +1659,8 @@ public void testIssue52() throws Exception {
Assert.assertNotEquals(dcMetadata, content);
Assert.assertEquals("Length must differ!", dcMetadata.length() + 3, content.length());
// check for higher versions which should be not available (if version > 2)
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "3")).andDo(print()).andExpect(status().isBadRequest());
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "4")).andDo(print()).andExpect(status().isBadRequest());
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "3")).andDo(print()).andExpect(status().isNotFound());
this.mockMvc.perform(get("/api/v1/metadata/" + metadataRecordId).param("version", "4")).andDo(print()).andExpect(status().isNotFound());
}

@Test
Expand Down Expand Up @@ -1697,7 +1742,7 @@ public void testLandingPage4MetadataWrongVersion() throws Exception {
this.mockMvc.perform(get(redirectedUrl)
.accept("text/html"))
.andDo(print())
.andExpect(status().isBadRequest());
.andExpect(status().isNotFound());
}

@Test
Expand Down

0 comments on commit 5112c38

Please sign in to comment.