Merge branch 'hotfix-1.4.6'

Author: arteymix

docs/release-notes.md

@@ -1,5 +1,14 @@
 # Release Notes
 
+## 1.4.6
+
+This release has a few fixes that provide better user feedback during registration and while updating the user profile.
+
+Fix deletion of user in the administrative section. Its associated tokens were somehow set to `NULL` by Hibernate before
+being deleted.
+
+Hide research focus in the user profile if nothing has been entered.
+
 ## 1.4.5
 
 This release resolves an issue with slow SMTP servers causing a proxy timeout by sending emails asynchronously.

mkdocs.yml

@@ -12,4 +12,4 @@ nav:
 plugins:
   - macros
 extra:
-  rdp_version: 1.4.3
+  rdp_version: 1.4.6

pom.xml

@@ -5,7 +5,7 @@
 
     <groupId>ubc.pavlab</groupId>
     <artifactId>rdp</artifactId>
-    <version>1.4.5</version>
+    <version>1.4.6</version>
 
     <developers>
         <developer>
@@ -175,7 +175,7 @@
 
     <properties>
         <java.version>1.8</java.version>
-        <tomcat.version>8.5.60</tomcat.version>
+        <tomcat.version>8.5.71</tomcat.version>
     </properties>
 
     <build>

src/main/java/ubc/pavlab/rdp/controllers/LoginController.java

@@ -67,7 +67,7 @@ public ModelAndView createNewUser( @Validated(User.ValidationUserAccount.class)
                                        RedirectAttributes redirectAttributes,
                                        Locale locale ) {
         ModelAndView modelAndView = new ModelAndView( "registration" );
-        User userExists = userService.findUserByEmailNoAuth( user.getEmail() );
+        User existingUser = userService.findUserByEmailNoAuth( user.getEmail() );
 
         user.setEnabled( false );
 
@@ -78,9 +78,15 @@ public ModelAndView createNewUser( @Validated(User.ValidationUserAccount.class)
         userProfile.setHideGenelist( false );
         userProfile.setContactEmailVerified( false );
 
-        if ( userExists != null ) {
-            bindingResult.rejectValue( "email", "error.user", "There is already a user registered this email." );
-            log.warn( "Trying to register an already registered email." );
+        if ( existingUser != null ) {
+            if ( existingUser.isEnabled() ) {
+                bindingResult.rejectValue( "email", "error.user", "There is already a user registered this email." );
+            } else {
+                // maybe the user is attempting to re-register, unaware that the confirmation hasn't been processed
+                userService.createVerificationTokenForUser( existingUser, locale );
+                bindingResult.rejectValue( "email", "error.user", "You have already registered an account with this email. We just sent you a new confirmation email." );
+            }
+            log.warn( "Trying to register an already registered email: " + user.getEmail() + "." );
         }
 
         if ( bindingResult.hasErrors() ) {

src/main/java/ubc/pavlab/rdp/controllers/UserController.java

@@ -5,14 +5,15 @@
 import lombok.extern.apachecommons.CommonsLog;
 import org.hibernate.validator.constraints.NotEmpty;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.context.ApplicationEventPublisher;
+import org.springframework.context.MessageSource;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
 import org.springframework.http.ResponseEntity;
 import org.springframework.security.access.annotation.Secured;
 import org.springframework.security.authentication.BadCredentialsException;
 import org.springframework.stereotype.Controller;
 import org.springframework.validation.BindingResult;
+import org.springframework.validation.FieldError;
 import org.springframework.web.bind.annotation.*;
 import org.springframework.web.multipart.MultipartFile;
 import org.springframework.web.servlet.ModelAndView;
@@ -57,6 +58,9 @@ public class UserController {
     @Autowired
     private ApplicationSettings applicationSettings;
 
+    @Autowired
+    private MessageSource messageSource;
+
     @GetMapping(value = { "/user/home" })
     public ModelAndView userHome() {
         ModelAndView modelAndView = new ModelAndView( "user/home" );
@@ -243,16 +247,54 @@ public String verifyContactEmail( @RequestParam String token, RedirectAttributes
     @Data
     static class ProfileWithOrganUberonIds {
         @Valid
-        private Profile profile;
-        private Set<String> organUberonIds;
+        private final Profile profile;
+        private final Set<String> organUberonIds;
+    }
+
+    @Data
+    static class ProfileSavedModel {
+        private final String message;
+        private final boolean contactEmailVerified;
+    }
+
+    @Data
+    static class FieldErrorModel {
+        private final String field;
+        private final String message;
+        private final Object rejectedValue;
+
+        public static FieldErrorModel fromFieldError( FieldError fieldError ) {
+            return new FieldErrorModel( fieldError.getField(), fieldError.getDefaultMessage(), fieldError.getRejectedValue() );
+        }
+    }
+
+    @Data
+    static class BindingResultModel {
+        private final List<FieldErrorModel> fieldErrors;
+
+        public static BindingResultModel fromBindingResult( BindingResult bindingResult ) {
+            return new BindingResultModel( bindingResult.getFieldErrors().stream().map( FieldErrorModel::fromFieldError ).collect( Collectors.toList() ) );
+        }
     }
 
     @ResponseBody
-    @PostMapping(value = "/user/profile", produces = MediaType.TEXT_PLAIN_VALUE)
-    public String saveProfile( @RequestBody ProfileWithOrganUberonIds profileWithOrganUberonIds, Locale locale ) {
+    @PostMapping(value = "/user/profile", produces = MediaType.APPLICATION_JSON_VALUE)
+    public ResponseEntity<?> saveProfile( @Valid @RequestBody ProfileWithOrganUberonIds profileWithOrganUberonIds, BindingResult bindingResult, Locale locale ) {
         User user = userService.findCurrentUser();
-        userService.updateUserProfileAndPublicationsAndOrgans( user, profileWithOrganUberonIds.profile, profileWithOrganUberonIds.profile.getPublications(), profileWithOrganUberonIds.organUberonIds, locale );
-        return "Saved.";
+        if ( bindingResult.hasErrors() ) {
+            return ResponseEntity.badRequest()
+                    .body( BindingResultModel.fromBindingResult( bindingResult ) );
+        } else {
+            String previousContactEmail = user.getProfile().getContactEmail();
+            user = userService.updateUserProfileAndPublicationsAndOrgans( user, profileWithOrganUberonIds.profile, profileWithOrganUberonIds.profile.getPublications(), profileWithOrganUberonIds.organUberonIds, locale );
+            String message = messageSource.getMessage( "UserController.profileSaved", new Object[]{ user.getProfile().getContactEmail() }, locale );
+            if ( user.getProfile().getContactEmail() != null &&
+                    !user.getProfile().getContactEmail().equals( previousContactEmail ) &&
+                    !user.getProfile().isContactEmailVerified() ) {
+                message = messageSource.getMessage( "UserController.profileSavedAndContactEmailUpdated", new String[]{ user.getProfile().getContactEmail() }, locale );
+            }
+            return ResponseEntity.ok( new ProfileSavedModel( message, user.getProfile().isContactEmailVerified() ) );
+        }
     }
 
     @ResponseBody

src/main/java/ubc/pavlab/rdp/model/AccessToken.java

@@ -22,8 +22,8 @@ public class AccessToken extends Token implements UserContent {
     @GeneratedValue(strategy = GenerationType.AUTO)
     private Integer id;
 
-    @ManyToOne(fetch = FetchType.EAGER)
-    @JoinColumn(name = "user_id", nullable = false)
+    @ManyToOne(optional = false)
+    @JoinColumn(name = "user_id")
     private User user;
 
     @Override

src/main/java/ubc/pavlab/rdp/model/PasswordResetToken.java

@@ -26,8 +26,8 @@ public class PasswordResetToken extends Token implements UserContent {
     @GeneratedValue(strategy = GenerationType.AUTO)
     private Integer id;
 
-    @OneToOne(fetch = FetchType.EAGER)
-    @JoinColumn(nullable = false, name = "user_id")
+    @ManyToOne(optional = false)
+    @JoinColumn(name = "user_id")
     private User user;
 
     @Override

src/main/java/ubc/pavlab/rdp/model/User.java

@@ -76,17 +76,17 @@ public interface ValidationServiceAccount {
     @JsonIgnore
     private final Set<Role> roles = new HashSet<>();
 
-    @OneToMany(cascade = CascadeType.ALL)
+    @OneToMany(cascade = CascadeType.ALL, orphanRemoval = true)
     @JoinColumn(name = "user_id")
     @JsonIgnore
     private final Set<AccessToken> accessTokens = new HashSet<>();
 
-    @OneToMany(cascade = CascadeType.ALL)
+    @OneToMany(cascade = CascadeType.ALL, orphanRemoval = true)
     @JoinColumn(name = "user_id")
     @JsonIgnore
     private final Set<VerificationToken> verificationTokens = new HashSet<>();
 
-    @OneToMany(cascade = CascadeType.ALL)
+    @OneToMany(cascade = CascadeType.ALL, orphanRemoval = true)
     @JoinColumn(name = "user_id")
     @JsonIgnore
     private final Set<PasswordResetToken> passwordResetTokens = new HashSet<>();

src/main/java/ubc/pavlab/rdp/model/VerificationToken.java

@@ -27,8 +27,8 @@ public class VerificationToken extends Token implements UserContent {
     @GeneratedValue(strategy = GenerationType.AUTO)
     private Integer id;
 
-    @OneToOne(fetch = FetchType.EAGER)
-    @JoinColumn(nullable = false, name = "user_id")
+    @ManyToOne(optional = false)
+    @JoinColumn(name = "user_id")
     private User user;
 
     @Email

src/main/java/ubc/pavlab/rdp/services/RemoteResourceService.java

@@ -1,5 +1,6 @@
 package ubc.pavlab.rdp.services;
 
+import ubc.pavlab.rdp.controllers.ApiController;
 import ubc.pavlab.rdp.exception.RemoteException;
 import ubc.pavlab.rdp.model.Taxon;
 import ubc.pavlab.rdp.model.User;
@@ -10,6 +11,7 @@
 
 import java.net.URI;
 import java.util.Collection;
+import java.util.Locale;
 import java.util.Set;
 import java.util.UUID;
 
@@ -19,15 +21,47 @@
  */
 public interface RemoteResourceService {
 
+    /**
+     * Get the version of the remote API by reading its OpenAPI specification.
+     *
+     * @param remoteHost from which only the authority is used with {@link URI#getAuthority()}
+     * @return the API version
+     * @throws RemoteException if any error occured while retrieving the API version
+     */
     String getApiVersion( URI remoteHost ) throws RemoteException;
 
+    /**
+     * Find users by name among all partner registries.
+     *
+     * @see ApiController#searchUsersByName(String, Boolean, Set, Set, Set, String, String, Locale)
+     */
     Collection<User> findUsersByLikeName( String nameLike, Boolean prefix, Set<ResearcherPosition> researcherPositions, Collection<ResearcherCategory> researcherTypes, Collection<String> organUberonIds );
 
+    /**
+     * Find users by description among all partner registries.
+     *
+     * @see ApiController#searchUsersByDescription(String, Set, Set, Set, String, String, Locale)
+     */
     Collection<User> findUsersByDescription( String descriptionLike, Set<ResearcherPosition> researcherPositions, Collection<ResearcherCategory> researcherTypes, Collection<String> organUberonIds );
 
+    /**
+     * Find genes by symbol among all partner registries.
+     *
+     * @see ApiController#searchUsersByGeneSymbol(String, Integer, Set, Integer, Set, Set, Set, String, String, Locale)
+     */
     Collection<UserGene> findGenesBySymbol( String symbol, Taxon taxon, Set<TierType> tier, Integer orthologTaxonId, Set<ResearcherPosition> researcherPositions, Set<ResearcherCategory> researcherTypes, Set<String> organUberonIds );
 
+    /**
+     * Retrieve a user from a specific registry.
+     *
+     * @see ApiController#getUserById(Integer, String, String, Locale)
+     */
     User getRemoteUser( Integer userId, URI remoteHost ) throws RemoteException;
 
+    /**
+     * Retrieve an anonymized user from a specific registry.
+     *
+     * @see ApiController#getUserByAnonymousId(UUID, String, String, Locale)
+     */
     User getAnonymizedUser( UUID anonymousId, URI remoteHost ) throws RemoteException;
 }

src/main/java/ubc/pavlab/rdp/services/RemoteResourceServiceImpl.java

@@ -11,10 +11,7 @@
 import org.springframework.stereotype.Service;
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
-import org.springframework.util.concurrent.ListenableFuture;
-import org.springframework.util.concurrent.ListenableFutureCallback;
 import org.springframework.web.client.AsyncRestTemplate;
-import org.springframework.web.client.RestClientException;
 import org.springframework.web.util.UriComponentsBuilder;
 import ubc.pavlab.rdp.exception.RemoteException;
 import ubc.pavlab.rdp.model.Taxon;
@@ -69,9 +66,11 @@ public String getApiVersion( URI remoteHost ) throws RemoteException {
                 .toUri();
         try {
             OpenAPI openAPI = asyncRestTemplate.getForEntity( uri, OpenAPI.class ).get().getBody();
-            // OpenAPI specification was introduced in 1.4, so we assume 1.0.0 for previous versions
+            // The OpenAPI specification was introduced in 1.4, so we assume 1.0.0 for previous versions
             if ( openAPI.getInfo() == null ) {
                 return "1.0.0";
+            } else if ( openAPI.getInfo().getVersion().equals( "v0" ) ) {
+                return "1.4.0"; // the version number was missing in early 1.4
             } else {
                 return openAPI.getInfo().getVersion();
             }
@@ -146,14 +145,7 @@ public User getRemoteUser( Integer userId, URI remoteHost ) throws RemoteExcepti
                 .buildAndExpand( Collections.singletonMap( "userId", userId ) )
                 .toUri();
 
-        try {
-            ResponseEntity<User> responseEntity = asyncRestTemplate.getForEntity( uri, User.class ).get();
-            User user = responseEntity.getBody();
-            initUser( user );
-            return user;
-        } catch ( InterruptedException | ExecutionException e ) {
-            throw new RemoteException( MessageFormat.format( "Unsuccessful response received for {0}.", uri ), e );
-        }
+        return getUserByUri( uri );
     }
 
     @Override
@@ -174,6 +166,10 @@ public User getAnonymizedUser( UUID anonymousId, URI remoteHost ) throws RemoteE
                 .buildAndExpand( Collections.singletonMap( "anonymousId", anonymousId ) )
                 .toUri();
 
+        return getUserByUri( uri );
+    }
+
+    private User getUserByUri( URI uri ) throws RemoteException {
         try {
             ResponseEntity<User> responseEntity = asyncRestTemplate.getForEntity( uri, User.class ).get();
             User user = responseEntity.getBody();

src/main/java/ubc/pavlab/rdp/util/VersionUtils.java

@@ -1,17 +1,20 @@
 package ubc.pavlab.rdp.util;
 
+import lombok.extern.apachecommons.CommonsLog;
+
+@CommonsLog
 public class VersionUtils {
 
     private static final int[] FACTORS = new int[]{ 99 * 99 * 99, 99 * 99, 99 };
 
-    private static int parseVersion( String version ) {
+    private static int parseVersion( String version ) throws IllegalArgumentException {
         int i = 0;
         int v = 0;
         String[] components = version.split( "\\." );
         for ( String c : components ) {
             int ci = Integer.parseInt( c );
             if ( ci < 0 || ci > 99 ) {
-                throw new RuntimeException( "Version component must be within 0 and 99." );
+                throw new IllegalArgumentException( "Version component must be within 0 and 99." );
             }
             v += FACTORS[i++] * ci;
         }

src/main/resources/db/migration/common/V1.4.6__make_token_user_id_nullable.sql

@@ -0,0 +1,4 @@
+-- I'm not exactly sure why this is necessary, but Hibernate set this column to NULL before removing it
+alter table access_token modify column user_id integer null;
+alter table password_reset_token modify column user_id integer null;
+alter table verification_token modify column user_id integer null;

src/main/resources/messages.properties

@@ -36,7 +36,7 @@ TierType.TIER1=Tier 1
 TierType.TIER2=Tier 2
 TierType.TIER3=Tier 3
 
-AbstractUserDetailsAuthenticationProvider.badCredentials=Username/Password is incorrect.
+AbstractUserDetailsAuthenticationProvider.badCredentials=Your username or password is incorrect.
 AbstractUserDetailsAuthenticationProvider.disabled=Your account is disabled, please confirm your email. <a href="resendConfirmation">Click here</a> to resend confirmation.
 AbstractUserDetailsAuthenticationProvider.expired=User account has expired.
 AbstractUserDetailsAuthenticationProvider.locked=User account is locked.
@@ -55,6 +55,11 @@ SearchController.errorNoOrthologs=No orthologs of <strong>{0}</strong> found in
 # {1} contains the taxon scientific name
 SearchController.errorNoGene=Unknown gene <strong>{0}</strong> in taxon <em>{1}</em>.
 
+UserController.profileSaved=Your profile has been saved.
+# {0} contains the contact email
+UserController.profileSavedAndContactEmailUpdated=Your profile has been saved. Your contact email was updated and an \
+email with a verification link has been sent to {0}.
+
 # {0} contains the site shortname
 ApiConfig.title={0} RESTful API
 # {0} contains the site shortname