From 9caa94555cc45c06076d44cac1ffaafe29a961ce Mon Sep 17 00:00:00 2001 From: milanmajchrak <90026355+milanmajchrak@users.noreply.github.com> Date: Thu, 28 Sep 2023 15:36:42 +0200 Subject: [PATCH] ufal/be-user_metadata-wrong-import (#440) * User Metadata is correctly imported - it is not updated but added every time with a transaction_id. * Refactoring and created test * Refactoring --- .../clarin/ClarinUserRegistration.java | 2 + .../ClarinUserRegistrationServiceImpl.java | 5 ++ .../dao/clarin/ClarinUserRegistrationDAO.java | 2 + .../clarin/ClarinUserRegistrationDAOImpl.java | 11 ++++ .../clarin/ClarinUserRegistrationService.java | 2 + .../ClarinUserMetadataRestController.java | 66 ++++++++----------- .../ClarinUserMetadataImportControllerIT.java | 60 +++++++++++++++++ 7 files changed, 109 insertions(+), 39 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/content/clarin/ClarinUserRegistration.java b/dspace-api/src/main/java/org/dspace/content/clarin/ClarinUserRegistration.java index a6fd61a10fc5..a3e3666a01f4 100644 --- a/dspace-api/src/main/java/org/dspace/content/clarin/ClarinUserRegistration.java +++ b/dspace-api/src/main/java/org/dspace/content/clarin/ClarinUserRegistration.java @@ -28,6 +28,8 @@ @Table(name = "user_registration") public class ClarinUserRegistration implements ReloadableEntity { + public static final String ANONYMOUS_USER_REGISTRATION = "anonymous"; + private static Logger log = org.apache.logging.log4j.LogManager.getLogger(ClarinUserRegistration.class); @Id diff --git a/dspace-api/src/main/java/org/dspace/content/clarin/ClarinUserRegistrationServiceImpl.java b/dspace-api/src/main/java/org/dspace/content/clarin/ClarinUserRegistrationServiceImpl.java index d79bc0cbb5f3..c96ee2db8410 100644 --- a/dspace-api/src/main/java/org/dspace/content/clarin/ClarinUserRegistrationServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/clarin/ClarinUserRegistrationServiceImpl.java @@ -80,6 +80,11 @@ public List findByEPersonUUID(Context context, UUID eper return clarinUserRegistrationDAO.findByEPersonUUID(context, epersonUUID); } + @Override + public List findByEmail(Context context, String email) throws SQLException { + return clarinUserRegistrationDAO.findByEmail(context, email); + } + @Override public void delete(Context context, ClarinUserRegistration clarinUserRegistration) throws SQLException, AuthorizeException { diff --git a/dspace-api/src/main/java/org/dspace/content/dao/clarin/ClarinUserRegistrationDAO.java b/dspace-api/src/main/java/org/dspace/content/dao/clarin/ClarinUserRegistrationDAO.java index 26e7541972ae..1a6a8b1be5d1 100644 --- a/dspace-api/src/main/java/org/dspace/content/dao/clarin/ClarinUserRegistrationDAO.java +++ b/dspace-api/src/main/java/org/dspace/content/dao/clarin/ClarinUserRegistrationDAO.java @@ -18,4 +18,6 @@ public interface ClarinUserRegistrationDAO extends GenericDAO { List findByEPersonUUID(Context context, UUID epersonUUID) throws SQLException; + + List findByEmail(Context context, String email) throws SQLException; } diff --git a/dspace-api/src/main/java/org/dspace/content/dao/impl/clarin/ClarinUserRegistrationDAOImpl.java b/dspace-api/src/main/java/org/dspace/content/dao/impl/clarin/ClarinUserRegistrationDAOImpl.java index 3d7d24b015b3..0537a45f42ed 100644 --- a/dspace-api/src/main/java/org/dspace/content/dao/impl/clarin/ClarinUserRegistrationDAOImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/dao/impl/clarin/ClarinUserRegistrationDAOImpl.java @@ -34,4 +34,15 @@ public List findByEPersonUUID(Context context, UUID eper return list(query); } + + @Override + public List findByEmail(Context context, String email) throws SQLException { + Query query = createQuery(context, "SELECT cur FROM ClarinUserRegistration as cur " + + "WHERE cur.email = :email"); + + query.setParameter("email", email); + query.setHint("org.hibernate.cacheable", Boolean.TRUE); + + return list(query); + } } diff --git a/dspace-api/src/main/java/org/dspace/content/service/clarin/ClarinUserRegistrationService.java b/dspace-api/src/main/java/org/dspace/content/service/clarin/ClarinUserRegistrationService.java index 209184aef703..acef876ab00a 100644 --- a/dspace-api/src/main/java/org/dspace/content/service/clarin/ClarinUserRegistrationService.java +++ b/dspace-api/src/main/java/org/dspace/content/service/clarin/ClarinUserRegistrationService.java @@ -25,6 +25,8 @@ ClarinUserRegistration create(Context context, ClarinUserRegistration find(Context context, int valueId) throws SQLException; List findAll(Context context) throws SQLException, AuthorizeException; List findByEPersonUUID(Context context, UUID epersonUUID) throws SQLException; + + List findByEmail(Context context, String email) throws SQLException; void delete(Context context, ClarinUserRegistration clarinUserRegistration) throws SQLException, AuthorizeException; void update(Context context, ClarinUserRegistration clarinUserRegistration) throws SQLException, AuthorizeException; } 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 e52c971869b1..b109ea3c2e69 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 @@ -10,6 +10,7 @@ import static javax.ws.rs.core.MediaType.APPLICATION_JSON; import static org.dspace.app.rest.utils.ContextUtil.obtainContext; import static org.dspace.content.clarin.ClarinLicense.SEND_TOKEN; +import static org.dspace.content.clarin.ClarinUserRegistration.ANONYMOUS_USER_REGISTRATION; import static org.springframework.web.bind.annotation.RequestMethod.POST; import java.io.IOException; @@ -222,44 +223,18 @@ public List processSignedInUser(Context context, EPerson cur currentUser.getID() + " is null."); } - List currentClarinUserMetadataList = clarinUserRegistration.getUserMetadata(); - List newClarinUserMetadataList; - // If exists ClarinResourceUserAllowance - Clrua record in the table, create a new clrua with current - // resource mapping, user metadata, user registration - if (CollectionUtils.isEmpty(currentClarinUserMetadataList)) { - // The current user doesn't fill in any user metadata, create a new UserMetadata objects - newClarinUserMetadataList = this.createUserMetadataFromRequest(context, - clarinUserMetadataRestList); - } else { - // The current user does fill in any user metadata, update actual UserMetadata objects and create - // the new ones is some are missing. - // Compare the old metadata value with the new one and if the value is changed or missing, create/update - // the metadata value. - newClarinUserMetadataList = new ArrayList<>(); - for (ClarinUserMetadataRest clarinUserMetadataRest : clarinUserMetadataRestList) { - boolean shouldCreate = true; - for (ClarinUserMetadata clarinUserMetadata: currentClarinUserMetadataList) { - if (StringUtils.equals(clarinUserMetadataRest.getMetadataKey(), - clarinUserMetadata.getMetadataKey())) { - shouldCreate = false; - // Set metadata value - clarinUserMetadata.setMetadataValue(clarinUserMetadataRest.getMetadataValue()); - // Update the user metadata record - clarinUserMetadataService.update(context, clarinUserMetadata); - // Add userMetadata to the list of the new user metadata - newClarinUserMetadataList.add(clarinUserMetadata); - } - } - if (shouldCreate) { - ClarinUserMetadata clarinUserMetadata = this.clarinUserMetadataService.create(context); - clarinUserMetadata.setMetadataKey(clarinUserMetadataRest.getMetadataKey()); - clarinUserMetadata.setMetadataValue(clarinUserMetadataRest.getMetadataValue()); - clarinUserMetadata.setEperson(clarinUserRegistration); - clarinUserMetadataService.update(context, clarinUserMetadata); - // Add userMetadata to the list of the new user metadata - newClarinUserMetadataList.add(clarinUserMetadata); - } - } + // Copy current user_metadata records into a list and append it by a new user metadata. + List newClarinUserMetadataList = new ArrayList<>(clarinUserRegistration.getUserMetadata()); + + // Create user metadata records from request + for (ClarinUserMetadataRest clarinUserMetadataRest : clarinUserMetadataRestList) { + ClarinUserMetadata clarinUserMetadata = this.clarinUserMetadataService.create(context); + clarinUserMetadata.setMetadataKey(clarinUserMetadataRest.getMetadataKey()); + clarinUserMetadata.setMetadataValue(clarinUserMetadataRest.getMetadataValue()); + clarinUserMetadata.setEperson(clarinUserRegistration); + clarinUserMetadataService.update(context, clarinUserMetadata); + // Add userMetadata to the list of the new user metadata + newClarinUserMetadataList.add(clarinUserMetadata); } // Process clrua with the new clarin user metadata @@ -306,12 +281,25 @@ public List processNonSignedInUser(Context context, List clarinUserMetadataList = this.createUserMetadataFromRequest(context, clarinUserMetadataRestList); + // Get anonymous user registration - user metadata should not have `user_registration_id = null` + ClarinUserRegistration clarinUserRegistration = null; + List clarinUserRegistrationList = clarinUserRegistrationService + .findByEmail(context, ANONYMOUS_USER_REGISTRATION); + for (ClarinUserRegistration fetchedClarinuserRegistration : clarinUserRegistrationList) { + if (!StringUtils.equals(fetchedClarinuserRegistration.getOrganization(), ANONYMOUS_USER_REGISTRATION)) { + continue; + } + clarinUserRegistration = fetchedClarinuserRegistration; + break; + } + // Create ClarinResourceUserAllowance record to generate token. ClarinLicenseResourceUserAllowance clrua = this.createClrua(context, clarinLicenseResourceMapping, - clarinUserMetadataList, downloadToken, null); + clarinUserMetadataList, downloadToken, clarinUserRegistration); // Add Clarin License Resource Allowance to the user metadata records for (ClarinUserMetadata clarinUserMetadata : clarinUserMetadataList) { clarinUserMetadata.setTransaction(clrua); + clarinUserMetadata.setEperson(clarinUserRegistration); clarinUserMetadataService.update(context, clarinUserMetadata); } return clarinUserMetadataList; diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinUserMetadataImportControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinUserMetadataImportControllerIT.java index 1db39d2b8d81..91187884f528 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinUserMetadataImportControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinUserMetadataImportControllerIT.java @@ -211,6 +211,66 @@ public void importUserMetadataWithoutEpersonTest() throws Exception { ClarinUserMetadataBuilder.deleteClarinUserMetadata(clarinUserRegistration.getID()); } + // The user metadata shouldn't be updated, but it should be added + @Test + public void importTwoTimesUserMetadataWithEpersonTest() throws Exception { + this.prepareEnvironment("NAME"); + 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"); + + List clarinUserMetadataRestList = new ArrayList<>(); + clarinUserMetadataRestList.add(clarinUserMetadata1); + + 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/clarin/import/usermetadata") + .content(mapper.writeValueAsBytes(clarinUserMetadataRestList.toArray())) + .contentType(MediaType.APPLICATION_JSON) + .param("userRegistrationId", clarinUserRegistration.getID().toString()) + .param("bitstreamUUID", bitstream.getID().toString()) + .param("createdOn", "2012-09-19T10:30:03.741633") + .param("token", "111")) + .andExpect(status().isOk()); + + getClient(adminToken).perform(post("/api/clarin/import/usermetadata") + .content(mapper.writeValueAsBytes(clarinUserMetadataRestList.toArray())) + .contentType(MediaType.APPLICATION_JSON) + .param("userRegistrationId", clarinUserRegistration.getID().toString()) + .param("bitstreamUUID", bitstream.getID().toString()) + .param("createdOn", "2012-09-19T10:30:03.741633") + .param("token", "111")) + .andExpect(status().isOk()); + + List allUserMetadata = clarinUserMetadataService.findAll(context); + // UserMetadata should be created and not updated + assertEquals(2, allUserMetadata.size()); + + // get first created data and check it + ClarinUserMetadata clarinUserMetadata = allUserMetadata.get(0); + assertEquals(clarinUserMetadata.getMetadataKey(), "NAME"); + assertEquals(clarinUserMetadata.getMetadataValue(), "Test"); + assertEquals(clarinUserMetadata.getEperson().getPersonID(), admin.getID()); + assertEquals(clarinUserMetadata.getTransaction().getCreatedOn().getTime(), + getDateFromString("2012-09-19T10:30:03.741633").getTime()); + assertEquals(clarinUserMetadata.getTransaction().getToken(), "111"); + + //clean all + ClarinUserMetadataBuilder.deleteClarinUserMetadata(clarinUserRegistration.getID()); + } + /** * Create Workspace item with file. */