diff --git a/src/main/java/cz/cvut/kbss/study/model/Institution.java b/src/main/java/cz/cvut/kbss/study/model/Institution.java index 580a46bd..1fd7b4cc 100644 --- a/src/main/java/cz/cvut/kbss/study/model/Institution.java +++ b/src/main/java/cz/cvut/kbss/study/model/Institution.java @@ -86,6 +86,19 @@ public void setReferenceId(Integer referenceId) { this.referenceId = referenceId; } + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof Institution institution)) return false; + if (uri == null) return false; + return uri.equals(institution.uri); + } + + @Override + public int hashCode() { + return uri != null ? uri.hashCode() : System.identityHashCode(this); + } + @Override public String toString() { return "Institution{<" + uri + diff --git a/src/main/java/cz/cvut/kbss/study/persistence/dao/RecordDao.java b/src/main/java/cz/cvut/kbss/study/persistence/dao/RecordDao.java index 4bb70eb8..e6fb741b 100644 --- a/src/main/java/cz/cvut/kbss/study/persistence/dao/RecordDao.java +++ b/src/main/java/cz/cvut/kbss/study/persistence/dao/RecordDao.java @@ -173,11 +173,16 @@ public int getNumberOfProcessedRecords() { * @param entity The local name to be checked for uniqueness */ public void requireUniqueNonEmptyLocalName(Record entity) { - Objects.requireNonNull(entity.getInstitution()); if (entity.getLocalName() == null || entity.getLocalName().isEmpty()) { throw new ValidationException("error.record.localNameOfRecordIsEmpty", "Local name of record is empty for entity " + entity); } + + if(entity.getInstitution() == null){ + em.clear(); + return; + } + boolean unique = findByInstitution(entity.getInstitution()).stream() .filter(pr -> (entity.getFormTemplate() != null) && entity.getFormTemplate() .equals(pr.getFormTemplate())) @@ -305,37 +310,55 @@ private void setQueryParameters(Query query, Map queryParams) { private static String constructWhereClause(RecordFilterParams filters, Map queryParams) { // Could not use Criteria API because it does not support OPTIONAL - String whereClause = "{" + - "?r a ?type ; " + - "?hasAuthor ?author ; " + - "?hasCreatedDate ?created ; " + - "?hasInstitution ?institution . " + - "?institution ?hasKey ?institutionKey ." + - "?author ?hasUsername ?username ." + - "OPTIONAL { ?r ?hasPhase ?phase . } " + - "OPTIONAL { ?r ?hasFormTemplate ?formTemplate . } " + - "OPTIONAL { ?r ?hasLastModified ?lastModified . } " + - "BIND (COALESCE(?lastModified, ?created) AS ?date) "; + + String whereClause = """ + { + ?r a ?type ; + ?hasAuthor ?author ; + ?hasCreatedDate ?created . + ?author ?hasUsername ?username . + + OPTIONAL { + ?r ?hasInstitution ?institution . + ?institution ?hasKey ?institutionKey . + } + + OPTIONAL { ?r ?hasPhase ?phase . } + OPTIONAL { ?r ?hasFormTemplate ?formTemplate . } + OPTIONAL { ?r ?hasLastModified ?lastModified . } + + BIND (COALESCE(?lastModified, ?created) AS ?date) + """; + whereClause += mapParamsToQuery(filters, queryParams); whereClause += "}"; + return whereClause; } private static String constructWhereClauseWithGraphs(RecordFilterParams filters, Map queryParams) { // Could not use Criteria API because it does not support OPTIONAL - String whereClause = "{GRAPH ?r{" + - "?r a ?type ; " + - "?hasCreatedDate ?created ; " + - "?hasInstitution ?institution . " + - "OPTIONAL { ?r ?hasPhase ?phase . } " + - "OPTIONAL { ?r ?hasFormTemplate ?formTemplate . } " + - "OPTIONAL { ?r ?hasLastModified ?lastModified . } " + - "BIND (COALESCE(?lastModified, ?created) AS ?date) "; - whereClause += mapParamsToQuery(filters, queryParams); - whereClause += "}" + - "GRAPH ?institutionGraph{" + - "?institution ?hasKey ?institutionKey ." + - "}}"; + String whereClause = """ + {GRAPH ?r { + ?r a ?type ; + ?hasCreatedDate ?created . + + OPTIONAL { ?r ?hasInstitution ?institution . } + OPTIONAL { ?r ?hasPhase ?phase . } + OPTIONAL { ?r ?hasFormTemplate ?formTemplate . } + OPTIONAL { ?r ?hasLastModified ?lastModified . } + + BIND (COALESCE(?lastModified, ?created) AS ?date) + """ + + mapParamsToQuery(filters, queryParams) + + """ + } + OPTIONAL { + GRAPH ?institutionGraph { + ?institution ?hasKey ?institutionKey . + } + }} + """; queryParams.put("institutionGraph", URI.create(Vocabulary.s_c_institution + "s")); diff --git a/src/main/java/cz/cvut/kbss/study/rest/RecordController.java b/src/main/java/cz/cvut/kbss/study/rest/RecordController.java index eaadb2b0..9a233d51 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/RecordController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/RecordController.java @@ -149,7 +149,8 @@ public ResponseEntity exportRecordsExcel(MultiValueMap createRecord(@RequestBody Record record) { - if(userService.getCurrentUser().getInstitution() == null) + if(configReader.getConfig(ConfigParam.RECORDS_ALLOWED_CREATION_WITHOUT_INSTITUTION).equals("false") && userService.getCurrentUser().getInstitution() == null) throw new ValidationException("record.save-error.user-not-assigned-to-institution", "User is not assigned to any institution."); diff --git a/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java b/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java index e3245d8a..a256b15b 100644 --- a/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java +++ b/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java @@ -1,15 +1,18 @@ package cz.cvut.kbss.study.service.repository; import cz.cvut.kbss.jopa.exceptions.EntityNotFoundException; +import cz.cvut.kbss.study.dto.RecordDto; import cz.cvut.kbss.study.exception.EntityExistsException; import cz.cvut.kbss.study.exception.NotFoundException; import cz.cvut.kbss.study.exception.ValidationException; import cz.cvut.kbss.study.model.Institution; +import cz.cvut.kbss.study.model.Record; import cz.cvut.kbss.study.model.Role; import cz.cvut.kbss.study.model.User; import cz.cvut.kbss.study.persistence.dao.GenericDao; import cz.cvut.kbss.study.persistence.dao.RecordDao; import cz.cvut.kbss.study.persistence.dao.UserDao; +import cz.cvut.kbss.study.persistence.dao.util.RecordFilterParams; import cz.cvut.kbss.study.service.ConfigReader; import cz.cvut.kbss.study.service.EmailService; import cz.cvut.kbss.study.service.UserService; @@ -26,11 +29,8 @@ import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; - -import java.util.Comparator; -import java.util.List; -import java.util.Objects; -import java.util.Optional; +import java.util.*; +import java.util.stream.Collectors; @Service public class RepositoryUserService extends BaseRepositoryService implements UserService { @@ -207,23 +207,15 @@ protected void prePersist(User instance) { } } - @Override - protected void preUpdate(User instance) { - final User currentUser = securityUtils.getCurrentUser(); - final User original = userDao.findByUsername(instance.getUsername()); - - if(original == null) { - throw new EntityNotFoundException("User with specified username does not exist."); - } - - boolean differentUser = !Objects.equals(instance.getUsername(), currentUser.getUsername()); + protected void validatePermissionToUpdate(User currentUser, User toUpdate, User original) { + boolean differentUser = !Objects.equals(toUpdate.getUsername(), currentUser.getUsername()); boolean hasWriteAllUsers = securityUtils.hasRole(Role.writeAllUsers); boolean lacksPrivilegeOfUpdatedUser = !securityUtils.hasSupersetOfRoles(currentUser, original); - boolean sameInstitution = instance.getInstitution() != null + boolean sameInstitution = toUpdate.getInstitution() != null && currentUser.getInstitution() != null - && instance.getInstitution().getKey().equals(currentUser.getInstitution().getKey()); + && toUpdate.getInstitution().getKey().equals(currentUser.getInstitution().getKey()); boolean hasWriteOrganizationUsers = securityUtils.hasRole(Role.writeOrganizationUsers) && sameInstitution; @@ -243,6 +235,71 @@ protected void preUpdate(User instance) { currentUser.getUsername())); } } + } + + protected void validateRecordsAgainstCollisions(User toUpdate, User original) { + + if (toUpdate.getInstitution() == null) return; + + Map existingInstitutionRecords = recordDao.findByInstitution(toUpdate.getInstitution()).stream().collect( + Collectors.toMap(RecordDto::getLocalName, r -> r) + ); + Map newInstitutionRecords = recordDao.findByAuthor(original).stream().collect( + Collectors.toMap(Record::getLocalName, r -> r) + ); + + Map> conflictingRecords = newInstitutionRecords.keySet().stream() + .filter(existingInstitutionRecords::containsKey) + .collect(Collectors.toMap( + key -> key, + key -> Arrays.asList(existingInstitutionRecords.get(key).getKey(), newInstitutionRecords.get(key).getKey()) + )); + + + if (!conflictingRecords.isEmpty()) { + + String conflictsFormatted = conflictingRecords.entrySet().stream() + .map(e -> String.format( + "• %s (existing record ID: %s, new record ID: %s)", + e.getKey(), + e.getValue().get(0), + e.getValue().get(1) + )) + .collect(Collectors.joining("\n")); + + String message = String.format( + "User cannot be moved to institution '%s' because there are conflicting records with the same name:\n%s.", + toUpdate.getInstitution().getName(), + conflictsFormatted + ); + + throw new ValidationException(message); + } + } + + protected void changeRecordsInstitution(User original, Institution newInstitution) { + List recordsToUpdate = recordDao.findByAuthor(original); + for (Record record : recordsToUpdate) { + record.setInstitution(newInstitution); + recordDao.update(record); + } + } + + @Override + protected void preUpdate(User instance) { + final User currentUser = securityUtils.getCurrentUser(); + final User original = userDao.findByUsername(instance.getUsername()); + + if (original == null) { + throw new EntityNotFoundException("User with specified username does not exist."); + } + + validatePermissionToUpdate(currentUser, instance, original); + + if (!Objects.equals(instance.getInstitution(), original.getInstitution())) { + validateRecordsAgainstCollisions(instance, original); + changeRecordsInstitution(original, instance.getInstitution()); + } try { Validator.validateEmail(instance.getEmailAddress()); diff --git a/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java b/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java index fe14bcf5..bc66b2e9 100644 --- a/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java +++ b/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java @@ -155,6 +155,7 @@ public boolean isMemberOfInstitution(String institutionKey) { /** * Checks whether the current user is in same institution as the record was created. + * Returns false if user is not member of any institution. * * @param recordKey Record identifier * @return Membership status of the current user and record @@ -162,9 +163,25 @@ public boolean isMemberOfInstitution(String institutionKey) { public boolean isRecordInUsersInstitution(String recordKey) { final User user = getCurrentUser(); final Record record = recordDao.findByKey(recordKey); + if (user.getInstitution() == null) { + return false; + } return user.getInstitution().getKey().equals(record.getInstitution().getKey()); } + /** + * Checks whether the current user is the author of the record. + * + * @param recordKey Record identifier + * @return Membership status of the current user and record + */ + public boolean isRecordCreatedByUser(String recordKey) { + final User user = getCurrentUser(); + final Record record = recordDao.findByKey(recordKey); + + return record.getAuthor() != null && record.getAuthor().equals(user); + } + /** * Checks whether the current user is in same institution as user we are asking for. * diff --git a/src/main/java/cz/cvut/kbss/study/util/ConfigParam.java b/src/main/java/cz/cvut/kbss/study/util/ConfigParam.java index 0f257bae..55b5a51a 100644 --- a/src/main/java/cz/cvut/kbss/study/util/ConfigParam.java +++ b/src/main/java/cz/cvut/kbss/study/util/ConfigParam.java @@ -45,7 +45,8 @@ public enum ConfigParam { CORS_ALLOWED_ORIGINS("security.cors.allowedOrigins"), - RECORDS_ALLOWED_REJECT_REASON("records.allowedRejectReason"); + RECORDS_ALLOWED_REJECT_REASON("records.allowedRejectReason"), + RECORDS_ALLOWED_CREATION_WITHOUT_INSTITUTION("records.allowedCreationWithoutInstitution"); private final String name; diff --git a/src/main/java/cz/cvut/kbss/study/util/Configuration.java b/src/main/java/cz/cvut/kbss/study/util/Configuration.java index 48a6579f..032c7d55 100644 --- a/src/main/java/cz/cvut/kbss/study/util/Configuration.java +++ b/src/main/java/cz/cvut/kbss/study/util/Configuration.java @@ -422,12 +422,25 @@ public static class Records { */ boolean allowedRejectReason; + /** + * Allow users who are not members of any institution to create records. + */ + boolean allowedCreationWithoutInstitution; + public boolean isAllowedRejectReason() { return allowedRejectReason; } + public boolean isAllowedCreationWithoutInstitution() { + return allowedCreationWithoutInstitution; + } + public void setAllowedRejectReason(boolean allowedRejectReason) { this.allowedRejectReason = allowedRejectReason; } + + public void setAllowedCreationWithoutInstitution(boolean allowedCreationWithoutInstitution) { + this.allowedCreationWithoutInstitution = allowedCreationWithoutInstitution; + } } } diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index 54fd3e46..9d643401 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -54,3 +54,4 @@ email: records: allowedRejectReason: true + allowedCreationWithoutInstitution: true \ No newline at end of file diff --git a/src/test/java/cz/cvut/kbss/study/rest/RecordControllerTest.java b/src/test/java/cz/cvut/kbss/study/rest/RecordControllerTest.java index 409784e5..f9f80772 100644 --- a/src/test/java/cz/cvut/kbss/study/rest/RecordControllerTest.java +++ b/src/test/java/cz/cvut/kbss/study/rest/RecordControllerTest.java @@ -203,6 +203,9 @@ public void createRecordReturnsResponseStatusCreated() throws Exception { Record record = Generator.generateRecord(user); when(userService.getCurrentUser()).thenReturn(user); + when(configReaderMock.getConfig(ConfigParam.RECORDS_ALLOWED_CREATION_WITHOUT_INSTITUTION)) + .thenReturn("false"); + final MvcResult result = mockMvc.perform(post("/records").content(toJson(record)) .contentType(MediaType.APPLICATION_JSON_VALUE)) .andReturn(); @@ -211,12 +214,14 @@ public void createRecordReturnsResponseStatusCreated() throws Exception { } @Test - public void createRecordWithoutInstitutionReturnsResponseStatusBadRequest() throws Exception { + public void createRecordWithoutInstitutionIfItIsNotAllowedReturnsResponseStatusBadRequest() throws Exception { user.setInstitution(null); Record record = Generator.generateRecord(user); when(userService.getCurrentUser()).thenReturn(user); + when(configReaderMock.getConfig(ConfigParam.RECORDS_ALLOWED_CREATION_WITHOUT_INSTITUTION)) + .thenReturn("false"); final MvcResult result = mockMvc.perform(post("/records").content(toJson(record)) .contentType(MediaType.APPLICATION_JSON_VALUE)) @@ -225,6 +230,22 @@ public void createRecordWithoutInstitutionReturnsResponseStatusBadRequest() thro assertEquals(HttpStatus.CONFLICT, HttpStatus.valueOf(result.getResponse().getStatus())); } + @Test + public void createRecordWithoutInstitutionIfItIsAllowedReturnsResponseStatusCreated() throws Exception { + user.setInstitution(null); + + Record record = Generator.generateRecord(user); + + when(configReaderMock.getConfig(ConfigParam.RECORDS_ALLOWED_CREATION_WITHOUT_INSTITUTION)) + .thenReturn("true"); + + final MvcResult result = mockMvc.perform(post("/records").content(toJson(record)) + .contentType(MediaType.APPLICATION_JSON_VALUE)) + .andReturn(); + + assertEquals(HttpStatus.CREATED, HttpStatus.valueOf(result.getResponse().getStatus())); + } + @Test public void updateRecordReturnsResponseStatusNoContent() throws Exception {