From dc084af635f080a963947c628dad6daebe1a1af3 Mon Sep 17 00:00:00 2001 From: Paurikova2 Date: Wed, 11 Mar 2026 08:28:29 +0100 Subject: [PATCH 1/4] Fix duplicate user_registration rows; add dedup guard and tests --- .../ClarinUserRegistrationServiceImpl.java | 19 +++ ...0__user_registration_unique_eperson_id.sql | 11 ++ ...0__user_registration_unique_eperson_id.sql | 13 ++ .../ClarinUserRegistrationServiceImplIT.java | 136 ++++++++++++++++++ 4 files changed, 179 insertions(+) create mode 100644 dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V7.6_2026.03.10__user_registration_unique_eperson_id.sql create mode 100644 dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V7.6_2026.03.10__user_registration_unique_eperson_id.sql 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/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..38c16e71c9c5 --- /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,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/ +-- +-- Add a unique index on eperson_id for non-null values. +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-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..27ced9d87e27 --- /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,13 @@ +-- +-- 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/ +-- +-- Add a unique partial index on eperson_id (only for non-null values). +-- This allows multiple rows with eperson_id = NULL (e.g., anonymous registrations) +-- but enforces that each non-null eperson_id appears at most once. +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/test/java/org/dspace/app/rest/ClarinUserRegistrationServiceImplIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinUserRegistrationServiceImplIT.java index c4c30bc9463e..8084fdf5302a 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,133 @@ 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(); + + // First creation — should insert a new row + 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()); + + // Second creation with same eperson_id — should update, not insert + 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); + + // The returned registration must be the same row as the first one + 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 still works normally + * (i.e. the dedup guard does not prevent distinct users from each having a registration). + */ + @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(); + } } From 8b6e41c724f8c6c17052262b350e8497b37d139e Mon Sep 17 00:00:00 2001 From: Paurikova2 Date: Wed, 11 Mar 2026 09:29:41 +0100 Subject: [PATCH 2/4] removed comments --- .../app/rest/ClarinUserRegistrationServiceImplIT.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) 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 8084fdf5302a..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 @@ -57,7 +57,6 @@ public void createShouldUpdateExistingRegistrationForSameEPersonId() throws Exce .withPassword("password123") .build(); - // First creation — should insert a new row ClarinUserRegistration first = new ClarinUserRegistration(); first.setEmail("duptest@example.com"); first.setPersonID(testPerson.getID()); @@ -71,7 +70,6 @@ public void createShouldUpdateExistingRegistrationForSameEPersonId() throws Exce assertEquals("OrgA", created.getOrganization()); assertEquals("duptest@example.com", created.getEmail()); - // Second creation with same eperson_id — should update, not insert ClarinUserRegistration second = new ClarinUserRegistration(); second.setEmail("duptest-updated@example.com"); second.setPersonID(testPerson.getID()); @@ -81,7 +79,6 @@ public void createShouldUpdateExistingRegistrationForSameEPersonId() throws Exce ClarinUserRegistration result = clarinUserRegistrationService.create(context, second); assertNotNull("Second create should return a non-null registration", result); - // The returned registration must be the same row as the first one assertEquals("Should return the existing registration ID, not a new one", firstId, result.getID()); @@ -100,8 +97,7 @@ public void createShouldUpdateExistingRegistrationForSameEPersonId() throws Exce } /** - * Verify that creating registrations for different ePersons still works normally - * (i.e. the dedup guard does not prevent distinct users from each having a registration). + * Verify that creating registrations for different ePersons works */ @Test public void createShouldAllowDifferentEPersonIds() throws Exception { From 4209e5f7add5d7d50a1effd4e2dbc3859bbd33de Mon Sep 17 00:00:00 2001 From: Paurikova2 Date: Wed, 11 Mar 2026 13:51:09 +0100 Subject: [PATCH 3/4] fix h2 partial index migration --- .../V7.6_2026.03.10__user_registration_unique_eperson_id.sql | 5 ++--- .../V7.6_2026.03.10__user_registration_unique_eperson_id.sql | 4 +--- 2 files changed, 3 insertions(+), 6 deletions(-) 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 index 38c16e71c9c5..6aada8f6ab7e 100644 --- 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 @@ -5,7 +5,6 @@ -- -- http://www.dspace.org/license/ -- --- Add a unique index on eperson_id for non-null values. + CREATE UNIQUE INDEX IF NOT EXISTS user_registration_eperson_id_unique - ON user_registration (eperson_id) - WHERE eperson_id IS NOT NULL; + 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 index 27ced9d87e27..587b881d3c01 100644 --- 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 @@ -5,9 +5,7 @@ -- -- http://www.dspace.org/license/ -- --- Add a unique partial index on eperson_id (only for non-null values). --- This allows multiple rows with eperson_id = NULL (e.g., anonymous registrations) --- but enforces that each non-null eperson_id appears at most once. + CREATE UNIQUE INDEX IF NOT EXISTS user_registration_eperson_id_unique ON user_registration (eperson_id) WHERE eperson_id IS NOT NULL; From 1a24fa0b5cfb56a8de6f5a846c9b39e3259ba390 Mon Sep 17 00:00:00 2001 From: Paurikova2 Date: Wed, 11 Mar 2026 15:19:57 +0100 Subject: [PATCH 4/4] added tests, removed user registration created during import when eperson is created, check emails with ; --- .../clarin/ClarinUserRegistrationDAOImpl.java | 10 +++++- .../ClarinEPersonImportController.java | 8 +++++ .../rest/ClarinEPersonImportControllerIT.java | 34 +++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) 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-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);