Merge pull request #2750 from Micheleboychuk/DS-4487

This is an alternative PR for DS-4487 that also include test to proof the issue and solution
This commit is contained in:
Tim Donohue
2020-04-21 10:01:17 -05:00
committed by GitHub
2 changed files with 61 additions and 4 deletions

View File

@@ -8,6 +8,7 @@
package org.dspace.app.rest.converter;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.List;
import org.apache.logging.log4j.LogManager;
@@ -78,23 +79,24 @@ public abstract class DSpaceObjectConverter<M extends DSpaceObject, R extends or
*/
public MetadataValueList getPermissionFilteredMetadata(Context context, M obj) {
List<MetadataValue> metadata = obj.getMetadata();
List<MetadataValue> visibleMetadata = new ArrayList<MetadataValue>();
try {
if (context != null && authorizeService.isAdmin(context)) {
return new MetadataValueList(metadata);
}
for (MetadataValue mv : metadata) {
MetadataField metadataField = mv.getMetadataField();
if (metadataExposureService
if (!metadataExposureService
.isHidden(context, metadataField.getMetadataSchema().getName(),
metadataField.getElement(),
metadataField.getQualifier())) {
metadata.remove(mv);
visibleMetadata.add(mv);
}
}
} catch (SQLException e) {
log.error("Error filtering metadata based on permissions", e);
}
return new MetadataValueList(metadata);
return new MetadataValueList(visibleMetadata);
}
/**

View File

@@ -1293,15 +1293,17 @@ public class CollectionRestRepositoryIT extends AbstractControllerIntegrationTes
}
@Test
public void testHiddenMetadataForAonymousUser() throws Exception {
public void testHiddenMetadataForAnonymousUser() throws Exception {
context.turnOffAuthorisationSystem();
parentCommunity = CommunityBuilder.createCommunity(context)
.withName("Parent Community")
.build();
// use multiple metadata to hit the scenario of the bug DS-4487 related to concurrent modification
Collection col1 = CollectionBuilder.createCollection(context, parentCommunity)
.withName("Collection 1")
.withProvenance("Provenance Data")
.withNameForLanguage("Col 1", "en")
.build();
context.restoreAuthSystemState();
@@ -1325,9 +1327,11 @@ public class CollectionRestRepositoryIT extends AbstractControllerIntegrationTes
parentCommunity = CommunityBuilder.createCommunity(context)
.withName("Parent Community")
.build();
// use multiple metadata to hit the scenario of the bug DS-4487 related to concurrent modification
Collection col1 = CollectionBuilder.createCollection(context, parentCommunity)
.withName("Collection 1")
.withProvenance("Provenance Data")
.withNameForLanguage("Col 1", "en")
.build();
@@ -1353,9 +1357,11 @@ public class CollectionRestRepositoryIT extends AbstractControllerIntegrationTes
parentCommunity = CommunityBuilder.createCommunity(context)
.withName("Parent Community")
.build();
// use multiple metadata to hit the scenario of the bug DS-4487 related to concurrent modification
Collection col1 = CollectionBuilder.createCollection(context, parentCommunity)
.withName("Collection 1")
.withProvenance("Provenance Data")
.withNameForLanguage("Col 1", "en")
.build();
@@ -1378,4 +1384,53 @@ public class CollectionRestRepositoryIT extends AbstractControllerIntegrationTes
.andExpect(jsonPath("$.metadata.['dc.description.provenance']").doesNotExist());
}
@Test
public void findAllWithHiddenMetadataTest() throws Exception {
context.turnOffAuthorisationSystem();
parentCommunity = CommunityBuilder.createCommunity(context)
.withName("Parent Community")
.build();
// use multiple metadata to hit the scenario of the bug DS-4487 related to concurrent modification
Collection col1 = CollectionBuilder.createCollection(context, parentCommunity)
.withName("Collection 1")
.withProvenance("Provenance Test 1")
.withNameForLanguage("col1", "en")
.build();
Collection col2 = CollectionBuilder.createCollection(context, parentCommunity)
.withName("Collection 2")
.withProvenance("Provenance Test 2")
.withNameForLanguage("col2", "it")
.build();
context.restoreAuthSystemState();
String tokenEPerson = getAuthToken(eperson.getEmail(), password);
getClient().perform(get("/api/core/collections")
.param("embed", CollectionMatcher.getEmbedsParameter()))
.andExpect(status().isOk())
.andExpect(content().contentType(contentType))
.andExpect(jsonPath("$._embedded.collections", Matchers.containsInAnyOrder(
CollectionMatcher.matchCollectionEntrySpecificEmbedProjection(col1.getName(), col1.getID(),
col1.getHandle()),
CollectionMatcher.matchCollectionEntrySpecificEmbedProjection(col2.getName(), col2.getID(),
col2.getHandle())
)))
.andExpect(jsonPath("$.metadata.['dc.description.provenance']").doesNotExist());
getClient(tokenEPerson).perform(get("/api/core/collections")
.param("embed", CollectionMatcher.getEmbedsParameter()))
.andExpect(status().isOk())
.andExpect(content().contentType(contentType))
.andExpect(jsonPath("$._embedded.collections", Matchers.containsInAnyOrder(
CollectionMatcher.matchCollectionEntrySpecificEmbedProjection(col1.getName(), col1.getID(),
col1.getHandle()),
CollectionMatcher.matchCollectionEntrySpecificEmbedProjection(col2.getName(), col2.getID(),
col2.getHandle())
)))
.andExpect(jsonPath("$.metadata.['dc.description.provenance']").doesNotExist());
}
}