Refactor: Replace brittle constraint-name string matching with proactive repository checks in AuthService

[Copilot]

Database constraint violations in AuthService were caught and identified by string-matching against e.getMessage() (e.g., contains("users_email_key")), making the logic fragile and silently broken if constraints are renamed or the DB driver changes.

Changes

• signUp: Replaced findByUsernameIgnoreCase(...).isPresent() / findByEmail(...).isPresent() with existsByUsernameIgnoreCase / existsByEmail. Stripped all constraint-name string matching from the DataIntegrityViolationException catch — a generic fallback is retained for race conditions.

• setUsername: Already had a proactive findByUsernameIgnoreCase guard. Removed the string-based error log interpolation from its DataIntegrityViolationException catch, keeping a clean generic handler.

• Tests: Updated testSignUp_duplicateEmail / testSignUp_duplicateUsername to mock the proactive repository checks (existsByEmail, existsByUsernameIgnoreCase) instead of simulating exception throws from save. Added verify(userRepository, never()).save(...) to assert no DB write occurs on duplicate detection.

Before:

} catch (DataIntegrityViolationException e) {
    if (e.getMessage().contains("users_email_key")) {
        authResponse.setMessage("Email already exists");
    } else if (e.getMessage().contains("idx_user_username") || e.getMessage().contains("users_username_key")) {
        authResponse.setMessage("Username already exists");
    } else { ... }
}

After:

if (userRepository.existsByUsernameIgnoreCase(request.getUsername())) {
    authResponse.setMessage("Username already exists");
    authResponse.setSuccess(false);
    return authResponse;
}
if (userRepository.existsByEmail(request.getEmail())) {
    authResponse.setMessage("Email already exists");
    authResponse.setSuccess(false);
    return authResponse;
}
// Generic DataIntegrityViolationException catch retained as a race-condition safety net

[fred-maina]

LGTM

[Copilot]

Pull request overview

This PR refactors AuthService’s duplicate-user handling to avoid brittle DataIntegrityViolationException message/constraint-name string matching by proactively querying the repository for existing usernames/emails, and updates unit tests accordingly.

Changes:

• In signUp, replaced findBy...().isPresent() checks with existsBy... checks and removed constraint-name parsing from the exception handler.
• In setUsername, removed string-based exception-message interpolation and kept a generic integrity-violation handler.
• Updated AuthServiceTest duplicate-email/username tests to mock proactive existsBy... checks and verify no save(...) occurs.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
src/main/java/com/fredmaina/chatapp/Auth/services/AuthService.java	Uses proactive repository existsBy... checks and simplifies DataIntegrityViolationException handling.
src/test/java/com/fredmaina/chatapp/Auth/Services/AuthServiceTest.java	Adjusts tests to validate early-return duplicate detection and avoid DB writes on duplicates.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

In the DataIntegrityViolationException handler, the error is logged without the exception object, which drops the stack trace and makes production debugging harder. Consider logging e as the throwable (and optionally using a stable identifier rather than only e.getMessage()).

Suggested change

	log.error("Data integrity violation during sign up: {}", e.getMessage());
	log.error("Data integrity violation during sign up: {}", e.getMessage(), e);

[Copilot]

After removing constraint-name matching, this catch-all now always returns the generic message "Data integrity violation". Since the PR intent is to keep this as a race-condition safety net, you can still provide a user-friendly duplicate email/username message by re-checking existsByUsernameIgnoreCase/existsByEmail inside the catch and only falling back to the generic message if neither exists. This keeps the logic resilient without depending on driver/constraint strings.

Suggested change

	authResponse.setMessage("Data integrity violation");
	if (request.getUsername() != null && userRepository.existsByUsernameIgnoreCase(request.getUsername())) {
	authResponse.setMessage("Username already exists");
	} else if (userRepository.existsByEmail(request.getEmail())) {
	authResponse.setMessage("Email already exists");
	} else {
	authResponse.setMessage("Data integrity violation");
	}

[Copilot]

Similar to signUp, this DataIntegrityViolationException log line only records e.getMessage() and omits the throwable, which can hide the stack trace and root cause. Consider including e in the log call so the full exception is captured.

Suggested change

	log.error("Data integrity violation while setting username for email {}: {}", email, e.getMessage());
	log.error("Data integrity violation while setting username for email {}: {}", email, e.getMessage(), e);

[Copilot]

testSignUp_duplicateEmail only stubs existsByEmail, implicitly relying on Mockito's default false return for the preceding existsByUsernameIgnoreCase call. To make the test intent clearer and avoid coupling to Mockito defaults, consider explicitly stubbing existsByUsernameIgnoreCase("fredmaina123") to false (or verifying it was called).

Suggested change

	when(userRepository.existsByUsernameIgnoreCase("fredmaina123")).thenReturn(false);

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Changing the duplicate-username message from "Username already exists (case-insensitive)" to "Username already exists" will break any callers that branch on the exact message text. For example, AuthController.register currently returns HTTP 409 only when the message equals the old string, so duplicate usernames will now likely return 400 instead of 409. Consider keeping the message stable or updating the controller to use the new message (or better: return/propagate a dedicated error code/status instead of matching on message strings).

Suggested change

	authResponse.setMessage("Username already exists");
	authResponse.setMessage("Username already exists (case-insensitive)");

[Copilot]

@CacheEvict uses key "#request.username.toLowerCase()" which will throw a SpEL evaluation error if signUp is ever called with a null username (and the method body already allows for null via request.getUsername() != null checks). To make this null-safe, add a condition = "#request.username != null" (or otherwise guard the key expression) so the cache eviction doesn't fail before the method executes.