Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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<ClarinUserRegistration> 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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,18 @@ public List<ClarinUserRegistration> findByEPersonUUID(Context context, UUID eper

@Override
public List<ClarinUserRegistration> 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);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
--
-- The contents of this file are subject to the license and copyright
-- detailed in the LICENSE and NOTICE files at the root of the source
-- tree and available online at
--
-- http://www.dspace.org/license/
--

CREATE UNIQUE INDEX IF NOT EXISTS user_registration_eperson_id_unique
ON user_registration (eperson_id);
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
--
-- The contents of this file are subject to the license and copyright
-- detailed in the LICENSE and NOTICE files at the root of the source
-- tree and available online at
--
-- http://www.dspace.org/license/
--

CREATE UNIQUE INDEX IF NOT EXISTS user_registration_eperson_id_unique
ON user_registration (eperson_id)
WHERE eperson_id IS NOT NULL;
Original file line number Diff line number Diff line change
Expand Up @@ -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<ClarinUserRegistration> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<UUID> 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<ClarinUserRegistration> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -36,4 +43,129 @@ public void testFind() throws Exception {
.find(context, clarinUserRegistration.getID()));
context.setCurrentUser(currentUser);
}

/**
* Verify that calling create() with the same eperson_id twice does not produce a duplicate row
* but instead updates the existing registration and returns it.
*/
@Test
public void createShouldUpdateExistingRegistrationForSameEPersonId() throws Exception {
context.turnOffAuthorisationSystem();

EPerson testPerson = EPersonBuilder.createEPerson(context)
.withEmail("duptest@example.com")
.withPassword("password123")
.build();

ClarinUserRegistration first = new ClarinUserRegistration();
first.setEmail("duptest@example.com");
first.setPersonID(testPerson.getID());
first.setOrganization("OrgA");
first.setConfirmation(false);

ClarinUserRegistration created = clarinUserRegistrationService.create(context, first);
assertNotNull("First create should return a non-null registration", created);
Integer firstId = created.getID();
assertNotNull("Created registration should have an ID", firstId);
assertEquals("OrgA", created.getOrganization());
assertEquals("duptest@example.com", created.getEmail());

ClarinUserRegistration second = new ClarinUserRegistration();
second.setEmail("duptest-updated@example.com");
second.setPersonID(testPerson.getID());
second.setOrganization("OrgB");
second.setConfirmation(true);

ClarinUserRegistration result = clarinUserRegistrationService.create(context, second);
assertNotNull("Second create should return a non-null registration", result);

assertEquals("Should return the existing registration ID, not a new one",
firstId, result.getID());

// Fields should be updated to the new values
assertEquals("OrgB", result.getOrganization());
assertEquals("duptest-updated@example.com", result.getEmail());
assertTrue("Confirmation should be updated to true", result.isConfirmation());

// Verify only one row exists in the database for this eperson_id
List<ClarinUserRegistration> allForEPerson =
clarinUserRegistrationService.findByEPersonUUID(context, testPerson.getID());
assertEquals("There must be exactly one registration for this eperson_id",
1, allForEPerson.size());

context.restoreAuthSystemState();
}

/**
* Verify that creating registrations for different ePersons works
*/
@Test
public void createShouldAllowDifferentEPersonIds() throws Exception {
context.turnOffAuthorisationSystem();

EPerson personA = EPersonBuilder.createEPerson(context)
.withEmail("personA@example.com")
.withPassword("password123")
.build();
EPerson personB = EPersonBuilder.createEPerson(context)
.withEmail("personB@example.com")
.withPassword("password123")
.build();

ClarinUserRegistration regA = new ClarinUserRegistration();
regA.setEmail("personA@example.com");
regA.setPersonID(personA.getID());
regA.setOrganization("OrgA");
regA.setConfirmation(true);

ClarinUserRegistration regB = new ClarinUserRegistration();
regB.setEmail("personB@example.com");
regB.setPersonID(personB.getID());
regB.setOrganization("OrgB");
regB.setConfirmation(true);

ClarinUserRegistration createdA = clarinUserRegistrationService.create(context, regA);
ClarinUserRegistration createdB = clarinUserRegistrationService.create(context, regB);

assertNotNull(createdA);
assertNotNull(createdB);
// They must be distinct rows
assertTrue("Registrations for different ePersons must have different IDs",
!createdA.getID().equals(createdB.getID()));
assertEquals("OrgA", createdA.getOrganization());
assertEquals("OrgB", createdB.getOrganization());

context.restoreAuthSystemState();
}

/**
* Verify that creating a registration with a null eperson_id (anonymous) does not trigger
* the dedup guard and creates a new row every time.
*/
@Test
public void createShouldAllowMultipleNullEPersonIds() throws Exception {
context.turnOffAuthorisationSystem();

ClarinUserRegistration anon1 = new ClarinUserRegistration();
anon1.setEmail("anonymous");
anon1.setOrganization("Unknown");
anon1.setConfirmation(true);
// personID is null by default

ClarinUserRegistration anon2 = new ClarinUserRegistration();
anon2.setEmail("anonymous");
anon2.setOrganization("Unknown");
anon2.setConfirmation(true);

ClarinUserRegistration created1 = clarinUserRegistrationService.create(context, anon1);
ClarinUserRegistration created2 = clarinUserRegistrationService.create(context, anon2);

assertNotNull(created1);
assertNotNull(created2);
// Both anonymous, so each should get its own row
assertTrue("Null-eperson registrations should create separate rows",
!created1.getID().equals(created2.getID()));

context.restoreAuthSystemState();
}
}
Loading