Skip to content

Commit

Permalink
More Nullable checks and fixes
Browse files Browse the repository at this point in the history
Use the Optional API for all repository methods that can return null.

Fix a pretty nasty bug where Spring Data would honor the non-null return
value for UserRepository.findByEmailIgnoreCase().

Remove unused imports.
  • Loading branch information
arteymix committed Oct 8, 2024
1 parent 2bfde1a commit a321a91
Show file tree
Hide file tree
Showing 87 changed files with 321 additions and 342 deletions.
1 change: 0 additions & 1 deletion src/main/java/ubc/pavlab/rdp/Application.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.cache.annotation.EnableCaching;
import org.springframework.data.jpa.repository.config.EnableJpaAuditing;
import org.springframework.scheduling.annotation.EnableScheduling;

@SpringBootApplication
Expand Down
4 changes: 0 additions & 4 deletions src/main/java/ubc/pavlab/rdp/ResourceLoaderConfig.java
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
package ubc.pavlab.rdp;

import lombok.extern.apachecommons.CommonsLog;
import org.springframework.beans.BeansException;
import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextAware;
import org.springframework.context.ResourceLoaderAware;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.support.GenericApplicationContext;
import org.springframework.core.io.DefaultResourceLoader;
import org.springframework.core.io.ResourceLoader;
import ubc.pavlab.rdp.util.PurlResolver;
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/ubc/pavlab/rdp/ValidationConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import org.springframework.context.annotation.Configuration;
import org.springframework.core.io.Resource;
import org.springframework.http.converter.FormHttpMessageConverter;
import org.springframework.lang.Nullable;
import org.springframework.web.client.RestTemplate;
import ubc.pavlab.rdp.validation.*;

Expand All @@ -27,8 +28,8 @@ public class ValidationConfig {

@Bean
public EmailValidator emailValidator(
@Value("${rdp.settings.allowed-email-domains}") List<String> allowedEmailDomains,
@Value("${rdp.settings.allowed-email-domains-file}") Resource allowedEmailDomainsFile,
@Value("${rdp.settings.allowed-email-domains}") @Nullable List<String> allowedEmailDomains,
@Value("${rdp.settings.allowed-email-domains-file}") @Nullable Resource allowedEmailDomainsFile,
@Value("${rdp.settings.allowed-email-domains-file-refresh-delay}") @DurationUnit(ChronoUnit.SECONDS) Duration refreshDelay,
@Value("${rdp.settings.allow-internationalized-email-domains}") boolean allowIdn ) throws IOException {
List<AllowedDomainStrategy> strategies = new ArrayList<>();
Expand Down
4 changes: 0 additions & 4 deletions src/main/java/ubc/pavlab/rdp/WebSecurityConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import ubc.pavlab.rdp.security.authentication.TokenBasedAuthenticationManager;
import ubc.pavlab.rdp.services.UserService;
import ubc.pavlab.rdp.settings.ApplicationSettings;
import ubc.pavlab.rdp.settings.SiteSettings;

import javax.servlet.*;
import javax.servlet.http.HttpServletRequest;
Expand Down Expand Up @@ -63,9 +62,6 @@ public class WebSecurityConfig extends WebSecurityConfigurerAdapter {
@Autowired
private UserService userService;

@Autowired
private SiteSettings siteSettings;

@Override
protected void configure( AuthenticationManagerBuilder auth ) throws Exception {
auth.userDetailsService( userDetailsService ).passwordEncoder( bCryptPasswordEncoder );
Expand Down
44 changes: 26 additions & 18 deletions src/main/java/ubc/pavlab/rdp/controllers/AdminController.java
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public Object createServiceAccount( @Validated(User.ValidationServiceAccount.cla
}

@PostMapping(value = "/admin/users/{user}/roles")
public Object updateRoles( @PathVariable User user, @RequestParam(required = false) Set<Role> roles, RedirectAttributes redirectAttributes, Locale locale ) {
public Object updateRoles( @PathVariable User user, @RequestParam(required = false) @Nullable Set<Role> roles, RedirectAttributes redirectAttributes, Locale locale ) {
if ( roles == null ) {
roles = Collections.emptySet();
}
Expand Down Expand Up @@ -176,7 +176,7 @@ public Object updateRoles( @PathVariable User user, @RequestParam(required = fal
* Retrieve a user's details.
*/
@GetMapping(value = "/admin/users/{user}")
public Object getUser( @PathVariable(required = false) User user, @SuppressWarnings("unused") ConfirmEmailForm confirmEmailForm, Locale locale ) {
public Object getUser( @PathVariable(required = false) @Nullable User user, @SuppressWarnings("unused") ConfirmEmailForm confirmEmailForm, Locale locale ) {
if ( user == null ) {
return new ModelAndView( "error/404", HttpStatus.NOT_FOUND )
.addObject( "message", messageSource.getMessage( "AdminController.userNotFoundById", null, locale ) );
Expand All @@ -185,7 +185,7 @@ public Object getUser( @PathVariable(required = false) User user, @SuppressWarni
}

@PostMapping(value = "/admin/users/{user}/create-access-token")
public Object createAccessTokenForUser( @PathVariable(required = false) User user, RedirectAttributes redirectAttributes, Locale locale ) {
public Object createAccessTokenForUser( @PathVariable(required = false) @Nullable User user, RedirectAttributes redirectAttributes, Locale locale ) {
if ( user == null ) {
return new ModelAndView( "error/404", HttpStatus.NOT_FOUND )
.addObject( "message", messageSource.getMessage( "AdminController.userNotFoundById", null, locale ) );
Expand All @@ -196,7 +196,7 @@ public Object createAccessTokenForUser( @PathVariable(required = false) User use
}

@PostMapping(value = "/admin/users/{user}/revoke-access-token/{accessToken}")
public Object revokeAccessTn( @PathVariable(required = false) User user, @PathVariable(required = false) AccessToken accessToken, RedirectAttributes redirectAttributes, Locale locale ) {
public Object revokeAccessTn( @PathVariable(required = false) @Nullable User user, @PathVariable(required = false) @Nullable AccessToken accessToken, RedirectAttributes redirectAttributes, Locale locale ) {
if ( user == null ) {
return new ModelAndView( "error/404", HttpStatus.NOT_FOUND )
.addObject( "message", messageSource.getMessage( "AdminController.userNotFoundById", null, locale ) );
Expand All @@ -218,7 +218,7 @@ public static class ConfirmEmailForm {
* Delete a given user.
*/
@DeleteMapping(value = "/admin/users/{user}")
public Object deleteUser( @PathVariable(required = false) User user,
public Object deleteUser( @PathVariable(required = false) @Nullable User user,
@Valid ConfirmEmailForm confirmEmailForm, BindingResult bindingResult,
Locale locale ) {
if ( user == null ) {
Expand All @@ -245,7 +245,7 @@ public ModelAndView getOntologies( @SuppressWarnings("unused") ImportOntologyFor
}

@GetMapping("/admin/ontologies/{ontology}")
public ModelAndView getOntology( @PathVariable(required = false) Ontology ontology, Locale locale ) {
public ModelAndView getOntology( @PathVariable(required = false) @Nullable Ontology ontology, Locale locale ) {
if ( ontology == null ) {
return new ModelAndView( "error/404", HttpStatus.NOT_FOUND )
.addObject( "message", messageSource.getMessage( "AdminController.ontologyNotFoundById", null, locale ) );
Expand Down Expand Up @@ -293,15 +293,17 @@ public static UpdateOntologyForm fromOntology( Ontology ontology ) {
@Size(min = 1, max = Ontology.MAX_NAME_LENGTH)
private String name;

@Nullable
private String definition;

@Nullable
private URL ontologyUrl;

private boolean availableForGeneSearch;
}

@PostMapping("/admin/ontologies/{ontology}")
public ModelAndView updateOntology( @PathVariable(required = false) Ontology ontology,
public ModelAndView updateOntology( @PathVariable(required = false) @Nullable Ontology ontology,
@Valid UpdateOntologyForm updateOntologyForm, BindingResult bindingResult,
Locale locale ) {
if ( ontology == null ) {
Expand Down Expand Up @@ -337,7 +339,7 @@ public ModelAndView updateOntology( @PathVariable(required = false) Ontology ont
}

@DeleteMapping("/admin/ontologies/{ontology}")
public Object deleteOntology( @PathVariable(required = false) Ontology ontology,
public Object deleteOntology( @PathVariable(required = false) @Nullable Ontology ontology,
@Valid DeleteOntologyForm deleteOntologyForm, BindingResult bindingResult,
RedirectAttributes redirectAttributes,
Locale locale ) {
Expand Down Expand Up @@ -395,7 +397,7 @@ public Object createSimpleOntology( @SuppressWarnings("unused") ImportOntologyFo
}

@PostMapping("/admin/ontologies/{ontology}/update-simple-ontology")
public Object updateSimpleOntology( @PathVariable(required = false) Ontology ontology,
public Object updateSimpleOntology( @PathVariable(required = false) @Nullable Ontology ontology,
@Valid SimpleOntologyForm simpleOntologyForm, BindingResult bindingResult,
RedirectAttributes redirectAttributes, Locale locale ) {
if ( ontology == null ) {
Expand Down Expand Up @@ -483,6 +485,7 @@ public static SimpleOntologyTermForm emptyRow() {
/**
* Auto-generated if null or empty, which is why we allow zero as a size.
*/
@Nullable
@Size(max = OntologyTermInfo.MAX_TERM_ID_LENGTH, groups = RowNotEmpty.class)
private String termId;
@NotNull(groups = RowNotEmpty.class)
Expand Down Expand Up @@ -631,7 +634,7 @@ public void initBinding2( WebDataBinder webDataBinder ) {
}

@PostMapping("/admin/ontologies/{ontology}/update")
public Object updateOntology( @PathVariable(required = false) Ontology ontology,
public Object updateOntology( @PathVariable(required = false) @Nullable Ontology ontology,
RedirectAttributes redirectAttributes,
Locale locale ) {
if ( ontology == null ) {
Expand Down Expand Up @@ -716,7 +719,12 @@ public Object importReactomePathways( RedirectAttributes redirectAttributes ) {
@PostMapping("/admin/ontologies/{ontology}/update-reactome-pathways")
public Object updateReactomePathways( @PathVariable(required = false) @Nullable Ontology ontology, RedirectAttributes redirectAttributes, Locale locale ) {
// null-check is not necessary, but can save a database call
if ( ontology == null || !ontology.equals( reactomeService.findPathwaysOntology() ) ) {
if ( ontology == null ) {
return new ModelAndView( "error/404", HttpStatus.NOT_FOUND )
.addObject( "message", messageSource.getMessage( "AdminController.ontologyNotFoundById", null, locale ) );
}
Ontology reactome = reactomeService.findPathwaysOntology();
if ( reactome == null || !reactome.equals( ontology ) ) {
return new ModelAndView( "error/404", HttpStatus.NOT_FOUND )
.addObject( "message", messageSource.getMessage( "AdminController.ontologyNotFoundById", null, locale ) );
}
Expand Down Expand Up @@ -821,7 +829,7 @@ public static class DeactivateOntologyForm extends ActivateOrDeactivateOntologyF
}

@PostMapping("/admin/ontologies/{ontology}/activate")
public Object activateOntology( @PathVariable(required = false) Ontology ontology, ActivateOntologyForm activateOntologyForm, RedirectAttributes redirectAttributes, Locale locale ) {
public Object activateOntology( @PathVariable(required = false) @Nullable Ontology ontology, ActivateOntologyForm activateOntologyForm, RedirectAttributes redirectAttributes, Locale locale ) {
if ( ontology == null ) {
return new ModelAndView( "error/404", HttpStatus.NOT_FOUND )
.addObject( "message", messageSource.getMessage( "AdminController.ontologyNotFoundById", null, locale ) );
Expand All @@ -833,7 +841,7 @@ public Object activateOntology( @PathVariable(required = false) Ontology ontolog
}

@PostMapping("/admin/ontologies/{ontology}/deactivate")
public Object deactivateOntology( @PathVariable(required = false) Ontology ontology, DeactivateOntologyForm deactivateOntologyForm, RedirectAttributes redirectAttributes, Locale locale ) {
public Object deactivateOntology( @PathVariable(required = false) @Nullable Ontology ontology, DeactivateOntologyForm deactivateOntologyForm, RedirectAttributes redirectAttributes, Locale locale ) {
if ( ontology == null ) {
return new ModelAndView( "error/404", HttpStatus.NOT_FOUND )
.addObject( "message", messageSource.getMessage( "AdminController.ontologyNotFoundById", null, locale ) );
Expand Down Expand Up @@ -890,7 +898,7 @@ public Object deactivateTerm( @PathVariable(required = false) Ontology ontology,
return activateOrDeactivateTerm( ontology, deactivateTermForm, bindingResult, redirectAttributes, locale );
}

private Object activateOrDeactivateTerm( Ontology ontology,
private Object activateOrDeactivateTerm( @Nullable Ontology ontology,
ActivateOrDeactivateTermForm activateOrDeactivateTermForm, BindingResult bindingResult,
RedirectAttributes redirectAttributes,
Locale locale ) {
Expand All @@ -901,7 +909,7 @@ private Object activateOrDeactivateTerm( Ontology ontology,
OntologyTermInfo ontologyTermInfo = null;

if ( !bindingResult.hasFieldErrors( "ontologyTermInfoId" ) ) {
ontologyTermInfo = ontologyService.findTermByTermIdAndOntology( activateOrDeactivateTermForm.ontologyTermInfoId, ontology );
ontologyTermInfo = ontologyService.findTermByTermIdAndOntology( activateOrDeactivateTermForm.ontologyTermInfoId, ontology ).orElse( null );
if ( ontologyTermInfo == null ) {
bindingResult.rejectValue( "ontologyTermInfoId", "AdminController.ActivateOrDeactivateTermForm.unknownTermInOntology",
new String[]{ activateOrDeactivateTermForm.getOntologyTermInfoId(), messageSource.getMessage( ontology.getResolvableTitle(), locale ) }, null );
Expand Down Expand Up @@ -953,7 +961,7 @@ private Object activateOrDeactivateTerm( Ontology ontology,
* Provides the ontology in OBO format.
*/
@GetMapping(value = "/admin/ontologies/{ontology}/download", produces = "text/plain")
public ResponseEntity<StreamingResponseBody> downloadOntology( @PathVariable(required = false) Ontology ontology, TimeZone timeZone ) {
public ResponseEntity<StreamingResponseBody> downloadOntology( @PathVariable(required = false) @Nullable Ontology ontology, TimeZone timeZone ) {
if ( ontology == null ) {
return ResponseEntity.notFound().build();
}
Expand All @@ -969,7 +977,7 @@ public ResponseEntity<StreamingResponseBody> downloadOntology( @PathVariable(req

@ResponseBody
@GetMapping("/admin/ontologies/{ontology}/autocomplete-terms")
public Object autocompleteOntologyTerms( @PathVariable(required = false) Ontology ontology, @RequestParam String query, Locale locale ) {
public Object autocompleteOntologyTerms( @PathVariable(required = false) @Nullable Ontology ontology, @RequestParam String query, Locale locale ) {
if ( ontology == null ) {
return ResponseEntity.status( HttpStatus.NOT_FOUND )
.body( messageSource.getMessage( "AdminController.ontologyNotFoundById", null, locale ) );
Expand All @@ -993,7 +1001,7 @@ public ResponseEntity<String> refreshMessages() {
}

@PostMapping("/admin/ontologies/{ontology}/move")
public Object move( @PathVariable(required = false) Ontology ontology, @RequestParam String direction,
public Object move( @PathVariable(required = false) @Nullable Ontology ontology, @RequestParam String direction,
@SuppressWarnings("unused") ImportOntologyForm importOntologyForm,
Locale locale ) {
if ( ontology == null ) {
Expand Down
14 changes: 3 additions & 11 deletions src/main/java/ubc/pavlab/rdp/controllers/LoginController.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import ubc.pavlab.rdp.validation.Recaptcha;
import ubc.pavlab.rdp.validation.RecaptchaValidator;

import javax.annotation.Nullable;
import javax.servlet.http.HttpServletRequest;
import java.util.List;
import java.util.Locale;
Expand Down Expand Up @@ -110,8 +111,8 @@ public ModelAndView registration() {
@PostMapping("/registration")
public ModelAndView createNewUser( @Validated(User.ValidationUserAccount.class) User user,
BindingResult bindingResult,
@RequestParam(name = "g-recaptcha-response", required = false) String recaptchaResponse,
@RequestHeader(name = "X-Forwarded-For", required = false) List<String> clientIp,
@RequestParam(name = "g-recaptcha-response", required = false) @Nullable String recaptchaResponse,
@RequestHeader(name = "X-Forwarded-For", required = false) @Nullable List<String> clientIp,
RedirectAttributes redirectAttributes,
Locale locale ) {
ModelAndView modelAndView = new ModelAndView( "registration" );
Expand All @@ -129,19 +130,10 @@ public ModelAndView createNewUser( @Validated(User.ValidationUserAccount.class)
}

User existingUser = userService.findUserByEmailNoAuth( user.getEmail() );

// profile can be missing of no profile.* fields have been set
if ( user.getProfile() == null ) {
user.setProfile( new Profile() );
}

user.setEnabled( false );

// initialize a basic user profile
Profile userProfile = user.getProfile();
if ( userProfile == null ) {
userProfile = new Profile();
}
userProfile.setPrivacyLevel( privacyService.getDefaultPrivacyLevel() );
userProfile.setShared( applicationSettings.getPrivacy().isDefaultSharing() );
userProfile.setHideGenelist( false );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import javax.validation.Valid;
import java.util.Locale;

import static java.util.Objects.requireNonNull;

/**
* Created by mjacobson on 23/01/18.
*/
Expand Down Expand Up @@ -103,7 +105,7 @@ public ModelAndView showChangePasswordPage( @RequestParam("id") int id, @Request
}

// Log in
User user = userService.findUserByIdNoAuth( id );
User user = requireNonNull( userService.findUserByIdNoAuth( id ) );
UserPrinciple principle = new UserPrinciple( user );
Authentication auth = new UsernamePasswordAuthenticationToken( principle, null, principle.getAuthorities() );
SecurityContextHolder.getContext().setAuthentication( auth );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,8 +264,8 @@ public ModelAndView searchUsersByGeneView( @RequestParam String symbol,
@GetMapping(value = "/search/view/orthologs")
public ModelAndView searchOrthologsForGene( @RequestParam String symbol,
@RequestParam Integer taxonId,
@RequestParam(required = false) Set<TierType> tiers,
@RequestParam(required = false) Integer orthologTaxonId,
@RequestParam(required = false) @Nullable Set<TierType> tiers,
@RequestParam(required = false) @Nullable Integer orthologTaxonId,
Locale locale ) {
// Only look for orthologs when taxon is human
if ( taxonId != 9606 ) {
Expand Down Expand Up @@ -324,7 +324,7 @@ public ModelAndView searchOrthologsForGene( @RequestParam String symbol,
*/
@PreAuthorize("hasPermission(null, 'international-search')")
@GetMapping("/search/view/international/available-terms-by-partner")
public ModelAndView getAvailableTermsByPartner( @RequestParam(required = false) List<Integer> ontologyTermIds ) {
public ModelAndView getAvailableTermsByPartner( @RequestParam(required = false) @Nullable List<Integer> ontologyTermIds ) {
if ( ontologyTermIds == null || ontologyTermIds.isEmpty() ) {
return new ModelAndView( "fragments/error::message", HttpStatus.BAD_REQUEST )
.addObject( "errorMessage", "There must be at least one specified ontology term ID via 'ontologyTermIds'." );
Expand Down
Loading

0 comments on commit a321a91

Please sign in to comment.