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.