Skip to content

Commit

Permalink
[#33] Fix implementation of updateUser
Browse files Browse the repository at this point in the history
- remove validation check if old and new institutions are the same
- use variable "original" to update the institution
- log trace level message about the update only if old and new institutions are different
- add method to check if HasUri entities are equal by URI
- add method to get the URI of HasUri entity
  • Loading branch information
kostobog committed Dec 13, 2023
1 parent cc50949 commit 3402342
Showing 1 changed file with 28 additions and 16 deletions.
44 changes: 28 additions & 16 deletions src/main/java/cz/cvut/kbss/study/rest/OidcUserController.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import cz.cvut.kbss.study.exception.NotFoundException;
import cz.cvut.kbss.study.model.Institution;
import cz.cvut.kbss.study.model.User;
import cz.cvut.kbss.study.model.util.HasUri;
import cz.cvut.kbss.study.rest.exception.BadRequestException;
import cz.cvut.kbss.study.security.SecurityConstants;
import cz.cvut.kbss.study.service.InstitutionService;
Expand All @@ -14,10 +15,9 @@
import org.springframework.web.bind.annotation.*;

import java.net.URI;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;
import java.util.Optional;

/**
* API for getting basic user info.
Expand Down Expand Up @@ -74,23 +74,35 @@ public void updateUser(@PathVariable("username") String username, @RequestBody U
final User original = getByUsername(username);

assert original != null;
Institution newInstitution = user.getInstitution();
Institution oldInstitution = original.getInstitution();
original.setInstitution(newInstitution);
userService.update(original, sendEmail, "profileUpdate");

// validate institution update is valid
List<URI> institutionURIs = Arrays.asList(user.getInstitution(), original.getInstitution())
.stream().map(i -> i != null ? i.getUri() : null).collect(Collectors.toList());
if (Objects.equals(institutionURIs.get(0), institutionURIs.get(1))){
LOG.warn("Ignoring attempt to add user {} to institution {} because it is already in institution {}.", user, institutionURIs.get(0), institutionURIs.get(1));
return;
if (LOG.isTraceEnabled() && ! equals(newInstitution, oldInstitution)) {
LOG.trace("Set user institution {} to institution {} successfully.", user, oldInstitution, newInstitution);
}
}

// make sure only institution is updated
Institution newInstitution = user.getInstitution();
user = original.copy();
user.setInstitution(newInstitution);
userService.update(user, sendEmail, "profileUpdate");
if (LOG.isTraceEnabled()) {
LOG.trace("Added user {} to institution {} successfully.", user, user.getInstitution());
}
/**
* Input arguments can be null and getUri can be null.
* @param entity1
* @param entity2
* @return true if the URIs of the input arguments are equal, false otherwise.
*/
private boolean equals(HasUri entity1, HasUri entity2){
URI u1 = getURI(entity1);
URI u2 = getURI(entity2);
return Objects.equals(u1, u2);
}

/**
* Retrieves the URI of entity argument. entity can be null.
* @param entity
* @return the URI of the entity or null if entity is null.
*/
private URI getURI(HasUri entity){
return Optional.of(entity).map(HasUri::getUri).orElse(null);
}

private List<User> getByInstitution(String institutionKey) {
Expand Down

0 comments on commit 3402342

Please sign in to comment.