-2
\$\begingroup\$

I read a book named Effective SQL and found this interesting part.

Enforcing and maintaining business rules and relationships in the data is part of the data model, and the responsibility belongs to the database, not the application program. Data rules should be separated from the applications in order to ensure that everyone is working with the same data and that updates are done one way.

Does this mean I should change below code ?

private User googleSignup(UserDTO.GoogleUser googleUser) {
    userRepository.findByUsername(googleUser.getName()).ifPresent(user -> {
        throw new DuplicatedEntityException(ErrorCode.DUPLICATED_USERNAME);
    });
    
    ...
    User user = User.ofSocial(googleUser.getName(), googleUser.getEmail());
    return userRepository.save(user);
}

like this. Should I change to this code? (Data Integrity is already set up)

private User googleSignup(UserDTO.GoogleUser googleUser) {
    User user = User.ofSocial(googleUser.getName(), googleUser.getEmail());
    return userRepository.save(user);
}

With second code, calling DB count is reduced and the codes are simplified but I've never seen a code like this. Can you please share your opinion?

\$\endgroup\$
1

1 Answer 1

0
\$\begingroup\$

You're essentially asking about CREATE UNIQUE INDEX, such as a table's PK, and how to interact with it.

Firstly, in the system you describe there absolutely should be a UNIQUE key. Databases are very good at enforcing that kind of thing. What if there wasn't one, and the app tried to prevent dups using the OP code? Well, think about these two concurrent calls racing:
googleSignup(alice);
googleSignup(alice);
Both of them get empty result from .findByUsername("alice"), then execute the ... part for a while, then both successfully .save(). That is, we suffered from a TOCTTOU bug, we INSERTed a pair of rows, and thereafter we'll never know which one to believe or to update when we see Alice again. So we need reliable arbitration at a foundational level. It could come from a mutex service that is global across all webserver instances, but an RDBMS would be most convenient here.

Secondly, while the alternative piece of code seems more production ready, I do worry a little about the UX. It has a DuplicatedEntityException failure mode, which we don't specifically handle, so I imagine some generic user-unfriendly "unique key constraint" database message comes back to the user. The code is correct, but I anticipate a bug / enhancement request may be logged against it, if users encounter that frequently, find the response confusing, and complain about it. We might choose to catch the error, not because we're worried about corrupting another layer's bits, but because we wish to hold the user's hand, explain what happened, and suggest some new action the user might attempt in order to win on the second try.

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.