Skip to content

Commit

Permalink
[kbss-cvut/record-manager-ui#38] Set imported record provenance accor…
Browse files Browse the repository at this point in the history
…ding to acceptance criteria.

Importing as admin preserves imported record provenance, as regular user replaces original author/created with current user/datetime.
  • Loading branch information
ledsoft committed Dec 22, 2023
1 parent ee9872d commit dce1d6d
Show file tree
Hide file tree
Showing 7 changed files with 146 additions and 25 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package cz.cvut.kbss.study.exception;

/**
* Indicates that the application is attempting to import a record with a nonexistent author.
*/
public class RecordAuthorNotFoundException extends RecordManagerException {

public RecordAuthorNotFoundException(String message) {
super(message);
}
}
20 changes: 19 additions & 1 deletion src/main/java/cz/cvut/kbss/study/model/User.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
package cz.cvut.kbss.study.model;

import com.fasterxml.jackson.annotation.JsonProperty;
import cz.cvut.kbss.jopa.model.annotations.*;
import cz.cvut.kbss.jopa.model.annotations.FetchType;
import cz.cvut.kbss.jopa.model.annotations.Id;
import cz.cvut.kbss.jopa.model.annotations.OWLClass;
import cz.cvut.kbss.jopa.model.annotations.OWLDataProperty;
import cz.cvut.kbss.jopa.model.annotations.OWLObjectProperty;
import cz.cvut.kbss.jopa.model.annotations.ParticipationConstraints;
import cz.cvut.kbss.jopa.model.annotations.Types;
import cz.cvut.kbss.study.model.util.HasDerivableUri;
import cz.cvut.kbss.study.util.Constants;
import cz.cvut.kbss.study.util.IdentificationUtils;
Expand Down Expand Up @@ -148,6 +154,18 @@ public void addType(String type) {
getTypes().add(type);
}

/**
* Returns true if this user is an admin.
* <p>
* That is, it has an admin type.
*
* @return {@code true} if this is admin, {@code false} otherwise
*/
public boolean isAdmin() {
assert types != null;
return getTypes().contains(Vocabulary.s_c_administrator);
}

public String getToken() {
return token;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
package cz.cvut.kbss.study.rest.handler;

import cz.cvut.kbss.study.exception.*;
import cz.cvut.kbss.study.exception.EntityExistsException;
import cz.cvut.kbss.study.exception.NotFoundException;
import cz.cvut.kbss.study.exception.PersistenceException;
import cz.cvut.kbss.study.exception.RecordAuthorNotFoundException;
import cz.cvut.kbss.study.exception.ValidationException;
import cz.cvut.kbss.study.exception.WebServiceIntegrationException;
import jakarta.servlet.http.HttpServletRequest;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -83,13 +88,14 @@ public ResponseEntity<ErrorInfo> persistenceException(HttpServletRequest request
return new ResponseEntity<>(errorInfo(request, e.getCause()), HttpStatus.INTERNAL_SERVER_ERROR);
}

@ExceptionHandler(RecordAuthorNotFoundException.class)
public ResponseEntity<ErrorInfo> recordAuthorNotFoundException(HttpServletRequest request,
RecordAuthorNotFoundException e) {
logException(request, e);
return new ResponseEntity<>(errorInfo(request, e), HttpStatus.CONFLICT);
}

void logException(HttpServletRequest request, RuntimeException e) {
LOG.debug(
String.format(
"Request to '%s' failed due to error: %s",
request.getRequestURI(),
e.getMessage()
)
);
LOG.debug("Request to '{}' failed due to error: {}", request.getRequestURI(), e.getMessage());
}
}
22 changes: 16 additions & 6 deletions src/main/java/cz/cvut/kbss/study/service/PatientRecordService.java
Original file line number Diff line number Diff line change
Expand Up @@ -57,27 +57,37 @@ public interface PatientRecordService extends BaseService<PatientRecord> {
/**
* Imports the specified records.
* <p>
* The current user is set as the author of the records. Only records whose identifiers do not already exist in the
* repository are imported. Existing records are skipped and the returned object contains a note that the record
* already exists.
* Only records whose identifiers do not already exist in the repository are imported. Existing records are skipped
* and the returned object contains a note that the record already exists.
* <p>
* This method, in contrast to {@link #importRecords(List, RecordPhase)}, preserves the phase of the imported
* records.
* <p>
* If the current user is an admin, the import procedure retains provenance data of the record. Otherwise, the
* current user is set as the record's author.
*
* @param records Records to import
* @return Instance representing the import result
* @throws cz.cvut.kbss.study.exception.RecordAuthorNotFoundException Thrown when importing a record whose author
* does not exist in this application instance's
* repository
*/
RecordImportResult importRecords(List<PatientRecord> records);

/**
* Imports the specified records and sets them all to the specified phase.
* <p>
* The current user is set as the author of the records. Only records whose identifiers do not already exist in the
* repository are imported. Existing records are skipped and the returned object contains a note that the record
* already exists.
* Only records whose identifiers do not already exist in the repository are imported. Existing records are skipped
* and the returned object contains a note that the record already exists.
* <p>
* If the current user is an admin, the import procedure retains provenance data of the record. Otherwise, the
* current user is set as the record's author.
*
* @param records Records to import
* @return Instance representing the import result
* @throws cz.cvut.kbss.study.exception.RecordAuthorNotFoundException Thrown when importing a record whose author
* does not exist in this application instance's
* repository
*/
RecordImportResult importRecords(List<PatientRecord> records, RecordPhase targetPhase);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import cz.cvut.kbss.study.dto.PatientRecordDto;
import cz.cvut.kbss.study.dto.RecordImportResult;
import cz.cvut.kbss.study.exception.RecordAuthorNotFoundException;
import cz.cvut.kbss.study.model.Institution;
import cz.cvut.kbss.study.model.PatientRecord;
import cz.cvut.kbss.study.model.RecordPhase;
Expand All @@ -10,6 +11,7 @@
import cz.cvut.kbss.study.persistence.dao.PatientRecordDao;
import cz.cvut.kbss.study.persistence.dao.util.RecordFilterParams;
import cz.cvut.kbss.study.service.PatientRecordService;
import cz.cvut.kbss.study.service.UserService;
import cz.cvut.kbss.study.service.security.SecurityUtils;
import cz.cvut.kbss.study.util.Utils;
import org.slf4j.Logger;
Expand All @@ -31,10 +33,13 @@ public class RepositoryPatientRecordService extends KeySupportingRepositoryServi

private final SecurityUtils securityUtils;

public RepositoryPatientRecordService(PatientRecordDao recordDao,
SecurityUtils securityUtils) {
private final UserService userService;

public RepositoryPatientRecordService(PatientRecordDao recordDao, SecurityUtils securityUtils,
UserService userService) {
this.recordDao = recordDao;
this.securityUtils = securityUtils;
this.userService = userService;
}

@Override
Expand Down Expand Up @@ -91,9 +96,7 @@ public RecordImportResult importRecords(List<PatientRecord> records) {
final Date created = new Date();
final RecordImportResult result = new RecordImportResult(records.size());
records.forEach(r -> {
r.setAuthor(author);
r.setInstitution(author.getInstitution());
r.setDateCreated(created);
setImportedRecordProvenance(author, created, r);
if (recordDao.exists(r.getUri())) {
LOG.warn("Record {} already exists. Skipping it.", Utils.uriToString(r.getUri()));
result.addError("Record " + Utils.uriToString(r.getUri()) + " already exists.");
Expand All @@ -105,6 +108,17 @@ public RecordImportResult importRecords(List<PatientRecord> records) {
return result;
}

private void setImportedRecordProvenance(User currentUser, Date now, PatientRecord record) {
if (!currentUser.isAdmin()) {
record.setAuthor(currentUser);
record.setInstitution(currentUser.getInstitution());
record.setDateCreated(now);
} else if (!userService.exists(record.getAuthor().getUri())) {
throw new RecordAuthorNotFoundException("Author of record " + record + "not found during import.");
}
}


@Transactional
@Override
public RecordImportResult importRecords(List<PatientRecord> records, RecordPhase targetPhase) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import cz.cvut.kbss.study.dto.RecordImportResult;
import cz.cvut.kbss.study.environment.generator.Generator;
import cz.cvut.kbss.study.environment.util.Environment;
import cz.cvut.kbss.study.exception.RecordAuthorNotFoundException;
import cz.cvut.kbss.study.model.Institution;
import cz.cvut.kbss.study.model.PatientRecord;
import cz.cvut.kbss.study.model.RecordPhase;
Expand Down Expand Up @@ -332,4 +333,14 @@ void importRecordsImportsSpecifiedRecordsWithSpecifiedPhaseAndReturnsImportResul
.param("phase", targetPhase.getIri())).andExpect(status().isOk());
verify(patientRecordServiceMock).importRecords(anyList(), eq(targetPhase));
}

@Test
void importRecordsReturnsConflictWhenServiceThrowsRecordAuthorNotFound() throws Exception {
final List<PatientRecord> records =
List.of(Generator.generatePatientRecord(user), Generator.generatePatientRecord(user));
when(patientRecordServiceMock.importRecords(anyList())).thenThrow(RecordAuthorNotFoundException.class);

mockMvc.perform(post("/records/import").content(toJson(records)).contentType(MediaType.APPLICATION_JSON))
.andExpect(status().isConflict());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@
import cz.cvut.kbss.study.dto.RecordImportResult;
import cz.cvut.kbss.study.environment.generator.Generator;
import cz.cvut.kbss.study.environment.util.Environment;
import cz.cvut.kbss.study.exception.RecordAuthorNotFoundException;
import cz.cvut.kbss.study.model.PatientRecord;
import cz.cvut.kbss.study.model.RecordPhase;
import cz.cvut.kbss.study.model.User;
import cz.cvut.kbss.study.model.Vocabulary;
import cz.cvut.kbss.study.persistence.dao.PatientRecordDao;
import cz.cvut.kbss.study.service.UserService;
import cz.cvut.kbss.study.service.security.SecurityUtils;
import cz.cvut.kbss.study.util.IdentificationUtils;
import org.junit.jupiter.api.BeforeEach;
Expand All @@ -26,6 +29,7 @@
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
Expand All @@ -40,6 +44,9 @@ class RepositoryPatientRecordServiceTest {
@Mock
private SecurityUtils securityUtils;

@Mock
private UserService userService;

@InjectMocks
private RepositoryPatientRecordService sut;

Expand All @@ -50,20 +57,19 @@ void setUp() {
this.user = Generator.generateUser(Generator.generateInstitution());
Environment.setCurrentUser(user);
when(securityUtils.getCurrentUser()).thenReturn(user);
when(recordDao.exists(any())).thenReturn(false);
}

@Test
void importRecordsSetsCurrentUserAsAuthorAndImportsSpecifiedRecords() {
final List<PatientRecord> toImport =
List.of(Generator.generatePatientRecord(user), Generator.generatePatientRecord(user));
void importRecordsSetsCurrentUserAsAuthorWhenTheyAreRegularUserAndImportsSpecifiedRecords() {
final User originalAuthor = Generator.generateUser(Generator.generateInstitution());
final List<PatientRecord> toImport =
List.of(Generator.generatePatientRecord(originalAuthor), Generator.generatePatientRecord(originalAuthor));
toImport.forEach(r -> {
// Simulate that the records existed in another deployment from which they are imported
r.setKey(IdentificationUtils.generateKey());
r.setDateCreated(new Date(System.currentTimeMillis() - 10000L));
r.setAuthor(originalAuthor);
});
when(recordDao.exists(any())).thenReturn(false);

final RecordImportResult result = sut.importRecords(toImport);
assertEquals(toImport.size(), result.getTotalCount());
Expand All @@ -81,6 +87,50 @@ void importRecordsSetsCurrentUserAsAuthorAndImportsSpecifiedRecords() {
}
}

@Test
void importRecordsRetainsRecordProvenanceDataWhenCurrentUserIsAdmin() {
final User originalAuthor = Generator.generateUser(Generator.generateInstitution());
final List<PatientRecord> toImport =
List.of(Generator.generatePatientRecord(originalAuthor), Generator.generatePatientRecord(originalAuthor));
toImport.forEach(r -> {
// Simulate that the records existed in another deployment from which they are imported
r.setKey(IdentificationUtils.generateKey());
r.setDateCreated(new Date(System.currentTimeMillis() - 10000L));
});
user.addType(Vocabulary.s_c_administrator);
Environment.setCurrentUser(user);
when(userService.exists(originalAuthor.getUri())).thenReturn(true);
when(recordDao.exists(any())).thenReturn(false);

sut.importRecords(toImport);
final ArgumentCaptor<PatientRecord> captor = ArgumentCaptor.forClass(PatientRecord.class);
verify(recordDao, times(toImport.size())).persist(captor.capture());
final List<PatientRecord> imported = captor.getAllValues();
assertEquals(toImport.size(), imported.size());
for (int i = 0; i < toImport.size(); i++) {
assertEquals(toImport.get(i).getUri(), imported.get(i).getUri());
assertEquals(toImport.get(i).getKey(), imported.get(i).getKey());
assertEquals(originalAuthor, imported.get(i).getAuthor());
assertEquals(toImport.get(i).getDateCreated(), imported.get(i).getDateCreated());
}
}

@Test
void importRecordsThrowsRecordAuthorNotFoundExceptionWhenAdminImportsRecordsAndRecordAuthorIsNotFound() {
final User originalAuthor = Generator.generateUser(Generator.generateInstitution());
final List<PatientRecord> toImport =
List.of(Generator.generatePatientRecord(originalAuthor), Generator.generatePatientRecord(originalAuthor));
toImport.forEach(r -> {
// Simulate that the records existed in another deployment from which they are imported
r.setKey(IdentificationUtils.generateKey());
r.setDateCreated(new Date(System.currentTimeMillis() - 10000L));
});
user.addType(Vocabulary.s_c_administrator);
Environment.setCurrentUser(user);

assertThrows(RecordAuthorNotFoundException.class, () -> sut.importRecords(toImport));
}

@Test
void importRecordsSkipsImportingRecordsThatAlreadyExist() {
final List<PatientRecord> toImport =
Expand Down Expand Up @@ -114,6 +164,7 @@ void importRecordsWithPhaseSetsSpecifiedPhaseToAllRecords() {
r.setDateCreated(new Date(System.currentTimeMillis() - 10000L));
r.setAuthor(originalAuthor);
});
when(recordDao.exists(any())).thenReturn(false);

sut.importRecords(toImport, targetPhase);
final ArgumentCaptor<PatientRecord> captor = ArgumentCaptor.forClass(PatientRecord.class);
Expand Down

0 comments on commit dce1d6d

Please sign in to comment.