9

I just started learning Spring Boot by reading the book Spring Boot in Action and I am learning the examples of this book, trying to run them myself but I have a problem using JpaRepository.findOne().

I've gone allover the Chapter to find my possible mismatches. However, it just DO NOT work.

The project is supposed to be a simple Reading List.

Here is the code :

The Reader @Entity:

package com.lixin.readinglist;

import org.springframework.data.annotation.Id;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
import org.springframework.security.core.userdetails.UserDetails;

import javax.persistence.Entity;
import java.util.Collection;
import java.util.Collections;

/**
 * @author lixin
 */
@Entity
public class Reader implements UserDetails {

    private static final long serialVersionUID = 1L;

    @Id
    private String username;
    private String fullname;
    private String password;

    @Override
    public String getUsername() {
        return username;
    }

    public void setUsername(String username) {
        this.username = username;
    }

    public String getFullname() {
        return fullname;
    }

    public void setFullname(String fullname) {
        this.fullname = fullname;
    }

    @Override
    public String getPassword() {
        return password;
    }

    public void setPassword(String password) {
        this.password = password;
    }

    @Override
    public Collection<? extends GrantedAuthority> getAuthorities() {
        return Collections.singletonList(new SimpleGrantedAuthority("READER"));
    }

    @Override
    public boolean isAccountNonExpired() {
        return true;
    }

    @Override
    public boolean isAccountNonLocked() {
        return true;
    }

    @Override
    public boolean isCredentialsNonExpired() {
        return true;
    }

    @Override
    public boolean isEnabled() {
        return true;
    }
}

The Jpa interface:

package com.lixin.readinglist;

import org.springframework.data.jpa.repository.JpaRepository;

/**
 * @author lixin
 */
public interface ReaderRepository extends JpaRepository<Reader, String> {
}

The SecurityConfig:

package com.lixin.readinglist;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Configuration;
import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
import org.springframework.security.core.userdetails.UserDetailsService;

/**
 * @author lixin
 */
@Configuration
@EnableWebSecurity
public class SecurityConfig extends WebSecurityConfigurerAdapter {

    private final ReaderRepository readerRepository;

    @Autowired
    public SecurityConfig(ReaderRepository readerRepository) {
        this.readerRepository = readerRepository;
    }

    @Override
    protected void configure(HttpSecurity http) throws Exception {
        http
                .authorizeRequests()
                .antMatchers("/").access("hasRole('READER')")
                .antMatchers("/**").permitAll()
                .and()
                .formLogin()
                .loginPage("/login")
                .failureUrl("/login?error=true");
    }

    @Override
    protected void configure(AuthenticationManagerBuilder auth) throws Exception {
        auth
                .userDetailsService((UserDetailsService) username -> readerRepository.findOne(username));
    }
}

And I kept getting this ERROR:

Error:(40, 86) java: method findOne in interface org.springframework.data.repository.query.QueryByExampleExecutor<T> cannot be applied to given types;
  required: org.springframework.data.domain.Example<S>
  found: java.lang.String
  reason: cannot infer type-variable(s) S
    (argument mismatch; java.lang.String cannot be converted to org.springframework.data.domain.Example<S>)
Justin Lee
  • 800
  • 1
  • 11
  • 22
  • 2
    You're probably reading an old book. findOne() has been renamed to findById() in recent versions of Spring Data. And it returns an Optional, not a Reader or null. So you need to adapt the code. – JB Nizet Jan 05 '19 at 08:57
  • 1
    Note also that it's usually an inefficient choice to use a `String` value as a primary key, as lookups tend to be slow. `Long` or `UUID` is usually better. – chrylis -cautiouslyoptimistic- Jan 05 '19 at 09:31
  • 1
    Finally, reviewing the [Semantic Versioning](semver.org) spec is useful. Your guide is written for Spring Data 1, while you are using Spring Data 2, and between major versions API components may be removed--in this case, the `findOne` method was removed from `CrudRepository` and replaced by the similar but not identical `findById`. – chrylis -cautiouslyoptimistic- Jan 05 '19 at 09:34
  • 1
    @JBNizet Thanks for your remark. I've tested the getOne() method and it threw the exact error as you remarked. I've updated my answer. As an undergraduate student, it's an honor to have you correct my mistake and lead me to the correct way. Appreciate for your time. – Justin Lee Jan 06 '19 at 16:42

6 Answers6

7

findOne() is defined as <S extends T> Optional<S> findOne(Example<S> example);.
It means that in your case it accepts a Example<Reader> and returns an Optional<Reader>.
You passed to it a String, which is wrong and you use it as lambda return in AuthenticationManagerBuilder.userDetailsService(), which is also wrong because UserDetailsService is an interface functional defined as

UserDetails loadUserByUsername(String username) throws UsernameNotFoundException;

So you need to return an UserDetails instance not an Optional of it or to throw UsernameNotFoundException if no matching with the username to be compliant with the javadoc :

Returns:

a fully populated user record (never null)

Throws:

UsernameNotFoundException - if the user could not be found or the user has no GrantedAuthority

Besides you don't need to use findOne() that is a query by example. A query by ID is enough.

So you could write something like that :

@Override
protected void configure(AuthenticationManagerBuilder auth) throws Exception {
   auth.userDetailsService(username -> readerRepository.findById(username)
                                                       .orElseThrow( () -> new UsernameNotFoundException("user with username " + username + " not found"));
}

As a side note, getOne() is tricky enough as it relies on lazy loading that may give bad surprises in some cases.
The remark of JB Nizet was interesting. So I tested right now. It happens that the JPA session is not still opened when the entity (namely isAccountNonLocked()) is accessed by the Spring Security classes.
So a LazyInitializationException is thrown in any case (username correct or no) :

org.hibernate.LazyInitializationException: could not initialize proxy - no Session
        at org.hibernate.proxy.AbstractLazyInitializer.initialize(AbstractLazyInitializer.java:155)
        at org.hibernate.proxy.AbstractLazyInitializer.getImplementation(AbstractLazyInitializer.java:268)
        at org.hibernate.proxy.pojo.javassist.JavassistLazyInitializer.invoke(JavassistLazyInitializer.java:73)
        at davidhxxx.example.angularsboot.model.db.User_$$_jvstd90_5.isAccountNonLocked(User_$$_jvstd90_5.java)
        at org.springframework.security.authentication.dao.AbstractUserDetailsAuthenticationProvider$DefaultPreAuthenticationChecks.check(AbstractUserDetailsAuthenticationProvider.java:352)
        at org.springframework.security.authentication.dao.AbstractUserDetailsAuthenticationProvider.authenticate(AbstractUserDetailsAuthenticationProvider.java:165)

This question may interest you.

davidxxx
  • 125,838
  • 23
  • 214
  • 215
  • 3
    It would definitely be an issue here: getOne() will return an uninitialized proxy over a UserDetails, whether or not the user actually exists or not. So that would violate the contract, since a UsernameNotFoundException is supposed to be thrown in that case. – JB Nizet Jan 05 '19 at 11:10
  • 1
    @JB Nizet In fact in any case it fails. I just tested. I updated consequently. Thanks for your remark ;) – davidxxx Jan 05 '19 at 11:45
  • Just as @JBNizet mentioned. I found that using `getOne()` instead of `findOne()` will definitely violate the contract and produce some bad surprises from time to time. And I realized that Nizet's comment was beyond excellent. I'm just a naive student, and it's an honor to have you correct my mistake and lead me to the correct way. I gonna edit my answer to correct the naive mistakes I've made. Thank you `&&` Appreciate that! – Justin Lee Jan 06 '19 at 16:08
2

As others have said, in the latest versions of Spring Data 2.x, you should use findById, instead of findOne, findOne in the latest version of Spring Data (that is part of Spring-Boot 2.x if you are using that) wants an example object. My guess is that the book you were using was written before the recent release of Spring 5 / Spring Boot 2 / Spring Data 2.x.

Hopefully reading the migration guide as a reference alongside your [slightly out-of-date] book will help: https://github.com/spring-projects/spring-boot/wiki/Spring-Boot-2.0-Migration-Guide

Rick Hanton
  • 93
  • 1
  • 7
1

You can use findById, instead of findOne, findOne wants an example object, you can look here for more

Hakob Hakobyan
  • 1,111
  • 8
  • 15
1

You can use getOne(), instead of findOne(). The author might have made a mistake.

  • Actually, I don't think that you should use `getOne` here, possibly that we have made the same mistake. Please read the comments made by Mr.Nizet below the question. – Justin Lee Jan 06 '19 at 16:27
0

The method findOne is from an interface named QueryByExampleExecutor, the JpaRepository interface extended it. while QueryByExampleExecutor is used for QBE(a kind of query which uses Example). In your code, you shouldn't use it, you can use the getOne method or findById method, the findById is inherited from the CrudRepository interface.

Juey
  • 123
  • 1
  • 6
0

After reading the answer from @davidxxx and the comment from @JB Nizet

I found that I've made a terrible mistake, the idea of using getOne() to take the place of findOne() will definitely violate the contract and produce some bad surprises from time to time.

And I realized that Nizet's comment was beyond excellent. I'm just a naive student, and it's an honor to have you correct my mistake and lead me to the correct way. I gonna edit my answer to correct the naive mistakes I've made. Thank you (@JB Nizet && @davidxxx) && Appreciate that!

The Solution:

@Override
protected void configure(AuthenticationManagerBuilder auth) throws Exception {
    auth.
            userDetailsService(username -> readerRepository.findById(username)
                    .orElseThrow(() -> new UsernameNotFoundException("user with username " + username + " not found")));
}

And you can find the reason here # Actually it is the answer from @davidxxx.

Justin Lee
  • 800
  • 1
  • 11
  • 22
  • 1
    I don't advise `getOne()` generally. It relies on lazy references and you can have bad surprises (not an issue here but well). You can read my answer https://stackoverflow.com/a/47370947/270371 and the post more generally. Besides you have to respect the contract of `loadUserByUsername()` : return a fully initialized `UserDetail` or `UsernameNotFoundException`. `JpaRepository.getOne()` doesn't guarantee that. – davidxxx Jan 05 '19 at 09:40
  • 3
    @davidxxx Thanks for your remark. I've tested the `getOne()` method and it threw the exact error as you remarked. I've updated my answer. As an undergraduate student, it's an honor to have you correct my mistake and lead me to the correct way. Appreciate for your time. – Justin Lee Jan 06 '19 at 16:34
  • 1
    That is a very good edit. Leaving good content in our answers for next readers of the post is a very good thing (+1). – davidxxx Jan 06 '19 at 18:38