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 ae7b7bb048a3..9be158d376f1 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 @@ -59,6 +59,25 @@ public ClarinUserRegistration create(Context context, "You must be an admin to create a CLARIN user registration"); } + // Prevent duplicate registrations for the same eperson_id. + // If a registration already exists for this EPerson, update it instead of creating a new one. + UUID epersonId = clarinUserRegistration.getPersonID(); + if (Objects.nonNull(epersonId)) { + List existing = clarinUserRegistrationDAO.findByEPersonUUID(context, epersonId); + if (!CollectionUtils.isEmpty(existing)) { + ClarinUserRegistration existingRegistration = existing.get(0); + log.info("ClarinUserRegistration already exists for eperson_id={}. " + + "Updating existing registration (id={}) instead of creating a duplicate.", + epersonId, existingRegistration.getID()); + // Update the existing registration with new values + existingRegistration.setEmail(clarinUserRegistration.getEmail()); + existingRegistration.setOrganization(clarinUserRegistration.getOrganization()); + existingRegistration.setConfirmation(clarinUserRegistration.isConfirmation()); + clarinUserRegistrationDAO.save(context, existingRegistration); + return existingRegistration; + } + } + return clarinUserRegistrationDAO.create(context, clarinUserRegistration); } 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 0537a45f42ed..0d92bc6eac6a 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 @@ -37,10 +37,18 @@ public List findByEPersonUUID(Context context, UUID eper @Override public List findByEmail(Context context, String email) throws SQLException { + // The email column may contain multiple semicolon-separated addresses (e.g. "a@x.com;b@x.com"). + // Match the address when it appears as the only value, at the start, in the middle, or at the end. Query query = createQuery(context, "SELECT cur FROM ClarinUserRegistration as cur " + - "WHERE cur.email = :email"); + "WHERE cur.email = :email " + + "OR cur.email LIKE :emailStart " + + "OR cur.email LIKE :emailMiddle " + + "OR cur.email LIKE :emailEnd"); query.setParameter("email", email); + query.setParameter("emailStart", email + ";%"); + query.setParameter("emailMiddle", "%;" + email + ";%"); + query.setParameter("emailEnd", "%;" + email); query.setHint("org.hibernate.cacheable", Boolean.TRUE); return list(query); diff --git a/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V7.6_2026.03.10__user_registration_unique_eperson_id.sql b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V7.6_2026.03.10__user_registration_unique_eperson_id.sql new file mode 100644 index 000000000000..6aada8f6ab7e --- /dev/null +++ b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V7.6_2026.03.10__user_registration_unique_eperson_id.sql @@ -0,0 +1,10 @@ +-- +-- The contents of this file are subject to the license and copyright +-- detailed in the LICENSE and NOTICE files at the root of the source +-- tree and available online at +-- +-- http://www.dspace.org/license/ +-- + +CREATE UNIQUE INDEX IF NOT EXISTS user_registration_eperson_id_unique + ON user_registration (eperson_id); diff --git a/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V7.6_2026.03.10__user_registration_unique_eperson_id.sql b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V7.6_2026.03.10__user_registration_unique_eperson_id.sql new file mode 100644 index 000000000000..587b881d3c01 --- /dev/null +++ b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V7.6_2026.03.10__user_registration_unique_eperson_id.sql @@ -0,0 +1,11 @@ +-- +-- The contents of this file are subject to the license and copyright +-- detailed in the LICENSE and NOTICE files at the root of the source +-- tree and available online at +-- +-- http://www.dspace.org/license/ +-- + +CREATE UNIQUE INDEX IF NOT EXISTS user_registration_eperson_id_unique + ON user_registration (eperson_id) + WHERE eperson_id IS NOT NULL; diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinEPersonImportController.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinEPersonImportController.java index 0bb7204fa437..1b81b63cfc14 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinEPersonImportController.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinEPersonImportController.java @@ -103,6 +103,14 @@ public EPersonRest importEPerson(HttpServletRequest request) //salt and digest_algorithm are changing with password EPersonRest epersonRest = ePersonRestRepository.createAndReturn(context); EPerson eperson = ePersonService.find(context, UUID.fromString(epersonRest.getUuid())); + + // Remove the automatically created "Unknown" user registration - during import, + // user registrations are managed separately via the importUserRegistration endpoint. + List autoCreatedRegistrations = + clarinUserRegistrationService.findByEPersonUUID(context, eperson.getID()); + for (ClarinUserRegistration reg : autoCreatedRegistrations) { + clarinUserRegistrationService.delete(context, reg); + } eperson.setSelfRegistered(selfRegistered); eperson.setLastActive(lastActive); //the password is already hashed in the request diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinEPersonImportControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinEPersonImportControllerIT.java index 956cf81c422a..530880961b50 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinEPersonImportControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinEPersonImportControllerIT.java @@ -322,6 +322,40 @@ public void updatesRegistrationWhenBothEmailAndEPersonIDMatch() throws Exception } } + @Test + public void importEpersonDoesNotCreateUserRegistration() throws Exception { + ObjectMapper mapper = new ObjectMapper(); + EPersonRest data = new EPersonRest(); + data.setEmail("importnoregistration@example.com"); + data.setCanLogIn(true); + data.setMetadata(new MetadataRest()); + + AtomicReference idRef = new AtomicReference<>(); + String authToken = getAuthToken(admin.getEmail(), password); + + try { + getClient(authToken).perform(post("/api/clarin/import/eperson") + .content(mapper.writeValueAsBytes(data)) + .contentType(contentType) + .param("selfRegistered", "false") + .param("lastActive", "2020-01-01T00:00:00.000")) + .andExpect(status().isOk()) + .andDo(result -> idRef + .set(UUID.fromString(read(result.getResponse().getContentAsString(), "$.id")))); + + // Verify no user registration was automatically created for the imported EPerson + EPerson currentUser = context.getCurrentUser(); + context.setCurrentUser(admin); + java.util.List registrations = + clarinUserRegistrationService.findByEPersonUUID(context, idRef.get()); + context.setCurrentUser(currentUser); + + assertTrue("No user registration should be created on EPerson import", registrations.isEmpty()); + } finally { + EPersonBuilder.deleteEPerson(idRef.get()); + } + } + private String getStringFromDate(Date value) throws ParseException { DateFormat df = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS"); return df.format(value); diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinUserRegistrationServiceImplIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinUserRegistrationServiceImplIT.java index c4c30bc9463e..285c98a7bd06 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinUserRegistrationServiceImplIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinUserRegistrationServiceImplIT.java @@ -7,8 +7,15 @@ */ package org.dspace.app.rest; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +import java.util.List; + import org.dspace.app.rest.test.AbstractControllerIntegrationTest; import org.dspace.builder.ClarinUserRegistrationBuilder; +import org.dspace.builder.EPersonBuilder; import org.dspace.content.clarin.ClarinUserRegistration; import org.dspace.content.service.clarin.ClarinUserRegistrationService; import org.dspace.eperson.EPerson; @@ -36,4 +43,129 @@ public void testFind() throws Exception { .find(context, clarinUserRegistration.getID())); context.setCurrentUser(currentUser); } + + /** + * Verify that calling create() with the same eperson_id twice does not produce a duplicate row + * but instead updates the existing registration and returns it. + */ + @Test + public void createShouldUpdateExistingRegistrationForSameEPersonId() throws Exception { + context.turnOffAuthorisationSystem(); + + EPerson testPerson = EPersonBuilder.createEPerson(context) + .withEmail("duptest@example.com") + .withPassword("password123") + .build(); + + ClarinUserRegistration first = new ClarinUserRegistration(); + first.setEmail("duptest@example.com"); + first.setPersonID(testPerson.getID()); + first.setOrganization("OrgA"); + first.setConfirmation(false); + + ClarinUserRegistration created = clarinUserRegistrationService.create(context, first); + assertNotNull("First create should return a non-null registration", created); + Integer firstId = created.getID(); + assertNotNull("Created registration should have an ID", firstId); + assertEquals("OrgA", created.getOrganization()); + assertEquals("duptest@example.com", created.getEmail()); + + ClarinUserRegistration second = new ClarinUserRegistration(); + second.setEmail("duptest-updated@example.com"); + second.setPersonID(testPerson.getID()); + second.setOrganization("OrgB"); + second.setConfirmation(true); + + ClarinUserRegistration result = clarinUserRegistrationService.create(context, second); + assertNotNull("Second create should return a non-null registration", result); + + assertEquals("Should return the existing registration ID, not a new one", + firstId, result.getID()); + + // Fields should be updated to the new values + assertEquals("OrgB", result.getOrganization()); + assertEquals("duptest-updated@example.com", result.getEmail()); + assertTrue("Confirmation should be updated to true", result.isConfirmation()); + + // Verify only one row exists in the database for this eperson_id + List allForEPerson = + clarinUserRegistrationService.findByEPersonUUID(context, testPerson.getID()); + assertEquals("There must be exactly one registration for this eperson_id", + 1, allForEPerson.size()); + + context.restoreAuthSystemState(); + } + + /** + * Verify that creating registrations for different ePersons works + */ + @Test + public void createShouldAllowDifferentEPersonIds() throws Exception { + context.turnOffAuthorisationSystem(); + + EPerson personA = EPersonBuilder.createEPerson(context) + .withEmail("personA@example.com") + .withPassword("password123") + .build(); + EPerson personB = EPersonBuilder.createEPerson(context) + .withEmail("personB@example.com") + .withPassword("password123") + .build(); + + ClarinUserRegistration regA = new ClarinUserRegistration(); + regA.setEmail("personA@example.com"); + regA.setPersonID(personA.getID()); + regA.setOrganization("OrgA"); + regA.setConfirmation(true); + + ClarinUserRegistration regB = new ClarinUserRegistration(); + regB.setEmail("personB@example.com"); + regB.setPersonID(personB.getID()); + regB.setOrganization("OrgB"); + regB.setConfirmation(true); + + ClarinUserRegistration createdA = clarinUserRegistrationService.create(context, regA); + ClarinUserRegistration createdB = clarinUserRegistrationService.create(context, regB); + + assertNotNull(createdA); + assertNotNull(createdB); + // They must be distinct rows + assertTrue("Registrations for different ePersons must have different IDs", + !createdA.getID().equals(createdB.getID())); + assertEquals("OrgA", createdA.getOrganization()); + assertEquals("OrgB", createdB.getOrganization()); + + context.restoreAuthSystemState(); + } + + /** + * Verify that creating a registration with a null eperson_id (anonymous) does not trigger + * the dedup guard and creates a new row every time. + */ + @Test + public void createShouldAllowMultipleNullEPersonIds() throws Exception { + context.turnOffAuthorisationSystem(); + + ClarinUserRegistration anon1 = new ClarinUserRegistration(); + anon1.setEmail("anonymous"); + anon1.setOrganization("Unknown"); + anon1.setConfirmation(true); + // personID is null by default + + ClarinUserRegistration anon2 = new ClarinUserRegistration(); + anon2.setEmail("anonymous"); + anon2.setOrganization("Unknown"); + anon2.setConfirmation(true); + + ClarinUserRegistration created1 = clarinUserRegistrationService.create(context, anon1); + ClarinUserRegistration created2 = clarinUserRegistrationService.create(context, anon2); + + assertNotNull(created1); + assertNotNull(created2); + // Both anonymous, so each should get its own row + assertTrue("Null-eperson registrations should create separate rows", + !created1.getID().equals(created2.getID())); + + context.restoreAuthSystemState(); + } }