#12 Service class

[Charlottehenriksson]

Related Issue(s)

12

Description

Changed a typo in repository folder class name, Service class for user and messages created, dont know if its needed to add a set privacy on messages in service layer

[Charlottehenriksson]

public UserDto createUser(UserDto userDto) { if (!userRepository.findByEmail(userDto.email()).isEmpty()) { throw new CustomExceptions.EmailAlreadyExistsException("Email already exists: " + userDto.email()); } User user = UserDto.map(userDto); User savedUser = userRepository.save(user); return UserDto.map(savedUser); }

Im having a bit of trouble here, the other exception throws are similar to the messageService class but this one im not sure how to change so it will be similiar to the messageService class :)

[Ebyrdeu]

The issue stems from the implementation of the Repository interface. For instance, in your case, the findByEmail method returns all users with that email (which doesn't quite make sense if you think about it we have unique email. I overlooked this previously, so my apologies for that).

However, what you need to do is go to the UserRepository and make the necessary changes.

@Repository
public interface UserRepository extends ListCrudRepository<User, Long> {
    List<User> findByProfileName(String profileName);
    List<User> findByFirstName(String firstName);
    List<User> findByLastName(String lastName);
    Optional<User> findByEmail(String email);
}

[Charlottehenriksson]

The issue stems from the implementation of the Repository interface. For instance, in your case, the findByEmail method returns all users with that email (which doesn't quite make sense if you think about it we have unique email. I overlooked this previously, so my apologies for that).

However, what you need to do is go to the UserRepository and make the necessary changes.

@Repository
public interface UserRepository extends ListCrudRepository<User, Long> {
    List<User> findByProfileName(String profileName);
    List<User> findByFirstName(String firstName);
    List<User> findByLastName(String lastName);
    Optional<User> findByEmail(String email);
}

I have updated the repository! And I have changed createUser in the code, hopefully its correct :)

[ValentinaSukonina]

The use of Optional conveys the possibility of a nullable result explicitly, and it's beneficial for methods that may or may not find a matching object. On the other hand, returning a List implies that there could be multiple matches (although it's unusual for email addresses to have multiple users, we however did not specify this parameter as unique), and if no matches are found, it returns an empty list. Eventually it is up to us how we want to handle it. I have no strong preferences and gladly accept your changes. Test for UserRepository is now modified accordingly, plus test for non-existing user is added.

[Ebyrdeu]

You forgot - @RequiredArgsConstructor in UserService

[Charlottehenriksson]

You forgot - @RequiredArgsConstructor in UserService

Fixed it! Good catch :)

[Ebyrdeu]

Sonnar compain that you don't use return value of userRepository.findById(userId)

public void deleteUser(Long userId) {
        userRepository.findById(userId)
                .orElseThrow(() -> new CustomExceptions.NotFoundException("User not found with id: " + userId));`

    @CacheEvict("users")
    public void deleteUser(Long userId) {
        User  user = userRepository.findById(userId)
                .orElseThrow(() -> new CustomExceptions.NotFoundException("User not found with id: " + userId));

        userRepository.deleteById(user.getId());
    }

[Charlottehenriksson]

Sonnar compain that you don't use return value of userRepository.findById(userId)

public void deleteUser(Long userId) {
        userRepository.findById(userId)
                .orElseThrow(() -> new CustomExceptions.NotFoundException("User not found with id: " + userId));`

    @CacheEvict("users")
    public void deleteUser(Long userId) {
        User  user = userRepository.findById(userId)
                .orElseThrow(() -> new CustomExceptions.NotFoundException("User not found with id: " + userId));

        userRepository.deleteById(user.getId());
    }

Fixed the return value and all checks passed :)

[sonarcloud[bot]]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
3.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[Charlottehenriksson]

Goot work with updating delete methods. Sonar still complains about using Stream.collect(Collectors.toList() instead of Stream.toList(). The toList() method was introduced in Java 16 as a shorthand for collect(Collectors.toList()), providing a more concise way to collect elements into a list. It offers the same behavior and performance characteristics as collect(Collectors.toList()). In summary, if we are using Java 16 or later, toList() is preferred for its brevity and readability. Otherwise, using collect(Collectors.toList()) is perfectly fine. Any suggestions from others? Otherwise it looks good!

Great explanation! The methods using collectors.toList() are now updated :D