Error in this loop of repetition. It is only getting the first value from the database

0

Error in this loopback. It is only getting the first value from the database. You are only doing the query in the first field and you are ignoring the others.

@Controller
// @RequestMapping("/login")
public class LoginController extends HttpServlet {


    private static final long serialVersionUID = 1L;
    private String username;
    private String password;

    @Autowired
    Usuarios usuarios;


    HttpServletRequest request;
    HttpServletResponse response;

    public String getUsername() {
        return username;
    }

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

    public String getPassword() {
        return password;
    }

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

    @RequestMapping("/login")
    public ModelAndView logins() {
        ModelAndView mv = new ModelAndView("/login");
        mv.addObject(new Documento());
        return mv;
    }

    @RequestMapping("/efetuaLogin")
    public ModelAndView login(HttpServletRequest request, HttpServletResponse response) {

        boolean validacao = false;

        HttpSession sessao;

        List<Usuario> usuariosCadastrados = usuarios.lista();


        String username = request.getParameter("username");
        String password = request.getParameter("password");

        for (Usuario usuario : usuariosCadastrados) {

            String loginbd = usuario.getUsername();
            String senhabd = usuario.getPassword();

            System.out.println("username do Formulario...:" + loginbd);
            System.out.println("Senha do banco........:" + senhabd);
            System.out.println("Senha do Formulario...:" + password);
            System.out.println("username do Formulario...:" + username);

            if (username.equals(loginbd) && password.equals(senhabd)) {
                validacao = true;

            }
            if (validacao == true) {

                return new ModelAndView("/documentos");

            } else {
                return new ModelAndView("hello");

            }
        }
        return null;
    }
}

    @Service
public class LoginService {

    @PersistenceContext
      private EntityManager em;

    @Autowired
    private Usuarios usuarios; 

    public List<Usuario> lista() {
      return em.createQuery("select u from Usuario u", Usuario.class).getResultList();
    }
}
    
asked by anonymous 22.07.2016 / 04:42

1 answer

1

The reason for going wrong is the structure of your for. Let's take a look:

    for (Usuario usuario : usuariosCadastrados) {

        // Várias linhas de código...

        if (validacao == true) {
            return new ModelAndView("/documentos");
        } else {
            return new ModelAndView("hello");
        }
    }
    return null;

Note that in the first iteration, it will either fall in if or else , and in both cases, a return will be executed. Ouch, if a return will always be executed in the first iteration, then it will always interrupt everything in the first interaction.

What you wanted was to keep looking instead of giving return to else :

    for (Usuario usuario : usuariosCadastrados) {

        String loginbd = usuario.getUsername();
        String senhabd = usuario.getPassword();

        System.out.println("username do Formulario...:" + loginbd);
        System.out.println("Senha do banco........:" + senhabd);
        System.out.println("Senha do Formulario...:" + password);
        System.out.println("username do Formulario...:" + username);

        if (username.equals(loginbd) && password.equals(senhabd)) {
            validacao = true;

        }
        if (validacao == true) {
            return new ModelAndView("/documentos");
        }
    }
    return new ModelAndView("hello");

However, there are still a lot of improvements that can be made. Using == true for example is totally unnecessary, and the variable becomes unnecessary since it is only to force the flow to arrive at return soon after. The sessao variable is also not being used. With that in mind, let's simplify your code even more:

@RequestMapping("/efetuaLogin")
public ModelAndView login(HttpServletRequest request, HttpServletResponse response) {
    List<Usuario> usuariosCadastrados = usuarios.lista();
    String username = request.getParameter("username");
    String password = request.getParameter("password");

    for (Usuario usuario : usuariosCadastrados) {
        String loginbd = usuario.getUsername();
        String senhabd = usuario.getPassword();
        System.out.println("username do Formulario...:" + loginbd);
        System.out.println("Senha do banco........:" + senhabd);
        System.out.println("Senha do Formulario...:" + password);
        System.out.println("username do Formulario...:" + username);

        if (username.equals(loginbd) && password.equals(senhabd)) {
            return new ModelAndView("/documentos");
        }
    }
    return new ModelAndView("hello");
}

However, you are getting all the users of the database and looking one by one to look for what has the correct login and password, which is inefficient. It would be much better to search the database only the user you want. The database is the ideal place to perform this type of search, since it has algorithms of data indexation very optimized for searches and in doing so you also significantly reduce the volume of data trafficked between the database and the application , which also improves performance and memory consumption. Therefore, your LoginService looks like this:

@Service
public class LoginService {

    @PersistenceContext
    private EntityManager em;

    @Autowired
    private Usuarios usuarios; 

    public Usuario logar(String login, String senha) {
        try {
            return em.createQuery("SELECT u FROM Usuario u WHERE u.username = :login AND u.password = :senha", Usuario.class)
                    .setParameter("login", login)
                    .setParameter("senha", senha)
                    .getSingleResult();
        } catch (NoResultException e) {
            return null;
        }
    }
}

Your login method looks like this:

@RequestMapping("/efetuaLogin")
public ModelAndView login(HttpServletRequest request, HttpServletResponse response) {
    List<Usuario> usuariosCadastrados = usuarios.lista();
    String username = request.getParameter("username");
    String password = request.getParameter("password");
    Usuario usuario = usuarios.logar(username, password)';
    if (usuario != null) return new ModelAndView("/documentos");
    return new ModelAndView("hello");
}

In addition, you are bringing the data from the database, through the Usuarios class and not through the LoginService class. However, you did not put the code of the Usuarios class in your question, so you can not tell if everything is all right or not. However, since you are now using a new method to search the database, the Usuarios class will also have to be changed.

And finally, let's see the rest of your controller:

It inherits from HttpServlet , which suggests that a single instance of this class will be created and shared by all requests. However, you have put the methods setPassword , getPassword , setUsername , getUsername , and username , password , request , and response , all of which are specific to each particular request. The result of this will be bad because you are putting in a place shared between all requests, data that belong to each request individually, and so if two requests use these methods and fields simultaneously the result will be disastrous.

To solve this problem, simply delete such methods and fields, as they are unnecessary since the data about them that you may need are already in your login method.

    
22.07.2016 / 08:48