diff --git a/dspace-api/src/main/java/org/dspace/content/clarin/ClarinUserMetadataServiceImpl.java b/dspace-api/src/main/java/org/dspace/content/clarin/ClarinUserMetadataServiceImpl.java index 4c3c3450b136..8704fc45a417 100644 --- a/dspace-api/src/main/java/org/dspace/content/clarin/ClarinUserMetadataServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/clarin/ClarinUserMetadataServiceImpl.java @@ -10,6 +10,8 @@ import java.sql.SQLException; import java.util.List; import java.util.Objects; +import java.util.UUID; +import java.util.stream.Collectors; import org.apache.commons.lang.NullArgumentException; import org.dspace.authorize.AuthorizeException; @@ -76,4 +78,44 @@ public void delete(Context context, ClarinUserMetadata clarinUserMetadata) throw } clarinUserMetadataDAO.delete(context, clarinUserMetadata); } + + @Override + public List findByUserRegistrationAndBitstream(Context context, Integer userRegUUID, + UUID bitstreamUUID, boolean lastTransaction) + throws SQLException { + if (lastTransaction) { + return getLastTransactionUserMetadata(clarinUserMetadataDAO.findByUserRegistrationAndBitstream(context, + userRegUUID, bitstreamUUID)); + } + return clarinUserMetadataDAO.findByUserRegistrationAndBitstream(context, userRegUUID, bitstreamUUID); + } + + private List getLastTransactionUserMetadata(List userMetadataList) { + Integer latestTransactionId = getIdOfLastTransaction(userMetadataList); + if (latestTransactionId == null) { + return userMetadataList; + } + + List filteredUserMetadata = null; + // Filter all user metadata by the last transaction + try { + filteredUserMetadata = userMetadataList.stream() + .filter(clarinUserMetadata -> clarinUserMetadata.getTransaction().getID() + .equals(latestTransactionId)) + .collect(Collectors.toList()); + } catch (Exception e) { + log.error("Error filtering user metadata by the last transaction", e); + } + return filteredUserMetadata; + } + + private Integer getIdOfLastTransaction(List userMetadataList) { + // userMetadataList is filtered by the last transaction - first element is the last transaction + try { + return userMetadataList.get(0).getTransaction().getID(); + } catch (IndexOutOfBoundsException e) { + log.error("No transaction found for the user metadata"); + return null; + } + } } diff --git a/dspace-api/src/main/java/org/dspace/content/dao/clarin/ClarinUserMetadataDAO.java b/dspace-api/src/main/java/org/dspace/content/dao/clarin/ClarinUserMetadataDAO.java index db18c605a4a7..c25b77435d11 100644 --- a/dspace-api/src/main/java/org/dspace/content/dao/clarin/ClarinUserMetadataDAO.java +++ b/dspace-api/src/main/java/org/dspace/content/dao/clarin/ClarinUserMetadataDAO.java @@ -7,8 +7,15 @@ */ package org.dspace.content.dao.clarin; +import java.sql.SQLException; +import java.util.List; +import java.util.UUID; + import org.dspace.content.clarin.ClarinUserMetadata; +import org.dspace.core.Context; import org.dspace.core.GenericDAO; public interface ClarinUserMetadataDAO extends GenericDAO { + List findByUserRegistrationAndBitstream(Context context, Integer userRegUUID, + UUID bitstreamUUID) throws SQLException; } diff --git a/dspace-api/src/main/java/org/dspace/content/dao/impl/clarin/ClarinUserMetadataDAOImpl.java b/dspace-api/src/main/java/org/dspace/content/dao/impl/clarin/ClarinUserMetadataDAOImpl.java index d0d45c11df0f..74fb5cee2ea6 100644 --- a/dspace-api/src/main/java/org/dspace/content/dao/impl/clarin/ClarinUserMetadataDAOImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/dao/impl/clarin/ClarinUserMetadataDAOImpl.java @@ -7,9 +7,15 @@ */ package org.dspace.content.dao.impl.clarin; +import java.sql.SQLException; +import java.util.List; +import java.util.UUID; +import javax.persistence.Query; + import org.dspace.content.clarin.ClarinUserMetadata; import org.dspace.content.dao.clarin.ClarinUserMetadataDAO; import org.dspace.core.AbstractHibernateDAO; +import org.dspace.core.Context; public class ClarinUserMetadataDAOImpl extends AbstractHibernateDAO implements ClarinUserMetadataDAO { @@ -17,4 +23,22 @@ public class ClarinUserMetadataDAOImpl extends AbstractHibernateDAO findByUserRegistrationAndBitstream(Context context, Integer userRegUUID, + UUID bitstreamUUID) throws SQLException { + Query query = createQuery(context, "SELECT cum FROM ClarinUserMetadata as cum " + + "JOIN cum.eperson as ur " + + "JOIN cum.transaction as clrua " + + "JOIN clrua.licenseResourceMapping as map " + + "WHERE ur.id = :userRegUUID " + + "AND map.bitstream.id = :bitstreamUUID " + + "ORDER BY clrua.id DESC"); + + query.setParameter("userRegUUID", userRegUUID); + query.setParameter("bitstreamUUID", bitstreamUUID); + query.setHint("org.hibernate.cacheable", Boolean.TRUE); + + return list(query); + } } diff --git a/dspace-api/src/main/java/org/dspace/content/service/clarin/ClarinUserMetadataService.java b/dspace-api/src/main/java/org/dspace/content/service/clarin/ClarinUserMetadataService.java index 12a4cb5ba01a..3ea93d398f05 100644 --- a/dspace-api/src/main/java/org/dspace/content/service/clarin/ClarinUserMetadataService.java +++ b/dspace-api/src/main/java/org/dspace/content/service/clarin/ClarinUserMetadataService.java @@ -9,6 +9,7 @@ import java.sql.SQLException; import java.util.List; +import java.util.UUID; import org.dspace.authorize.AuthorizeException; import org.dspace.content.clarin.ClarinUserMetadata; @@ -22,4 +23,8 @@ public interface ClarinUserMetadataService { List findAll(Context context) throws SQLException; void update(Context context, ClarinUserMetadata clarinUserMetadata) throws SQLException; void delete(Context context, ClarinUserMetadata clarinUserMetadata) throws SQLException, AuthorizeException; + + List findByUserRegistrationAndBitstream(Context context, Integer userRegUUID, + UUID bitstreamUUID, boolean lastTransaction) + throws SQLException; } diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinUserMetadataRestController.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinUserMetadataRestController.java index e726c600ce80..44036147c7f4 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinUserMetadataRestController.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinUserMetadataRestController.java @@ -238,8 +238,8 @@ public List processSignedInUser(Context context, EPerson cur currentUser.getID() + " is null."); } - // Copy current user_metadata records into a list and append it by a new user metadata. - List newClarinUserMetadataList = new ArrayList<>(clarinUserRegistration.getUserMetadata()); + // List of the new user metadata - passed from the request + List clarinUserMetadataList = new ArrayList<>(); // Create user metadata records from request for (ClarinUserMetadataRest clarinUserMetadataRest : clarinUserMetadataRestList) { @@ -249,20 +249,20 @@ public List processSignedInUser(Context context, EPerson cur clarinUserMetadata.setEperson(clarinUserRegistration); clarinUserMetadataService.update(context, clarinUserMetadata); // Add userMetadata to the list of the new user metadata - newClarinUserMetadataList.add(clarinUserMetadata); + clarinUserMetadataList.add(clarinUserMetadata); } // Process clrua with the new clarin user metadata ClarinLicenseResourceUserAllowance clrua = - this.createClrua(context, clarinLicenseResourceMapping, newClarinUserMetadataList, downloadToken, + this.createClrua(context, clarinLicenseResourceMapping, clarinUserMetadataList, downloadToken, clarinUserRegistration); // Add Clarin License Resource Allowance to the user metadata records - for (ClarinUserMetadata clarinUserMetadata : newClarinUserMetadataList) { + for (ClarinUserMetadata clarinUserMetadata : clarinUserMetadataList) { clarinUserMetadata.setTransaction(clrua); clarinUserMetadataService.update(context, clarinUserMetadata); } - return newClarinUserMetadataList; + return clarinUserMetadataList; } private ClarinLicenseResourceUserAllowance createClrua(Context context, diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinUserMetadataRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinUserMetadataRestRepository.java index acb9fef41e22..3d384427448c 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinUserMetadataRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinUserMetadataRestRepository.java @@ -10,7 +10,10 @@ import java.sql.SQLException; import java.util.List; import java.util.Objects; +import java.util.UUID; +import org.dspace.app.rest.Parameter; +import org.dspace.app.rest.SearchRestMethod; import org.dspace.app.rest.model.ClarinUserMetadataRest; import org.dspace.content.clarin.ClarinUserMetadata; import org.dspace.content.service.clarin.ClarinUserMetadataService; @@ -53,6 +56,19 @@ public Page findAll(Context context, Pageable pageable) } } + @SearchRestMethod(name = "byUserRegistrationAndBitstream") + public Page findByUserRegistrationAndBitstream( + @Parameter(value = "userRegUUID", required = true) Integer userRegId, + @Parameter(value = "bitstreamUUID", required = true) UUID bitstreamUUID, + Pageable pageable) throws SQLException { + Context context = obtainContext(); + + List clarinUserMetadataList = + clarinUserMetadataService.findByUserRegistrationAndBitstream(context, userRegId, bitstreamUUID, true); + + return converter.toRestPage(clarinUserMetadataList, pageable, utils.obtainProjection()); + } + @Override public Class getDomainClass() { return ClarinUserMetadataRest.class; diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinUserMetadataRestControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinUserMetadataRestControllerIT.java index 6eba735a2dfb..592363ba4270 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinUserMetadataRestControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinUserMetadataRestControllerIT.java @@ -9,6 +9,7 @@ import static org.dspace.app.rest.repository.ClarinLicenseRestRepository.OPERATION_PATH_LICENSE_RESOURCE; import static org.dspace.app.rest.repository.ClarinUserMetadataRestController.CHECK_EMAIL_RESPONSE_CONTENT; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; @@ -45,9 +46,12 @@ import org.dspace.content.WorkspaceItem; import org.dspace.content.clarin.ClarinLicense; import org.dspace.content.clarin.ClarinLicenseLabel; +import org.dspace.content.clarin.ClarinLicenseResourceUserAllowance; +import org.dspace.content.clarin.ClarinUserMetadata; import org.dspace.content.clarin.ClarinUserRegistration; import org.dspace.content.service.clarin.ClarinLicenseLabelService; import org.dspace.content.service.clarin.ClarinLicenseService; +import org.dspace.content.service.clarin.ClarinUserMetadataService; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.MediaType; @@ -58,10 +62,14 @@ public class ClarinUserMetadataRestControllerIT extends AbstractControllerIntegr ClarinLicenseService clarinLicenseService; @Autowired ClarinLicenseLabelService clarinLicenseLabelService; + @Autowired + ClarinUserMetadataService clarinUserMetadataService; WorkspaceItem witem; + WorkspaceItem witem2; ClarinLicense clarinLicense; Bitstream bitstream; + Bitstream bitstream2; // Attach ClarinLicense to the Bitstream private void prepareEnvironment(String requiredInfo, Integer confirmation) throws Exception { @@ -73,7 +81,8 @@ private void prepareEnvironment(String requiredInfo, Integer confirmation) throw // 1. Create WI with uploaded file context.turnOffAuthorisationSystem(); - witem = createWorkspaceItemWithFile(); + witem = this.createWorkspaceItemWithFile(false); + witem2 = this.createWorkspaceItemWithFile(true); List replaceOperations = new ArrayList(); String clarinLicenseName = "Test Clarin License"; @@ -96,16 +105,23 @@ private void prepareEnvironment(String requiredInfo, Integer confirmation) throw .content(updateBody) .contentType(MediaType.APPLICATION_JSON)) .andExpect(status().isOk()); + getClient(tokenAdmin).perform(patch("/api/submission/workspaceitems/" + witem2.getID()) + .content(updateBody) + .contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()); // 4. Check if the Clarin License name was added to the Item's metadata `dc.rights` getClient(tokenAdmin).perform(get("/api/submission/workspaceitems/" + witem.getID())) .andExpect(status().isOk()) .andExpect(jsonPath("$._embedded.item.metadata['dc.rights'][0].value", is(clarinLicenseName))); + getClient(tokenAdmin).perform(get("/api/submission/workspaceitems/" + witem2.getID())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.item.metadata['dc.rights'][0].value", is(clarinLicenseName))); // 5. Check if the Clarin License was attached to the Bitstream getClient(tokenAdmin).perform(get("/api/core/clarinlicenses/" + clarinLicense.getID())) .andExpect(status().isOk()) - .andExpect(jsonPath("$.bitstreams", is(1))); + .andExpect(jsonPath("$.bitstreams", is(2))); } @Test @@ -408,7 +424,137 @@ public void authorizedUserWithoutMetadata_shouldDownloadToken() throws Exception .andExpect(jsonPath("$.page.totalElements", is(1))); } - private WorkspaceItem createWorkspaceItemWithFile() { + @Test + public void shouldNotCreateDuplicateUserMetadataBasedOnHistory() throws Exception { + // Prepare environment with Clarin License, resource mapping, allowance, user registration and user metadata + // then try to download the same bitstream again and the user metadata should not be created based on history + this.prepareEnvironment("NAME,ADDRESS", 0); + context.turnOffAuthorisationSystem(); + ClarinUserRegistration clarinUserRegistration = ClarinUserRegistrationBuilder + .createClarinUserRegistration(context).withEPersonID(admin.getID()).build(); + context.restoreAuthSystemState(); + + ObjectMapper mapper = new ObjectMapper(); + ClarinUserMetadataRest clarinUserMetadata1 = new ClarinUserMetadataRest(); + clarinUserMetadata1.setMetadataKey("NAME"); + clarinUserMetadata1.setMetadataValue("Test"); + + ClarinUserMetadataRest clarinUserMetadata2 = new ClarinUserMetadataRest(); + clarinUserMetadata2.setMetadataKey("ADDRESS"); + clarinUserMetadata2.setMetadataValue("Test2"); + + List clarinUserMetadataRestList = new ArrayList<>(); + clarinUserMetadataRestList.add(clarinUserMetadata1); + clarinUserMetadataRestList.add(clarinUserMetadata2); + + String adminToken = getAuthToken(admin.getEmail(), password); + + // There should exist record in the UserRegistration table + getClient(adminToken).perform(get("/api/core/clarinuserregistrations") + .contentType(contentType)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.page.totalElements", is(1))); + + // Manage UserMetadata and get token + getClient(adminToken).perform(post("/api/core/clarinusermetadata/manage?bitstreamUUID=" + bitstream.getID()) + .content(mapper.writeValueAsBytes(clarinUserMetadataRestList.toArray())) + .contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$", notNullValue())) + .andExpect(jsonPath("$", not(CHECK_EMAIL_RESPONSE_CONTENT))); + + // Get created CLRUA + getClient(adminToken).perform(get("/api/core/clarinlruallowances") + .contentType(contentType)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.page.totalElements", is(1))); + + + // Get created User Metadata - there should be 2 records + getClient(adminToken).perform(get("/api/core/clarinusermetadata") + .contentType(contentType)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.page.totalElements", is(2))); + + // Second download + + // Manage UserMetadata and get token + getClient(adminToken).perform(post("/api/core/clarinusermetadata/manage?bitstreamUUID=" + bitstream2.getID()) + .content(mapper.writeValueAsBytes(clarinUserMetadataRestList.toArray())) + .contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$", notNullValue())) + .andExpect(jsonPath("$", not(CHECK_EMAIL_RESPONSE_CONTENT))); + + // Get created two CLRUA + getClient(adminToken).perform(get("/api/core/clarinlruallowances") + .contentType(contentType)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.page.totalElements", is(2))); + + // Get created User Metadata - there should be 4 records + getClient(adminToken).perform(get("/api/core/clarinusermetadata") + .contentType(contentType)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.page.totalElements", is(4))); + + // The User Metadata should not have updated transaction ID after a new download - test for fixed issue + List allUserMetadata = clarinUserMetadataService.findAll(context); + ClarinLicenseResourceUserAllowance clrua1 = allUserMetadata.get(0).getTransaction(); + ClarinLicenseResourceUserAllowance clrua2 = allUserMetadata.get(3).getTransaction(); + assertThat(clrua1.getID(), not(clrua2.getID())); + + // Check that the user registration for test data full user has been created + // Test /api/core/clarinusermetadatas search by userRegistrationAndBitstream endpoint + getClient(adminToken).perform(get("/api/core/clarinusermetadata/search/byUserRegistrationAndBitstream") + .param("userRegUUID", String.valueOf(clarinUserRegistration.getID())) + .param("bitstreamUUID", String.valueOf(bitstream2.getID())) + .contentType(contentType)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.page.totalElements", is(2))); + + // Download again the second bitstream and the user metadata should be returned only from the last transaction + + // Create a new User Metadata + ClarinUserMetadataRest clarinUserMetadata3 = new ClarinUserMetadataRest(); + clarinUserMetadata3.setMetadataKey("NAME"); + clarinUserMetadata3.setMetadataValue("New Test"); + + ClarinUserMetadataRest clarinUserMetadata4 = new ClarinUserMetadataRest(); + clarinUserMetadata4.setMetadataKey("ADDRESS"); + clarinUserMetadata4.setMetadataValue("New Test"); + + List newUserMetadataRestList = new ArrayList<>(); + newUserMetadataRestList.add(clarinUserMetadata3); + newUserMetadataRestList.add(clarinUserMetadata4); + + // Manage UserMetadata and get token + getClient(adminToken).perform(post("/api/core/clarinusermetadata/manage?bitstreamUUID=" + bitstream2.getID()) + .content(mapper.writeValueAsBytes(newUserMetadataRestList.toArray())) + .contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$", notNullValue())) + .andExpect(jsonPath("$", not(CHECK_EMAIL_RESPONSE_CONTENT))); + + // Get created two CLRUA + getClient(adminToken).perform(get("/api/core/clarinlruallowances") + .contentType(contentType)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.page.totalElements", is(3))); + + // Get created User Metadata from the new transaction - there should be 2 records + getClient(adminToken).perform(get("/api/core/clarinusermetadata/search/byUserRegistrationAndBitstream") + .param("userRegUUID", String.valueOf(clarinUserRegistration.getID())) + .param("bitstreamUUID", String.valueOf(bitstream2.getID())) + .contentType(contentType)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.page.totalElements", is(2))); + + // Delete all created user metadata - clean test environment + ClarinUserMetadataBuilder.deleteClarinUserMetadata(clarinUserRegistration.getID()); + } + + private WorkspaceItem createWorkspaceItemWithFile(boolean secondBitstream) { parentCommunity = CommunityBuilder.createCommunity(context) .withName("Parent Community") .build(); @@ -428,7 +574,11 @@ private WorkspaceItem createWorkspaceItemWithFile() { .withFulltext("simple-article.pdf", "/local/path/simple-article.pdf", pdf) .build(); - bitstream = witem.getItem().getBundles().get(0).getBitstreams().get(0); + if (secondBitstream) { + this.bitstream2 = witem.getItem().getBundles().get(0).getBitstreams().get(0); + } else { + this.bitstream = witem.getItem().getBundles().get(0).getBitstreams().get(0); + } return witem; }