From dc084af635f080a963947c628dad6daebe1a1af3 Mon Sep 17 00:00:00 2001 From: Paurikova2 Date: Wed, 11 Mar 2026 08:28:29 +0100 Subject: [PATCH 1/9] 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/9] 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/9] 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/9] 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); From 7b50731dd3adeaf87f6eefd0fdbddbeea777d584 Mon Sep 17 00:00:00 2001 From: Paurikova2 Date: Wed, 25 Mar 2026 11:43:01 +0100 Subject: [PATCH 5/9] removed email from user_registration table, added missing creating user_registration, updated tests, upadted import for user_registration --- .../administer/CreateAdministrator.java | 1 - .../authenticate/LDAPAuthentication.java | 10 ++ .../authenticate/OidcAuthenticationBean.java | 12 +++ .../authenticate/OrcidAuthenticationBean.java | 12 +++ .../authenticate/ShibAuthentication.java | 37 +++++--- .../authenticate/X509Authentication.java | 10 ++ .../clarin/ClarinShibAuthentication.java | 38 +++++--- .../clarin/ClarinUserRegistration.java | 13 +-- .../ClarinUserRegistrationServiceImpl.java | 5 +- .../content/crosswalk/AIPTechMDCrosswalk.java | 15 +++ .../dao/clarin/ClarinUserRegistrationDAO.java | 2 +- .../clarin/ClarinUserRegistrationDAOImpl.java | 17 +--- .../dspace/content/packager/RoleIngester.java | 13 +++ .../clarin/ClarinUserRegistrationService.java | 3 +- .../org/dspace/eperson/EPersonCLITool.java | 1 - ...0__user_registration_unique_eperson_id.sql | 2 + ...0__user_registration_unique_eperson_id.sql | 2 + .../ClarinUserRegistrationBuilder.java | 5 - .../ClarinUserRegistrationConverter.java | 1 - .../model/ClarinUserRegistrationRest.java | 9 -- .../ClarinEPersonImportController.java | 94 ++----------------- .../ClarinUserMetadataRestController.java | 2 +- .../ClarinUserRegistrationRestRepository.java | 1 + .../repository/EPersonRestRepository.java | 1 - .../rest/ClarinEPersonImportControllerIT.java | 53 ++++------- .../ClarinUserRegistrationServiceImplIT.java | 10 +- .../app/rest/EPersonRestRepositoryIT.java | 5 - 27 files changed, 164 insertions(+), 210 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/administer/CreateAdministrator.java b/dspace-api/src/main/java/org/dspace/administer/CreateAdministrator.java index 0006f5c01afd..bfc4ccc7594c 100644 --- a/dspace-api/src/main/java/org/dspace/administer/CreateAdministrator.java +++ b/dspace-api/src/main/java/org/dspace/administer/CreateAdministrator.java @@ -307,7 +307,6 @@ protected void createAdministrator(String email, String first, String last, ClarinUserRegistration clarinUserRegistration = new ClarinUserRegistration(); clarinUserRegistration.setOrganization(organization); clarinUserRegistration.setConfirmation(true); - clarinUserRegistration.setEmail(eperson.getEmail()); clarinUserRegistration.setPersonID(eperson.getID()); clarinUserRegistrationService.create(context, clarinUserRegistration); diff --git a/dspace-api/src/main/java/org/dspace/authenticate/LDAPAuthentication.java b/dspace-api/src/main/java/org/dspace/authenticate/LDAPAuthentication.java index da6a70924818..fa1e026879c9 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/LDAPAuthentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/LDAPAuthentication.java @@ -38,6 +38,9 @@ import org.dspace.authenticate.factory.AuthenticateServiceFactory; import org.dspace.authenticate.service.AuthenticationService; import org.dspace.authorize.AuthorizeException; +import org.dspace.content.clarin.ClarinUserRegistration; +import org.dspace.content.factory.ClarinServiceFactory; +import org.dspace.content.service.clarin.ClarinUserRegistrationService; import org.dspace.core.Context; import org.dspace.core.LogHelper; import org.dspace.eperson.EPerson; @@ -80,6 +83,8 @@ public class LDAPAuthentication implements AuthenticationMethod { = EPersonServiceFactory.getInstance().getEPersonService(); protected GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); + private ClarinUserRegistrationService clarinUserRegistrationService = + ClarinServiceFactory.getInstance().getClarinUserRegistration(); protected ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService(); @@ -344,6 +349,11 @@ public int authenticate(Context context, authenticationService.initEPerson(context, request, eperson); ePersonService.update(context, eperson); context.dispatchEvents(); + ClarinUserRegistration clarinUserRegistration = new ClarinUserRegistration(); + clarinUserRegistration.setPersonID(eperson.getID()); + clarinUserRegistration.setOrganization(ClarinUserRegistration.UNKNOWN_USER_REGISTRATION); + clarinUserRegistration.setConfirmation(false); + clarinUserRegistrationService.create(context, clarinUserRegistration); context.setCurrentUser(eperson); request.setAttribute(LDAP_AUTHENTICATED, true); diff --git a/dspace-api/src/main/java/org/dspace/authenticate/OidcAuthenticationBean.java b/dspace-api/src/main/java/org/dspace/authenticate/OidcAuthenticationBean.java index 8a4ac190c816..2dbe43e70550 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/OidcAuthenticationBean.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/OidcAuthenticationBean.java @@ -26,6 +26,9 @@ import org.apache.commons.lang3.StringUtils; import org.dspace.authenticate.oidc.OidcClient; import org.dspace.authenticate.oidc.model.OidcTokenResponseDTO; +import org.dspace.content.clarin.ClarinUserRegistration; +import org.dspace.content.factory.ClarinServiceFactory; +import org.dspace.content.service.clarin.ClarinUserRegistrationService; import org.dspace.core.Context; import org.dspace.eperson.EPerson; import org.dspace.eperson.Group; @@ -64,6 +67,9 @@ public class OidcAuthenticationBean implements AuthenticationMethod { @Autowired private EPersonService ePersonService; + private ClarinUserRegistrationService clarinUserRegistrationService = + ClarinServiceFactory.getInstance().getClarinUserRegistration(); + @Override public boolean allowSetPassword(Context context, HttpServletRequest request, String username) throws SQLException { return false; @@ -221,6 +227,12 @@ private int registerNewEPerson(Context context, Map userInfo, St context.setCurrentUser(eperson); context.dispatchEvents(); + ClarinUserRegistration clarinUserRegistration = new ClarinUserRegistration(); + clarinUserRegistration.setPersonID(eperson.getID()); + clarinUserRegistration.setOrganization(ClarinUserRegistration.UNKNOWN_USER_REGISTRATION); + clarinUserRegistration.setConfirmation(false); + clarinUserRegistrationService.create(context, clarinUserRegistration); + return SUCCESS; } catch (Exception ex) { diff --git a/dspace-api/src/main/java/org/dspace/authenticate/OrcidAuthenticationBean.java b/dspace-api/src/main/java/org/dspace/authenticate/OrcidAuthenticationBean.java index a11bbfc867b4..ee1c25b6c942 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/OrcidAuthenticationBean.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/OrcidAuthenticationBean.java @@ -24,6 +24,9 @@ import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.lang3.StringUtils; import org.dspace.authorize.AuthorizeException; +import org.dspace.content.clarin.ClarinUserRegistration; +import org.dspace.content.factory.ClarinServiceFactory; +import org.dspace.content.service.clarin.ClarinUserRegistrationService; import org.dspace.core.Context; import org.dspace.eperson.EPerson; import org.dspace.eperson.Group; @@ -78,6 +81,9 @@ public class OrcidAuthenticationBean implements AuthenticationMethod { @Autowired private OrcidTokenService orcidTokenService; + private ClarinUserRegistrationService clarinUserRegistrationService = + ClarinServiceFactory.getInstance().getClarinUserRegistration(); + @Override public int authenticate(Context context, String username, String password, String realm, HttpServletRequest request) throws SQLException { @@ -245,6 +251,12 @@ private int registerNewEPerson(Context context, Person person, OrcidTokenRespons context.setCurrentUser(eperson); context.dispatchEvents(); + ClarinUserRegistration clarinUserRegistration = new ClarinUserRegistration(); + clarinUserRegistration.setPersonID(eperson.getID()); + clarinUserRegistration.setOrganization(ClarinUserRegistration.UNKNOWN_USER_REGISTRATION); + clarinUserRegistration.setConfirmation(false); + clarinUserRegistrationService.create(context, clarinUserRegistration); + return SUCCESS; } catch (Exception ex) { diff --git a/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java b/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java index d9d5338877e7..8553d467ec83 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java @@ -771,20 +771,17 @@ protected EPerson registerNewEPerson(Context context, HttpServletRequest request * Register User in the CLARIN license database * */ - // if no email the registration is postponed after entering and confirming mail - if (Objects.nonNull(email)) { - try { - ClarinUserRegistration clarinUserRegistration = new ClarinUserRegistration(); - clarinUserRegistration.setConfirmation(true); - clarinUserRegistration.setEmail(email); - clarinUserRegistration.setPersonID(eperson.getID()); - clarinUserRegistration.setOrganization(netid); - clarinUserRegistrationService.create(context, clarinUserRegistration); - eperson.setCanLogIn(false); - ePersonService.update(context, eperson); - } catch (Exception e) { - throw new AuthorizeException("User has not been added among registred users!"); - } + try { + ClarinUserRegistration clarinUserRegistration = new ClarinUserRegistration(); + clarinUserRegistration.setConfirmation(true); + clarinUserRegistration.setPersonID(eperson.getID()); + clarinUserRegistration.setOrganization( + Objects.nonNull(netid) ? netid : ClarinUserRegistration.UNKNOWN_USER_REGISTRATION); + clarinUserRegistrationService.create(context, clarinUserRegistration); + eperson.setCanLogIn(false); + ePersonService.update(context, eperson); + } catch (Exception e) { + throw new AuthorizeException("User has not been added among registred users!"); } /* CLARIN */ @@ -905,6 +902,18 @@ protected void updateEPerson(Context context, HttpServletRequest request, EPerso } ePersonService.update(context, eperson); context.dispatchEvents(); + + // Sync ClarinUserRegistration organization from the current Shibboleth netid + if (netid != null) { + List registrations = + clarinUserRegistrationService.findByEPersonUUID(context, eperson.getID()); + if (!registrations.isEmpty()) { + ClarinUserRegistration reg = registrations.get(0); + reg.setOrganization(netid); + clarinUserRegistrationService.update(context, reg); + } + } + context.restoreAuthSystemState(); } diff --git a/dspace-api/src/main/java/org/dspace/authenticate/X509Authentication.java b/dspace-api/src/main/java/org/dspace/authenticate/X509Authentication.java index 12dc5feda583..e2e76df91069 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/X509Authentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/X509Authentication.java @@ -34,6 +34,9 @@ import org.dspace.authenticate.factory.AuthenticateServiceFactory; import org.dspace.authenticate.service.AuthenticationService; import org.dspace.authorize.AuthorizeException; +import org.dspace.content.clarin.ClarinUserRegistration; +import org.dspace.content.factory.ClarinServiceFactory; +import org.dspace.content.service.clarin.ClarinUserRegistrationService; import org.dspace.core.Context; import org.dspace.core.LogHelper; import org.dspace.eperson.EPerson; @@ -125,6 +128,8 @@ public class X509Authentication implements AuthenticationMethod { .getAuthenticationService(); protected EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService(); protected GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); + private ClarinUserRegistrationService clarinUserRegistrationService = + ClarinServiceFactory.getInstance().getClarinUserRegistration(); protected ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService(); @@ -544,6 +549,11 @@ && canSelfRegister(context, request, null)) { eperson); ePersonService.update(context, eperson); context.dispatchEvents(); + ClarinUserRegistration clarinUserRegistration = new ClarinUserRegistration(); + clarinUserRegistration.setPersonID(eperson.getID()); + clarinUserRegistration.setOrganization(ClarinUserRegistration.UNKNOWN_USER_REGISTRATION); + clarinUserRegistration.setConfirmation(false); + clarinUserRegistrationService.create(context, clarinUserRegistration); context.restoreAuthSystemState(); context.setCurrentUser(eperson); request.setAttribute(X509_AUTHENTICATED, true); diff --git a/dspace-api/src/main/java/org/dspace/authenticate/clarin/ClarinShibAuthentication.java b/dspace-api/src/main/java/org/dspace/authenticate/clarin/ClarinShibAuthentication.java index ba5d8cd65bfa..754ea098daee 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/clarin/ClarinShibAuthentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/clarin/ClarinShibAuthentication.java @@ -745,20 +745,17 @@ protected EPerson registerNewEPerson(Context context, HttpServletRequest request * Register User in the CLARIN license database * */ - // if no email the registration is postponed after entering and confirming mail - if (Objects.nonNull(email)) { - try { - ClarinUserRegistration clarinUserRegistration = new ClarinUserRegistration(); - clarinUserRegistration.setConfirmation(true); - clarinUserRegistration.setEmail(email); - clarinUserRegistration.setPersonID(eperson.getID()); - clarinUserRegistration.setOrganization(org); - clarinUserRegistrationService.create(context, clarinUserRegistration); - eperson.setCanLogIn(false); - ePersonService.update(context, eperson); - } catch (Exception e) { - throw new AuthorizeException("User has not been added among registred users!") ; - } + try { + ClarinUserRegistration clarinUserRegistration = new ClarinUserRegistration(); + clarinUserRegistration.setConfirmation(true); + clarinUserRegistration.setPersonID(eperson.getID()); + clarinUserRegistration.setOrganization( + Objects.nonNull(org) ? org : ClarinUserRegistration.UNKNOWN_USER_REGISTRATION); + clarinUserRegistrationService.create(context, clarinUserRegistration); + eperson.setCanLogIn(false); + ePersonService.update(context, eperson); + } catch (Exception e) { + throw new AuthorizeException("User has not been added among registred users!") ; } /* CLARIN */ @@ -901,6 +898,19 @@ protected void updateEPerson(Context context, HttpServletRequest request, EPerso } ePersonService.update(context, eperson); context.dispatchEvents(); + + // Sync ClarinUserRegistration organization from the current IdP + String currentOrg = shibheaders.get_idp(); + if (currentOrg != null) { + List registrations = + clarinUserRegistrationService.findByEPersonUUID(context, eperson.getID()); + if (!registrations.isEmpty()) { + ClarinUserRegistration reg = registrations.get(0); + reg.setOrganization(currentOrg); + clarinUserRegistrationService.update(context, reg); + } + } + context.restoreAuthSystemState(); } 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 8c8fd8def9b6..666de3985588 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 @@ -45,12 +45,9 @@ public class ClarinUserRegistration implements ReloadableEntity { allocationSize = 1) protected Integer id; - @Column(name = "eperson_id") + @Column(name = "eperson_id", unique = true) private UUID ePersonID = null; - @Column(name = "email") - private String email = null; - @Column(name = "organization") private String organization = null; @@ -86,14 +83,6 @@ public Integer getID() { return id; } - public String getEmail() { - return email; - } - - public void setEmail(String email) { - this.email = email; - } - public String getOrganization() { return organization; } 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 9be158d376f1..cdd8ac1a94d0 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 @@ -70,7 +70,6 @@ public ClarinUserRegistration create(Context context, "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); @@ -115,9 +114,9 @@ public List findByEPersonUUID(Context context, UUID eper } @Override - public List findByEmail(Context context, String email) + public List findByOrganization(Context context, String organization) throws SQLException { - return clarinUserRegistrationDAO.findByEmail(context, email); + return clarinUserRegistrationDAO.findByOrganization(context, organization); } @Override diff --git a/dspace-api/src/main/java/org/dspace/content/crosswalk/AIPTechMDCrosswalk.java b/dspace-api/src/main/java/org/dspace/content/crosswalk/AIPTechMDCrosswalk.java index 978cabfb4bd6..8738fba16606 100644 --- a/dspace-api/src/main/java/org/dspace/content/crosswalk/AIPTechMDCrosswalk.java +++ b/dspace-api/src/main/java/org/dspace/content/crosswalk/AIPTechMDCrosswalk.java @@ -19,6 +19,9 @@ import org.dspace.content.BitstreamFormat; import org.dspace.content.Collection; import org.dspace.content.Community; +import org.dspace.content.clarin.ClarinUserRegistration; +import org.dspace.content.factory.ClarinServiceFactory; +import org.dspace.content.service.clarin.ClarinUserRegistrationService; import org.dspace.content.DSpaceObject; import org.dspace.content.Item; import org.dspace.content.Site; @@ -89,6 +92,8 @@ public class AIPTechMDCrosswalk implements IngestionCrosswalk, DisseminationCros protected final HandleService handleService = HandleServiceFactory.getInstance().getHandleService(); protected final ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService(); + private final ClarinUserRegistrationService clarinUserRegistrationService = + ClarinServiceFactory.getInstance().getClarinUserRegistration(); /** * Get XML namespaces of the elements this crosswalk may return. @@ -405,6 +410,16 @@ public void ingest(Context context, DSpaceObject dso, List dimList, boo sub.setEmail(value); sub.setCanLogIn(false); ePersonService.update(context, sub); + ClarinUserRegistration clarinUserRegistration = new ClarinUserRegistration(); + clarinUserRegistration.setPersonID(sub.getID()); + clarinUserRegistration.setOrganization( + ClarinUserRegistration.UNKNOWN_USER_REGISTRATION); + clarinUserRegistration.setConfirmation(false); + try { + clarinUserRegistrationService.create(context, clarinUserRegistration); + } catch (AuthorizeException e) { + log.warn("Failed to create ClarinUserRegistration for submitter {}", value, e); + } } else { log.warn( "Ignoring unknown Submitter=" + value + " in AIP Tech MD, no matching EPerson" + 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 1a6a8b1be5d1..0a1aabeadaed 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 @@ -19,5 +19,5 @@ public interface ClarinUserRegistrationDAO extends GenericDAO findByEPersonUUID(Context context, UUID epersonUUID) throws SQLException; - List findByEmail(Context context, String email) throws SQLException; + List findByOrganization(Context context, String organization) 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 0d92bc6eac6a..881f8c83a5a4 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 @@ -36,19 +36,12 @@ 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. + public List findByOrganization(Context context, String organization) + throws SQLException { Query query = createQuery(context, "SELECT cur FROM ClarinUserRegistration as cur " + - "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); + "WHERE cur.organization = :organization"); + + query.setParameter("organization", organization); query.setHint("org.hibernate.cacheable", Boolean.TRUE); return list(query); diff --git a/dspace-api/src/main/java/org/dspace/content/packager/RoleIngester.java b/dspace-api/src/main/java/org/dspace/content/packager/RoleIngester.java index 5c4cf214445e..1437a1367c82 100644 --- a/dspace-api/src/main/java/org/dspace/content/packager/RoleIngester.java +++ b/dspace-api/src/main/java/org/dspace/content/packager/RoleIngester.java @@ -23,6 +23,9 @@ import org.dspace.authorize.AuthorizeException; import org.dspace.content.Collection; import org.dspace.content.Community; +import org.dspace.content.clarin.ClarinUserRegistration; +import org.dspace.content.factory.ClarinServiceFactory; +import org.dspace.content.service.clarin.ClarinUserRegistrationService; import org.dspace.content.DSpaceObject; import org.dspace.content.crosswalk.CrosswalkException; import org.dspace.content.factory.ContentServiceFactory; @@ -60,6 +63,8 @@ public class RoleIngester implements PackageIngester { protected GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); protected EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService(); + private ClarinUserRegistrationService clarinUserRegistrationService = + ClarinServiceFactory.getInstance().getClarinUserRegistration(); /** * Common code to ingest roles from a Document. @@ -195,6 +200,14 @@ void ingestDocument(Context context, DSpaceObject parent, // NOTE: this update() doesn't call a commit(). So, Eperson info // may still be rolled back if a subsequent error occurs ePersonService.update(context, eperson); + if (collider == null) { + // Only create a registration for newly created EPersons, not pre-existing colliders + ClarinUserRegistration clarinUserRegistration = new ClarinUserRegistration(); + clarinUserRegistration.setPersonID(eperson.getID()); + clarinUserRegistration.setOrganization(ClarinUserRegistration.UNKNOWN_USER_REGISTRATION); + clarinUserRegistration.setConfirmation(false); + clarinUserRegistrationService.create(context, clarinUserRegistration); + } } // Now ingest the Groups 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 0434e3f417a9..c6517ea814a5 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,7 +25,8 @@ ClarinUserRegistration create(Context context, List findByEPersonUUID(Context context, UUID epersonUUID) throws SQLException, AuthorizeException; - List findByEmail(Context context, String email) throws SQLException; + List findByOrganization(Context context, String organization) throws SQLException; + void delete(Context context, ClarinUserRegistration clarinUserRegistration) throws SQLException, AuthorizeException; void update(Context context, ClarinUserRegistration clarinUserRegistration) throws SQLException, AuthorizeException; } diff --git a/dspace-api/src/main/java/org/dspace/eperson/EPersonCLITool.java b/dspace-api/src/main/java/org/dspace/eperson/EPersonCLITool.java index fbc16cba90e8..f61f731b616e 100644 --- a/dspace-api/src/main/java/org/dspace/eperson/EPersonCLITool.java +++ b/dspace-api/src/main/java/org/dspace/eperson/EPersonCLITool.java @@ -229,7 +229,6 @@ private static int cmdAdd(Context context, String[] argv) throws AuthorizeExcept ClarinUserRegistration clarinUserRegistration = new ClarinUserRegistration(); clarinUserRegistration.setOrganization(command.getOptionValue(OPT_ORGANIZATION.getOpt())); clarinUserRegistration.setConfirmation(true); - clarinUserRegistration.setEmail(eperson.getEmail()); clarinUserRegistration.setPersonID(eperson.getID()); clarinUserRegistrationService.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 index 6aada8f6ab7e..6fe98cbc81f5 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 @@ -8,3 +8,5 @@ CREATE UNIQUE INDEX IF NOT EXISTS user_registration_eperson_id_unique ON user_registration (eperson_id); + +ALTER TABLE user_registration DROP COLUMN IF EXISTS email; 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 587b881d3c01..08f847fd296d 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 @@ -9,3 +9,5 @@ CREATE UNIQUE INDEX IF NOT EXISTS user_registration_eperson_id_unique ON user_registration (eperson_id) WHERE eperson_id IS NOT NULL; + +ALTER TABLE user_registration DROP COLUMN IF EXISTS email; diff --git a/dspace-api/src/test/java/org/dspace/builder/ClarinUserRegistrationBuilder.java b/dspace-api/src/test/java/org/dspace/builder/ClarinUserRegistrationBuilder.java index 87df549e08bc..8edf182e4b89 100644 --- a/dspace-api/src/test/java/org/dspace/builder/ClarinUserRegistrationBuilder.java +++ b/dspace-api/src/test/java/org/dspace/builder/ClarinUserRegistrationBuilder.java @@ -47,11 +47,6 @@ public ClarinUserRegistrationBuilder withEPersonID(UUID epersonID) { return this; } - public ClarinUserRegistrationBuilder withEmail(String email) { - clarinUserRegistration.setEmail(email); - return this; - } - public ClarinUserRegistrationBuilder withOrganization(String organization) { clarinUserRegistration.setOrganization(organization); return this; diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ClarinUserRegistrationConverter.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ClarinUserRegistrationConverter.java index 7ac59aa1a575..a787ac090984 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ClarinUserRegistrationConverter.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ClarinUserRegistrationConverter.java @@ -22,7 +22,6 @@ public ClarinUserRegistrationRest convert(ClarinUserRegistration modelObject, Pr clarinUserRegistrationRest.setProjection(projection); clarinUserRegistrationRest.setId(modelObject.getID()); clarinUserRegistrationRest.setePersonID(modelObject.getPersonID()); - clarinUserRegistrationRest.setEmail(modelObject.getEmail()); clarinUserRegistrationRest.setOrganization(modelObject.getOrganization()); clarinUserRegistrationRest.setConfirmation(modelObject.isConfirmation()); return clarinUserRegistrationRest; diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/ClarinUserRegistrationRest.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/ClarinUserRegistrationRest.java index efbe80a88b35..d507dbaac254 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/ClarinUserRegistrationRest.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/ClarinUserRegistrationRest.java @@ -35,7 +35,6 @@ public class ClarinUserRegistrationRest extends BaseObjectRest { public static final String USER_METADATA = "userMetadata"; public UUID ePersonID; - public String email; public String organization; public boolean confirmation; @@ -50,14 +49,6 @@ public void setePersonID(UUID ePersonID) { this.ePersonID = ePersonID; } - public String getEmail() { - return email; - } - - public void setEmail(String email) { - this.email = email; - } - public String getOrganization() { return organization; } 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 1b81b63cfc14..53d4724e03c2 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 @@ -155,97 +155,20 @@ public ClarinUserRegistrationRest importUserRegistration(HttpServletRequest requ throw new UnprocessableEntityException("Error parsing request body", e1); } - ClarinUserRegistration clarinUserRegistration = null; - boolean foundByEmail = false; - - // Check if user registration already exists by email or ePersonID - if (StringUtils.isNotBlank(userRegistrationRest.getEmail())) { - List existingRegistrations = clarinUserRegistrationService.findByEmail(context, - userRegistrationRest.getEmail()); - if (!existingRegistrations.isEmpty()) { - clarinUserRegistration = existingRegistrations.get(0); - foundByEmail = true; - } + if (Objects.isNull(userRegistrationRest.getePersonID())) { + throw new UnprocessableEntityException("Cannot import user registration: ePersonID must not be null."); } - // If not found by email, try by ePersonID - if (Objects.isNull(clarinUserRegistration) && Objects.nonNull(userRegistrationRest.getePersonID())) { - List existingRegistrationsByEPerson = - clarinUserRegistrationService.findByEPersonUUID(context, - userRegistrationRest.getePersonID()); - if (!existingRegistrationsByEPerson.isEmpty()) { - clarinUserRegistration = existingRegistrationsByEPerson.get(0); - } - } + // Look up existing registration by ePersonID + List existingRegistrationsByEPerson = + clarinUserRegistrationService.findByEPersonUUID(context, userRegistrationRest.getePersonID()); + ClarinUserRegistration clarinUserRegistration = + existingRegistrationsByEPerson.isEmpty() ? null : existingRegistrationsByEPerson.get(0); if (Objects.nonNull(clarinUserRegistration)) { - // Update existing registration if values are different - boolean needsUpdate = false; - + // Update organization if the value has changed if (!Objects.equals(clarinUserRegistration.getOrganization(), userRegistrationRest.getOrganization())) { clarinUserRegistration.setOrganization(userRegistrationRest.getOrganization()); - needsUpdate = true; - } - - if (!Objects.equals(clarinUserRegistration.isConfirmation(), userRegistrationRest.isConfirmation())) { - clarinUserRegistration.setConfirmation(userRegistrationRest.isConfirmation()); - needsUpdate = true; - } - - // Do not update email if registration was matched by ePersonID instead of email - // to prevent data inconsistency - if (!Objects.equals(clarinUserRegistration.getEmail(), userRegistrationRest.getEmail())) { - if (foundByEmail) { - clarinUserRegistration.setEmail(userRegistrationRest.getEmail()); - needsUpdate = true; - } else { - // Registration found by ePersonID but email differs - potential data corruption - log.warn("User registration found by ePersonID={} has different email. " + - "Existing email='{}', incoming email='{}'. " + - "Email will NOT be updated to prevent data inconsistency.", - userRegistrationRest.getePersonID(), - clarinUserRegistration.getEmail(), - userRegistrationRest.getEmail()); - } - } - - if (!Objects.equals(clarinUserRegistration.getPersonID(), userRegistrationRest.getePersonID())) { - boolean canUpdate = true; - - // Check for ePersonID conflicts if the incoming value is non-null - if (Objects.nonNull(userRegistrationRest.getePersonID())) { - List conflictingRegs = clarinUserRegistrationService - .findByEPersonUUID(context, userRegistrationRest.getePersonID()); - if (!conflictingRegs.isEmpty() && - !conflictingRegs.get(0).getID().equals(clarinUserRegistration.getID())) { - // Conflict detected - log appropriate message based on how registration was found - if (foundByEmail) { - log.warn("User registration found by email='{}' has different ePersonID. " + - "Incoming ePersonID={} is already associated with another registration ID={}. " + - "ePersonID will NOT be updated to prevent data inconsistency.", - clarinUserRegistration.getEmail(), - userRegistrationRest.getePersonID(), - conflictingRegs.get(0).getID()); - } else { - log.warn("User registration found by ePersonID={} has different incoming ePersonID. " + - "Incoming ePersonID={} is already associated with another registration ID={}. " + - "ePersonID will NOT be updated to prevent data inconsistency.", - clarinUserRegistration.getPersonID(), - userRegistrationRest.getePersonID(), - conflictingRegs.get(0).getID()); - } - canUpdate = false; - } - } - - // Update ePersonID if no conflict was detected - if (canUpdate) { - clarinUserRegistration.setPersonID(userRegistrationRest.getePersonID()); - needsUpdate = true; - } - } - - if (needsUpdate) { clarinUserRegistrationService.update(context, clarinUserRegistration); } } else { @@ -253,7 +176,6 @@ public ClarinUserRegistrationRest importUserRegistration(HttpServletRequest requ clarinUserRegistration = new ClarinUserRegistration(); clarinUserRegistration.setOrganization(userRegistrationRest.getOrganization()); clarinUserRegistration.setConfirmation(userRegistrationRest.isConfirmation()); - clarinUserRegistration.setEmail(userRegistrationRest.getEmail()); clarinUserRegistration.setPersonID(userRegistrationRest.getePersonID()); clarinUserRegistration = clarinUserRegistrationService.create(context, clarinUserRegistration); } 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 31101e97f3c7..1902bf39fcaf 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 @@ -519,7 +519,7 @@ public List processNonSignedInUser(Context context, // Get anonymous user registration - user metadata should not have `user_registration_id = null` ClarinUserRegistration clarinUserRegistration = null; List clarinUserRegistrationList = clarinUserRegistrationService - .findByEmail(context, ANONYMOUS_USER_REGISTRATION); + .findByOrganization(context, ANONYMOUS_USER_REGISTRATION); for (ClarinUserRegistration fetchedClarinuserRegistration : clarinUserRegistrationList) { if (!StringUtils.equals(fetchedClarinuserRegistration.getOrganization(), ANONYMOUS_USER_REGISTRATION)) { continue; diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinUserRegistrationRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinUserRegistrationRestRepository.java index fc6727ae83ef..77a0fd8a6463 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinUserRegistrationRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinUserRegistrationRestRepository.java @@ -1,3 +1,4 @@ + /** * 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 diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EPersonRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EPersonRestRepository.java index b213b6d1eabb..16d2eb1ddf88 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EPersonRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EPersonRestRepository.java @@ -152,7 +152,6 @@ private EPerson createEPersonFromRestObject(Context context, EPersonRest eperson ClarinUserRegistration clarinUserRegistration = new ClarinUserRegistration(); clarinUserRegistration.setOrganization(UNKNOWN_USER_REGISTRATION); clarinUserRegistration.setConfirmation(true); - clarinUserRegistration.setEmail(eperson.getEmail()); clarinUserRegistration.setPersonID(eperson.getID()); clarinUserRegistrationService.create(context, clarinUserRegistration); } catch (SQLException e) { 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 530880961b50..1cffada3af80 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 @@ -75,11 +75,10 @@ private EPerson createTestEPerson(String emailSuffix, String password) throws Ex /** * Helper method to create an initial ClarinUserRegistration for testing. */ - private ClarinUserRegistration createInitialRegistration(EPerson ePerson, String email, String org) + private ClarinUserRegistration createInitialRegistration(EPerson ePerson, String org) throws Exception { return ClarinUserRegistrationBuilder .createClarinUserRegistration(context) - .withEmail(email) .withEPersonID(ePerson.getID()) .withOrganization(org) .withConfirmation(false) @@ -89,10 +88,9 @@ private ClarinUserRegistration createInitialRegistration(EPerson ePerson, String /** * Helper method to build a ClarinUserRegistrationRest import request. */ - private ClarinUserRegistrationRest buildImportRequest(String email, UUID ePersonID, String org, + private ClarinUserRegistrationRest buildImportRequest(UUID ePersonID, String org, boolean confirmation) { ClarinUserRegistrationRest request = new ClarinUserRegistrationRest(); - request.setEmail(email); request.setePersonID(ePersonID); request.setOrganization(org); request.setConfirmation(confirmation); @@ -215,7 +213,6 @@ public void createUserRegistrationTest() throws Exception { context.restoreAuthSystemState(); ClarinUserRegistrationRest userRegistrationRest = new ClarinUserRegistrationRest(); userRegistrationRest.setConfirmation(true); - userRegistrationRest.setEmail("test@test.edu"); userRegistrationRest.setePersonID(ePerson.getID()); userRegistrationRest.setOrganization("Test"); @@ -236,7 +233,6 @@ public void createUserRegistrationTest() throws Exception { ClarinUserRegistration clarinUserRegistration = clarinUserRegistrationService.find(context, idRef.get()); context.setCurrentUser(currentUser); assertTrue(clarinUserRegistration.isConfirmation()); - assertEquals(clarinUserRegistration.getEmail(), "test@test.edu"); assertEquals(clarinUserRegistration.getPersonID(), ePerson.getID()); assertEquals(clarinUserRegistration.getOrganization(), "Test"); } finally { @@ -245,78 +241,67 @@ public void createUserRegistrationTest() throws Exception { } @Test - public void updatesExistingRegistrationWhenMatchedByEmail() throws Exception { + public void updatesExistingRegistrationByEPersonID() throws Exception { context.turnOffAuthorisationSystem(); EPerson ePerson = createTestEPerson("4", "qwerty04"); - ClarinUserRegistration initialRegistration = createInitialRegistration(ePerson, "user@test.edu", - "Original Org"); + ClarinUserRegistration initialRegistration = createInitialRegistration(ePerson, "Original Org"); context.restoreAuthSystemState(); - // Import with same email to match by email - ClarinUserRegistrationRest request = buildImportRequest("user@test.edu", ePerson.getID(), - "Updated Org", true); + // Import with same ePersonID to match existing registration + ClarinUserRegistrationRest request = buildImportRequest(ePerson.getID(), "Updated Org", true); String authToken = getAuthToken(admin.getEmail(), password); try { performImportRequest(authToken, request); - // Verify updates were applied + // Verify organization was updated ClarinUserRegistration updated = findAsAdmin(initialRegistration.getID()); - assertEquals("user@test.edu", updated.getEmail()); assertEquals("Updated Org", updated.getOrganization()); - assertTrue(updated.isConfirmation()); } finally { ClarinUserRegistrationBuilder.deleteClarinUserRegistration(initialRegistration.getID()); } } @Test - public void preventsEmailUpdateWhenMatchedByEPersonID() throws Exception { + public void updatesOrganizationWhenMatchedByEPersonID() throws Exception { context.turnOffAuthorisationSystem(); EPerson ePerson = createTestEPerson("5", "qwerty05"); - ClarinUserRegistration initialRegistration = createInitialRegistration(ePerson, "existing@test.edu", - "Original Org"); + ClarinUserRegistration initialRegistration = createInitialRegistration(ePerson, "Original Org"); context.restoreAuthSystemState(); - // Import with different email - will match by ePersonID instead - ClarinUserRegistrationRest request = buildImportRequest("different@test.edu", ePerson.getID(), - "Updated Org", true); + // Import with same ePersonID and different organization + ClarinUserRegistrationRest request = buildImportRequest(ePerson.getID(), "Updated Org", true); String authToken = getAuthToken(admin.getEmail(), password); try { performImportRequest(authToken, request); - // Verify email was NOT updated but other fields were + // Verify only organization was updated ClarinUserRegistration updated = findAsAdmin(initialRegistration.getID()); - assertEquals("existing@test.edu", updated.getEmail()); // Email unchanged - assertEquals("Updated Org", updated.getOrganization()); // Organization updated - assertTrue(updated.isConfirmation()); // Confirmation updated + assertEquals("Updated Org", updated.getOrganization()); + assertFalse(updated.isConfirmation()); } finally { ClarinUserRegistrationBuilder.deleteClarinUserRegistration(initialRegistration.getID()); } } @Test - public void updatesRegistrationWhenBothEmailAndEPersonIDMatch() throws Exception { + public void updatesRegistrationWhenMatchedByEPersonID() throws Exception { context.turnOffAuthorisationSystem(); EPerson ePerson = createTestEPerson("6", "qwerty06"); - ClarinUserRegistration initialRegistration = createInitialRegistration(ePerson, "consistent@test.edu", - "Original Org"); + ClarinUserRegistration initialRegistration = createInitialRegistration(ePerson, "Original Org"); context.restoreAuthSystemState(); - // Import with same email and ePersonID - both match the same record - ClarinUserRegistrationRest request = buildImportRequest("consistent@test.edu", ePerson.getID(), - "Updated Org", true); + // Import with same ePersonID + ClarinUserRegistrationRest request = buildImportRequest(ePerson.getID(), "Updated Org", true); String authToken = getAuthToken(admin.getEmail(), password); try { performImportRequest(authToken, request); - // Verify all fields were updated (happy path) + // Verify organization was updated ClarinUserRegistration updated = findAsAdmin(initialRegistration.getID()); - assertEquals("consistent@test.edu", updated.getEmail()); assertEquals("Updated Org", updated.getOrganization()); - assertTrue(updated.isConfirmation()); } finally { ClarinUserRegistrationBuilder.deleteClarinUserRegistration(initialRegistration.getID()); } 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 285c98a7bd06..7ac3364a6705 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 @@ -58,7 +58,6 @@ public void createShouldUpdateExistingRegistrationForSameEPersonId() throws Exce .build(); ClarinUserRegistration first = new ClarinUserRegistration(); - first.setEmail("duptest@example.com"); first.setPersonID(testPerson.getID()); first.setOrganization("OrgA"); first.setConfirmation(false); @@ -68,10 +67,8 @@ public void createShouldUpdateExistingRegistrationForSameEPersonId() throws Exce 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); @@ -82,9 +79,8 @@ public void createShouldUpdateExistingRegistrationForSameEPersonId() throws Exce assertEquals("Should return the existing registration ID, not a new one", firstId, result.getID()); - // Fields should be updated to the new values + // Organization should be updated to the new value 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 @@ -113,13 +109,11 @@ public void createShouldAllowDifferentEPersonIds() throws Exception { .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); @@ -147,13 +141,11 @@ 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); diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java index e1febcfa0fe0..b9ffc6101682 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java @@ -173,8 +173,6 @@ public void createTest() throws Exception { .andExpect(jsonPath("$.page.totalElements", is(1))) .andExpect(jsonPath( "$._embedded.clarinuserregistrations[0].id", is(not(empty())))) - .andExpect(jsonPath( - "$._embedded.clarinuserregistrations[0].email", is("createtest@example.com"))) .andExpect(jsonPath( "$._embedded.clarinuserregistrations[0].confirmation", is(true))) .andExpect(jsonPath( @@ -191,9 +189,6 @@ public void createTest() throws Exception { .andExpect(jsonPath("$.page.totalElements", is(1))) .andExpect(jsonPath( "$._embedded.clarinuserregistrations[0].id", is(not(empty())))) - .andExpect(jsonPath( - "$._embedded.clarinuserregistrations[0].email", - is("createtestfull@example.com"))) .andExpect(jsonPath( "$._embedded.clarinuserregistrations[0].confirmation", is(true))) .andExpect(jsonPath( From 95d695198e7ef7621bc6507bd8a26ca357af39c7 Mon Sep 17 00:00:00 2001 From: Paurikova2 Date: Wed, 25 Mar 2026 12:25:26 +0100 Subject: [PATCH 6/9] fix import ordering --- .../java/org/dspace/authenticate/LDAPAuthentication.java | 3 ++- .../org/dspace/content/crosswalk/AIPTechMDCrosswalk.java | 6 +++--- .../main/java/org/dspace/content/packager/RoleIngester.java | 6 +++--- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/authenticate/LDAPAuthentication.java b/dspace-api/src/main/java/org/dspace/authenticate/LDAPAuthentication.java index fa1e026879c9..3b3b9569a0eb 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/LDAPAuthentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/LDAPAuthentication.java @@ -351,7 +351,8 @@ public int authenticate(Context context, context.dispatchEvents(); ClarinUserRegistration clarinUserRegistration = new ClarinUserRegistration(); clarinUserRegistration.setPersonID(eperson.getID()); - clarinUserRegistration.setOrganization(ClarinUserRegistration.UNKNOWN_USER_REGISTRATION); + clarinUserRegistration.setOrganization( + ClarinUserRegistration.UNKNOWN_USER_REGISTRATION); clarinUserRegistration.setConfirmation(false); clarinUserRegistrationService.create(context, clarinUserRegistration); context.setCurrentUser(eperson); diff --git a/dspace-api/src/main/java/org/dspace/content/crosswalk/AIPTechMDCrosswalk.java b/dspace-api/src/main/java/org/dspace/content/crosswalk/AIPTechMDCrosswalk.java index 8738fba16606..58d8f2a8246a 100644 --- a/dspace-api/src/main/java/org/dspace/content/crosswalk/AIPTechMDCrosswalk.java +++ b/dspace-api/src/main/java/org/dspace/content/crosswalk/AIPTechMDCrosswalk.java @@ -19,13 +19,12 @@ import org.dspace.content.BitstreamFormat; import org.dspace.content.Collection; import org.dspace.content.Community; -import org.dspace.content.clarin.ClarinUserRegistration; -import org.dspace.content.factory.ClarinServiceFactory; -import org.dspace.content.service.clarin.ClarinUserRegistrationService; import org.dspace.content.DSpaceObject; import org.dspace.content.Item; import org.dspace.content.Site; +import org.dspace.content.clarin.ClarinUserRegistration; import org.dspace.content.dto.MetadataValueDTO; +import org.dspace.content.factory.ClarinServiceFactory; import org.dspace.content.factory.ContentServiceFactory; import org.dspace.content.packager.DSpaceAIPIngester; import org.dspace.content.packager.METSManifest; @@ -34,6 +33,7 @@ import org.dspace.content.service.CollectionService; import org.dspace.content.service.ItemService; import org.dspace.content.service.SiteService; +import org.dspace.content.service.clarin.ClarinUserRegistrationService; import org.dspace.core.Constants; import org.dspace.core.Context; import org.dspace.eperson.EPerson; diff --git a/dspace-api/src/main/java/org/dspace/content/packager/RoleIngester.java b/dspace-api/src/main/java/org/dspace/content/packager/RoleIngester.java index 1437a1367c82..8765cdd649fa 100644 --- a/dspace-api/src/main/java/org/dspace/content/packager/RoleIngester.java +++ b/dspace-api/src/main/java/org/dspace/content/packager/RoleIngester.java @@ -23,15 +23,15 @@ import org.dspace.authorize.AuthorizeException; import org.dspace.content.Collection; import org.dspace.content.Community; -import org.dspace.content.clarin.ClarinUserRegistration; -import org.dspace.content.factory.ClarinServiceFactory; -import org.dspace.content.service.clarin.ClarinUserRegistrationService; import org.dspace.content.DSpaceObject; +import org.dspace.content.clarin.ClarinUserRegistration; import org.dspace.content.crosswalk.CrosswalkException; +import org.dspace.content.factory.ClarinServiceFactory; import org.dspace.content.factory.ContentServiceFactory; import org.dspace.content.service.CollectionService; import org.dspace.content.service.CommunityService; import org.dspace.content.service.DSpaceObjectService; +import org.dspace.content.service.clarin.ClarinUserRegistrationService; import org.dspace.core.Constants; import org.dspace.core.Context; import org.dspace.eperson.EPerson; From d8231d7265fea591d90f2863d59f93658456e9db Mon Sep 17 00:00:00 2001 From: Paurikova2 Date: Tue, 31 Mar 2026 09:47:15 +0200 Subject: [PATCH 7/9] added confirmation updating, removed empty line, removed unique = true --- .../org/dspace/content/clarin/ClarinUserRegistration.java | 2 +- .../rest/repository/ClarinEPersonImportController.java | 8 ++++++++ .../repository/ClarinUserRegistrationRestRepository.java | 1 - 3 files changed, 9 insertions(+), 2 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 666de3985588..e14c58fd05bc 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 @@ -45,7 +45,7 @@ public class ClarinUserRegistration implements ReloadableEntity { allocationSize = 1) protected Integer id; - @Column(name = "eperson_id", unique = true) + @Column(name = "eperson_id") private UUID ePersonID = null; @Column(name = "organization") 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 53d4724e03c2..71c056540c4a 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 @@ -167,8 +167,16 @@ public ClarinUserRegistrationRest importUserRegistration(HttpServletRequest requ if (Objects.nonNull(clarinUserRegistration)) { // Update organization if the value has changed + boolean requiresUpdate = false; if (!Objects.equals(clarinUserRegistration.getOrganization(), userRegistrationRest.getOrganization())) { clarinUserRegistration.setOrganization(userRegistrationRest.getOrganization()); + requiresUpdate = true; + } + if (clarinUserRegistration.isConfirmation() != userRegistrationRest.isConfirmation()) { + clarinUserRegistration.setConfirmation(userRegistrationRest.isConfirmation()); + requiresUpdate = true; + } + if (requiresUpdate) { clarinUserRegistrationService.update(context, clarinUserRegistration); } } else { diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinUserRegistrationRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinUserRegistrationRestRepository.java index 77a0fd8a6463..fc6727ae83ef 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinUserRegistrationRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinUserRegistrationRestRepository.java @@ -1,4 +1,3 @@ - /** * 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 From 93a332d2a83a469a2b38e9e3473b6ac2d6aec25d Mon Sep 17 00:00:00 2001 From: Paurikova2 Date: Tue, 31 Mar 2026 13:38:29 +0200 Subject: [PATCH 8/9] fix tests, remove user_registration deletion during import, added missing control --- .../ClarinEPersonImportController.java | 36 ++- .../rest/ClarinEPersonImportControllerIT.java | 8 +- .../ClarinUserRegistrationServiceImplIT.java | 205 ++++++++++-------- 3 files changed, 148 insertions(+), 101 deletions(-) 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 71c056540c4a..3bc4a9745cc7 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,14 +103,6 @@ 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 @@ -166,7 +158,7 @@ public ClarinUserRegistrationRest importUserRegistration(HttpServletRequest requ existingRegistrationsByEPerson.isEmpty() ? null : existingRegistrationsByEPerson.get(0); if (Objects.nonNull(clarinUserRegistration)) { - // Update organization if the value has changed + // Update organization and registration if the values have changed boolean requiresUpdate = false; if (!Objects.equals(clarinUserRegistration.getOrganization(), userRegistrationRest.getOrganization())) { clarinUserRegistration.setOrganization(userRegistrationRest.getOrganization()); @@ -176,6 +168,32 @@ public ClarinUserRegistrationRest importUserRegistration(HttpServletRequest requ clarinUserRegistration.setConfirmation(userRegistrationRest.isConfirmation()); requiresUpdate = true; } + if (!Objects.equals(clarinUserRegistration.getPersonID(), userRegistrationRest.getePersonID())) { + boolean canUpdate = true; + + // Check for ePersonID conflicts if the incoming value is non-null + if (Objects.nonNull(userRegistrationRest.getePersonID())) { + List conflictingRegs = clarinUserRegistrationService + .findByEPersonUUID(context, userRegistrationRest.getePersonID()); + if (!conflictingRegs.isEmpty() && + !conflictingRegs.get(0).getID().equals(clarinUserRegistration.getID())) { + // Conflict detected - log appropriate message based on how registration was found + log.warn("User registration found by ePersonID={} has different incoming ePersonID. " + + "Incoming ePersonID={} is already associated with another registration ID={}. " + + "ePersonID will NOT be updated to prevent data inconsistency.", + clarinUserRegistration.getPersonID(), + userRegistrationRest.getePersonID(), + conflictingRegs.get(0).getID()); + canUpdate = false; + } + } + // Update ePersonID if no conflict was detected + if (canUpdate) { + clarinUserRegistration.setPersonID(userRegistrationRest.getePersonID()); + requiresUpdate = true; + } + } + // Update ePersonID if no conflict was detected if (requiresUpdate) { clarinUserRegistrationService.update(context, clarinUserRegistration); } 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 1cffada3af80..6d45ca26143c 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 @@ -279,7 +279,7 @@ public void updatesOrganizationWhenMatchedByEPersonID() throws Exception { // Verify only organization was updated ClarinUserRegistration updated = findAsAdmin(initialRegistration.getID()); assertEquals("Updated Org", updated.getOrganization()); - assertFalse(updated.isConfirmation()); + assertTrue(updated.isConfirmation()); } finally { ClarinUserRegistrationBuilder.deleteClarinUserRegistration(initialRegistration.getID()); } @@ -335,7 +335,11 @@ public void importEpersonDoesNotCreateUserRegistration() throws Exception { clarinUserRegistrationService.findByEPersonUUID(context, idRef.get()); context.setCurrentUser(currentUser); - assertTrue("No user registration should be created on EPerson import", registrations.isEmpty()); + assertEquals("Exactly one user registration should be created on EPerson import", + 1, registrations.size()); + + ClarinUserRegistration reg = registrations.get(0); + assertEquals(idRef.get(), reg.getPersonID()); } finally { EPersonBuilder.deleteEPerson(idRef.get()); } 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 7ac3364a6705..554b68947d33 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 @@ -51,45 +51,46 @@ public void testFind() throws Exception { @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.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()); - - ClarinUserRegistration second = new ClarinUserRegistration(); - 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()); - - // Organization should be updated to the new value - assertEquals("OrgB", result.getOrganization()); - 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(); + try { + EPerson testPerson = EPersonBuilder.createEPerson(context) + .withEmail("duptest@example.com") + .withPassword("password123") + .build(); + + ClarinUserRegistration first = new ClarinUserRegistration(); + 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()); + + ClarinUserRegistration second = new ClarinUserRegistration(); + 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()); + + // Organization should be updated to the new value + assertEquals("OrgB", result.getOrganization()); + 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()); + } finally { + context.restoreAuthSystemState(); + } } /** @@ -98,38 +99,49 @@ public void createShouldUpdateExistingRegistrationForSameEPersonId() throws Exce @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.setPersonID(personA.getID()); - regA.setOrganization("OrgA"); - regA.setConfirmation(true); - - ClarinUserRegistration regB = new ClarinUserRegistration(); - 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(); + ClarinUserRegistration createdA = null; + ClarinUserRegistration createdB = null; + + try { + 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.setPersonID(personA.getID()); + regA.setOrganization("OrgA"); + regA.setConfirmation(true); + + ClarinUserRegistration regB = new ClarinUserRegistration(); + regB.setPersonID(personB.getID()); + regB.setOrganization("OrgB"); + regB.setConfirmation(true); + + createdA = clarinUserRegistrationService.create(context, regA); + 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()); + } finally { + // cleanup + if (createdA != null) { + clarinUserRegistrationService.delete(context, createdA); + } + if (createdB != null) { + clarinUserRegistrationService.delete(context, createdB); + } + context.restoreAuthSystemState(); + } } /** @@ -140,24 +152,37 @@ public void createShouldAllowDifferentEPersonIds() throws Exception { public void createShouldAllowMultipleNullEPersonIds() throws Exception { context.turnOffAuthorisationSystem(); - ClarinUserRegistration anon1 = new ClarinUserRegistration(); - anon1.setOrganization("Unknown"); - anon1.setConfirmation(true); - // personID is null by default - - ClarinUserRegistration anon2 = new ClarinUserRegistration(); - 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(); + ClarinUserRegistration created1 = null; + ClarinUserRegistration created2 = null; + + try { + ClarinUserRegistration anon1 = new ClarinUserRegistration(); + anon1.setOrganization("Unknown"); + anon1.setConfirmation(true); + // personID is null by default + + ClarinUserRegistration anon2 = new ClarinUserRegistration(); + anon2.setOrganization("Unknown"); + anon2.setConfirmation(true); + + created1 = clarinUserRegistrationService.create(context, anon1); + 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())); + } finally { + // cleanup + if (created1 != null) { + clarinUserRegistrationService.delete(context, created1); + } + if (created2 != null) { + clarinUserRegistrationService.delete(context, created2); + } + + context.restoreAuthSystemState(); + } } } From 0ae9ca9bd517c3a584465efd25ac03f5bfd7d794 Mon Sep 17 00:00:00 2001 From: Paurikova2 Date: Tue, 31 Mar 2026 13:55:15 +0200 Subject: [PATCH 9/9] checkstyle --- .../ClarinUserRegistrationServiceImplIT.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 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 554b68947d33..9e14a00d9b26 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 @@ -56,33 +56,33 @@ public void createShouldUpdateExistingRegistrationForSameEPersonId() throws Exce .withEmail("duptest@example.com") .withPassword("password123") .build(); - + ClarinUserRegistration first = new ClarinUserRegistration(); 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()); - + ClarinUserRegistration second = new ClarinUserRegistration(); 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()); - + // Organization should be updated to the new value assertEquals("OrgB", result.getOrganization()); 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()); @@ -111,20 +111,20 @@ public void createShouldAllowDifferentEPersonIds() throws Exception { .withEmail("personB@example.com") .withPassword("password123") .build(); - + ClarinUserRegistration regA = new ClarinUserRegistration(); regA.setPersonID(personA.getID()); regA.setOrganization("OrgA"); regA.setConfirmation(true); - + ClarinUserRegistration regB = new ClarinUserRegistration(); regB.setPersonID(personB.getID()); regB.setOrganization("OrgB"); regB.setConfirmation(true); - + createdA = clarinUserRegistrationService.create(context, regA); createdB = clarinUserRegistrationService.create(context, regB); - + assertNotNull(createdA); assertNotNull(createdB); // They must be distinct rows @@ -160,14 +160,14 @@ public void createShouldAllowMultipleNullEPersonIds() throws Exception { anon1.setOrganization("Unknown"); anon1.setConfirmation(true); // personID is null by default - + ClarinUserRegistration anon2 = new ClarinUserRegistration(); anon2.setOrganization("Unknown"); anon2.setConfirmation(true); created1 = clarinUserRegistrationService.create(context, anon1); created2 = clarinUserRegistrationService.create(context, anon2); - + assertNotNull(created1); assertNotNull(created2); // Both anonymous, so each should get its own row