Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .claude/skills/multi-tool-code-review/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ Follow `04-fix-protocol.md`. If the harness supports plan mode, enter it first a
## Conventions (apply throughout)

- Ask if you are unsure of anything rather than assuming. Follow the host repo's `AGENTS.md` / `CLAUDE.md` closely.
- **Always hand back full absolute paths, on their own line.** Every artifact you write (the triage doc in each format, and any raw runner output you point at) gets its real path via `realpath`, never a bare filename or a repo-relative fragment buried in a sentence. The user clicks these to open them, and a path that is not absolute is not clickable. Reviews often run from a git worktree while the user sits in the main checkout, so resolve the path instead of assuming a shared working directory, and say which checkout it is in. See "Delivering the doc" in `03-triage-doc-format.md`.
- **Always hand back artifacts as `[filename](file:///absolute/path)` Markdown links, one per line.** Every artifact you write (the triage doc in each format, and any raw runner output you point at) gets its real path via `realpath`, wrapped in a Markdown link with a `file://` target. That is the only form the user can click; a bare absolute path, a bare `file://` URI, and any `vscode://` variant were all tested and none of them work. Reviews often run from a git worktree while the user sits in the main checkout, so resolve the path instead of assuming a shared working directory, and say which checkout it is in. See "Delivering the doc" in `03-triage-doc-format.md`.
- **The HTML variant follows the system colour scheme, dark by default.** Base palette dark in `:root`, light via `@media (prefers-color-scheme: light)`, print forced light, every colour a CSS variable. Full rules in `03-triage-doc-format.md`.
- No em dashes anywhere (chat, docs, commits, comments).
- Do not hardcode any model; ask the user each run and recommend from a fresh online check.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,17 @@ For the HTML variant, build a single self-contained page with the `frontend-desi

Self-contained means genuinely self-contained: inline the CSS, use system font stacks, and reference no CDN, webfont, or image. The page is opened over `file://`, often with no network, and anything external renders as a broken document.

### The HTML layout is built for reading a wide table

The whole point of the HTML variant is that a long table is easier to scan than Markdown, so the layout must give the table room. These rules are not cosmetic preferences; each one fixes a way the page became unreadable in practice:

- **Use the full browser width. Do not put the page in a centred fixed-width container.** A `max-width` on the wrapper squeezes nine columns into a column of text and forces horizontal scrolling on a screen that had plenty of room. Prose blocks can keep their own reading measure, but the tables get the whole viewport.
- **The Finding column needs roughly three times the width the browser gives it by default.** Left to itself it collapses to one word per line and the table becomes unreadable vertical confetti. Give it a generous `min-width`.
- **The Where column should be about half its natural width.** It holds a `file:line`, which is the least important cell on the row, and letting it sit `nowrap` lets one long path dictate the whole table's geometry. Let it wrap and break on the path separators.
- **Never render Now, Expected and Test as one paragraph.** They are three distinct things and a reader scans for one of them at a time. Split the cell into a separate block per part, each with its own label, so the eye can land on Test without reading Now first.

Because the Maintainability tail uses a different, shorter column set, key any positional column rules to the main table only (for example by tagging the nine-column tables with a class), or the tail's cells inherit widths meant for columns it does not have.

### The HTML must follow the system colour scheme

Default to **dark**, and let a light system preference override it. Not the other way round: the user's environment is dark nearly all the time, so dark is the right base and the right fallback when the preference is unknown.
Expand Down Expand Up @@ -132,7 +143,24 @@ PY

## Delivering the doc

Give the user the **full absolute path**, on its own line, for every artifact you wrote. Terminal and desktop chat interfaces turn an absolute path into a clickable link, and clicking is how the user actually opens these. A bare filename, a repo-relative path, or a path in prose is not clickable and forces them to reconstruct it.
Hand back every artifact as a **Markdown link whose target is a `file://` URI**, built from the absolute path. This is the only form that is clickable here, confirmed by testing five variants against this user's terminal on 2026-08-06:

```markdown
[REVIEW-2026-08-06-DEV-VS-MAIN-BY-AREA.md](file:///home/silver/Desktop/Disscount/reviews/REVIEW-2026-08-06-DEV-VS-MAIN-BY-AREA.md)
```

Use the filename as the link text, and put each artifact on its own line. Do not bury a link mid-sentence.

What does NOT work, so do not fall back to any of it:

- A bare absolute path (`/home/silver/...`). Claude Code does not wrap paths in OSC 8 escapes, so a bare path is clickable only in terminals that auto-detect paths themselves, and this one does not.
- A bare `file://` URI as plain text.
- A `vscode://file/...` URI in either form.
- A bare filename, a repo-relative path, or a path inside prose.

Target the **system default handler** via `file://`, not an editor scheme. The two artifacts want different applications (an editor for the Markdown, a browser for the HTML) and `file://` lets the desktop pick correctly for each.

Note this applies to the artifacts you deliver. Inline `file.ts:42` references to source, which the harness renders as its own clickable reference, keep their existing form.

Get the directory right, not just the name. Reviews are frequently run from a **git worktree**, and `reviews/` is typically gitignored, so the file exists only under the worktree it was written in and no git operation will ever move it. Resolve the real path rather than assuming the user shares your working directory:

Expand Down
4 changes: 3 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ Ask first:
- Widening scope beyond what I asked for.
- Any instruction of mine that has two plausible readings. Ask before you edit, do not pick one and start.

**Ask means ask.** Every item above is a question to put to me, not a reason to pick the lesser option and move on. Do not work around a missing endpoint, skip the dependency and hand-roll it, or narrow the task to avoid the question. I say yes far more often than no, so quietly choosing the workaround costs me the better answer and I never learn the choice was there. If you are mid-task and cannot stop, do the parts that do not depend on the answer, then ask before you finish. Never let "I did not want to widen scope" be the reason something shipped worse.

Hand back interactive installers and `init` wizards, except `pnpm dlx shadcn@latest add <component>`, which you may run.

Safe without asking, run from `frontend/`:
Expand Down Expand Up @@ -96,7 +98,7 @@ Conventions:
- `I`-prefixed Props interfaces, in the same file as the component. One component per file, default export.
- `function name() {}`, not `const name = () => {}`, except for small inline callbacks.
- `import { useState } from "react"`, never `React.useState`.
- `components/ui/` is shadcn output, so do not hand-edit it. Our components live in `components/custom/`, grouped by concern.
- `components/ui/` is shadcn output. Editing it is allowed where the primitive is the natural home for the change, such as a prop the component itself should own or a sizing rule our `--spacing` override breaks. Our components live in `components/custom/`, grouped by concern.
- Types: API and domain go in `lib/api/schemas/*` as zod `*Dto` / `*Response`; external price API types in `lib/cijene-api/schemas.ts`; shared UI types in `@/typings`; feature-only types stay colocated in `*-types.ts`.
- React Query hooks live next to their service in `lib/api/<domain>/`. Feature composition hooks go in the feature's `hooks/`.
- Before generating or redesigning UI, read `frontend/.github/skills/frontend-design/SKILL.md` and follow it.
Expand Down
47 changes: 31 additions & 16 deletions backend/src/main/java/disscount/config/SecurityConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import org.springframework.security.oauth2.jwt.NimbusJwtDecoder;
import org.springframework.security.oauth2.server.resource.web.BearerTokenAuthenticationFilter;
import org.springframework.security.web.SecurityFilterChain;
import org.springframework.security.web.util.matcher.OrRequestMatcher;
import org.springframework.security.web.util.matcher.RequestMatcher;

@Configuration
@EnableWebSecurity
Expand Down Expand Up @@ -46,7 +48,7 @@ public JwtDecoder jwtDecoder(
* {@code @Component} filter meant only for a security chain runs twice: once where it was
* placed and once for every request that never reaches that chain. Both of these extend
* {@code OncePerRequestFilter}, whose already-filtered attribute makes the second run a
* no-op only when the first one happened, so on any path outside {@code /api/shared/**}
* no-op only when the first one happened, so on any path outside the optional-auth chain
* the optional bearer filter would decode the token again after the real chain had
* finished with it. These beans turn the servlet registration off and leave the security
* chains as the only place either filter runs.
Expand All @@ -70,37 +72,50 @@ public FilterRegistrationBean<OptionalBearerAuthenticationFilter> optionalBearer
}

/**
* Shared lists get their own chain because they are the one place where a bearer token is
* optional. Authorization happens on the share token plus ShoppingListAccessService, and
* the caller may legitimately be anonymous, so a token that fails to decode must degrade
* to anonymous rather than 401. permitAll on the main chain cannot express that: its
* bearer filter rejects a stale token before authorization is ever consulted.
* The by-id shopping list routes, which are the one place a bearer token is optional.
* A list is shared by its own id, so the caller may legitimately be anonymous and a token
* that fails to decode must degrade to anonymous rather than 401. permitAll on the main
* chain cannot express that: its bearer filter rejects a stale token before authorization
* is ever consulted.
*
* <p><b>Nothing here authorizes anything.</b> The rule is {@code permitAll}, so the
* access checks in {@link disscount.shoppingList.service.ShoppingListService} are an
* authentication boundary: relaxing one removes authentication, not a convenience.
*/
@Bean
@Order(1)
public SecurityFilterChain sharedShoppingListChain(
public SecurityFilterChain optionalAuthShoppingListChain(
HttpSecurity http,
OptionalBearerAuthenticationFilter optionalBearerAuthenticationFilter
) throws Exception {
http
.securityMatcher("/api/shared/**")
.securityMatcher(shoppingListByIdMatcher())
.csrf(csrf -> csrf.disable())
.sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.STATELESS))
.authorizeHttpRequests(authz -> authz.anyRequest().permitAll())
// Both are anchored on the slot where a bearer token is normally decoded, which
// is where these two belong and which leaves them 98 places of headroom before
// the next registered filter. addFilterAfter is order + 1, so this is bearer at
// +1 and provisioning at +2: a strict sequence, which provisioning needs because
// it acts on the authentication the bearer filter produced. Anchoring either one
// on AnonymousAuthenticationFilter instead lands exactly on top of it, since a
// before is order - 1 and the following after adds the 1 straight back, and the
// resulting tie is broken only by the sort happening to be stable.
// Anchored on the bearer slot so provisioning lands strictly after it. Anchoring
// on AnonymousAuthenticationFilter ties with it, since before is -1 and the
// following after adds the 1 straight back.
.addFilterAfter(optionalBearerAuthenticationFilter, BearerTokenAuthenticationFilter.class)
.addFilterAfter(userProvisioningFilter, OptionalBearerAuthenticationFilter.class);

return http.build();
}

/** Unlisted methods fall through to the authenticated chain, so the default is deny. */
private static RequestMatcher shoppingListByIdMatcher() {
return new OrRequestMatcher(
new UuidScopedRequestMatcher(HttpMethod.GET, "/api/shopping-lists/{id}"),
// Link unfurlers probe with HEAD, which the matcher compares exactly.
new UuidScopedRequestMatcher(HttpMethod.HEAD, "/api/shopping-lists/{id}"),
new UuidScopedRequestMatcher(HttpMethod.PUT, "/api/shopping-lists/{id}"),
new UuidScopedRequestMatcher(HttpMethod.DELETE, "/api/shopping-lists/{id}"),
new UuidScopedRequestMatcher(HttpMethod.POST, "/api/shopping-lists/{id}/items"),
new UuidScopedRequestMatcher(HttpMethod.POST, "/api/shopping-lists/{id}/copy"),
new UuidScopedRequestMatcher(HttpMethod.PUT, "/api/shopping-lists/{id}/items/{itemId}"),
new UuidScopedRequestMatcher(HttpMethod.DELETE, "/api/shopping-lists/{id}/items/{itemId}"));
}

@Bean
@Order(2)
public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package disscount.config;

import jakarta.servlet.http.HttpServletRequest;
import org.springframework.http.HttpMethod;
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
import org.springframework.security.web.util.matcher.RequestMatcher;

import java.util.regex.Pattern;

/**
* Matches one method and path only when the {@code id} segment is UUID-shaped.
*
* <p>An allowlist on both axes. A wildcard minus the authenticated routes would be a
* denylist inside an allowlist, silently exposing the next literal route somebody adds;
* requiring a UUID excludes {@code /me}, {@code /items} and any future literal for free.
*
* <p>Canonical dashed form only. Bare 32-hex is excluded because Spring's
* StringToUUIDConverter rejects it, so it would only buy an anonymous caller a logged 500.
*/
final class UuidScopedRequestMatcher implements RequestMatcher {

private static final Pattern UUID_SHAPE = Pattern.compile(
"^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$");

private final AntPathRequestMatcher delegate;

UuidScopedRequestMatcher(HttpMethod method, String pattern) {
this.delegate = new AntPathRequestMatcher(pattern, method.name());
}

@Override
public boolean matches(HttpServletRequest request) {
MatchResult result = delegate.matcher(request);
if (!result.isMatch()) {
return false;
}

String id = result.getVariables().get("id");
return id != null && UUID_SHAPE.matcher(id).matches();
}
Comment on lines +31 to +40

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Confirm the Spring Security version and the matcher API surface.
set -euo pipefail

fd -g 'pom.xml' backend --exec rg -n -C3 'spring-boot-starter-parent|spring-security|<version>' {}
fd -g 'build.gradle*' backend --exec rg -n -C3 'springframework.security|springframework.boot' {}

Repository: OffCrazyFreak/Disscount

Length of output: 1645


🌐 Web query:

Spring Security AntPathRequestMatcher matcher getVariables URI template variables

💡 Result:

In Spring Security, the AntPathRequestMatcher.extractUriTemplateVariables(HttpServletRequest request) method is deprecated [1][2][3]. While AntPathRequestMatcher implements the RequestVariablesExtractor interface, its primary purpose is request matching, and the ability to extract URI template variables via this class is no longer the recommended or standard way to handle path variables within Spring Security's authorization workflow [4][1][5]. If you need to access and use URI template variables for authorization purposes, the following approaches are recommended: 1. Spring Expression Language (SpEL) in Authorization Rules: You can use template variables directly in your security configuration using SpEL. If you define a pattern with a path variable (e.g., /user/{id}), you can reference that variable in an .access() expression. For example:.antMatchers("/user/{id}/").access("@mySecurityGuard.check(#id, authentication)") In this case, Spring Security automatically resolves the #id placeholder from the path variable [6]. 2. Use PathPatternRequestMatcher: For modern Spring MVC applications, Spring Security recommends using PathPatternRequestMatcher [7]. This matcher integrates more deeply with Spring MVC's URL matching logic and is better suited for applications that utilize sophisticated path pattern parsing [7]. 3. HandlerMapping: If you are working outside the standard security expression context and need to access the variables manually, you can retrieve them from the request attributes using the constant HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE [6]. The extractUriTemplateVariables method on AntPathRequestMatcher is deprecated and should be avoided in favor of these more integrated approaches [1][3][8].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== pom files and Spring versions =="
for f in $(fd -g 'pom.xml' backend); do
  echo "-- $f"
  sed -n '1,140p' "$f" | nl -ba
done

echo "== UuidScopedRequestMatcher source =="
for f in $(fd -g 'UuidScopedRequestMatcher.java' backend/src/main/java); do
  echo "-- $f"
  wc -l "$f"
  sed -n '1,140p' "$f" | nl -ba
done

echo "== matcher usages =="
rg -n "UuidScopedRequestMatcher|new AntPathRequestMatcher|matchers\\(|requestMatchers" backend/src/main/java backend/src/test/java || true

echo "== Search package-lock / dependency versions for org.springframework.security artifacts =="
rg -n "org:springframework:security|versioning|springSecurity" backend pom.xml backend/pom.xml backend/src/main/java backend/src/test/java 2>/dev/null || true

Repository: OffCrazyFreak/Disscount

Length of output: 256


🌐 Web query:

Spring Security 6.1.3 AntPathRequestMatcher RequestMatcher matcher getVariables API ant match variables request result

💡 Result:

In Spring Security 6.1.3, the AntPathRequestMatcher class is designed for pattern matching against an HttpServletRequest URL (servletPath + pathInfo) and does not natively support capturing URI template variables via the matcher API in the way Spring MVC does [1][2]. While AntPathRequestMatcher implements the RequestVariablesExtractor interface, the implementation of matcher(HttpServletRequest) returns a MatchResult where the default implementation of getVariables returns an empty map [1]. Additionally, the method extractUriTemplateVariables(HttpServletRequest) is marked as @Deprecated [1]. If your goal is to extract path variables from a request within Spring Security, you should consider the following approaches: 1. Use MvcRequestMatcher: This class utilizes Spring MVC's HandlerMappingIntrospector to match paths and is the standard way to handle path variables within Spring Security [3]. 2. Use PathPatternRequestMatcher: Available in more recent versions of Spring Security (specifically introduced/standardized for this purpose in later versions), this matcher is explicitly designed to handle path patterns with placeholders (e.g., /path/{value}/) and capture those values in RequestAuthorizationContext.getVariables [4][5]. For simple Ant-style matching where no variable extraction is required, AntPathRequestMatcher remains functional, but it should not be relied upon for extracting dynamic path parameters [1][2].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import subprocess, pathlib, re, os
os.chdir('/testbed')  # sandbox cwd
root = pathlib.Path('backend/src/main/java/disscount/config/UuidScopedRequestMatcher.java')
if root.exists():
    lines = root.read_text().splitlines()
    for i, line in enumerate(lines, start=1):
        print(f"{i:4}: {line}")
else:
    print("UuidScopedRequestMatcher.java not found")

pom = pathlib.Path('backend/pom.xml')
if pom.exists():
    s = pom.read_text()
    m = re.search(r"<spring-boot-starter-parent>.*?</spring-boot-starter-parent>", s, re.S)
    print("\nBackend Spring parent block:")
    print(m.group(0) if m else "none")
PY

Repository: OffCrazyFreak/Disscount

Length of output: 301


🌐 Web query:

Spring Security 6.1.4 AntPathRequestMatcher RequestMatcher matcher HttpServletRequest MatchResult getVariables implementation

💡 Result:

In Spring Security 6.1.4, the AntPathRequestMatcher implementation of the matcher(HttpServletRequest) method does not automatically extract path variables into the returned MatchResult [1]. The RequestMatcher interface defines the matcher(HttpServletRequest) method, which returns a MatchResult object [2][3]. By default, the RequestMatcher interface provides a default implementation for matcher(request) that invokes the matches(request) method and returns a MatchResult containing an empty map for variables [3]. While AntPathRequestMatcher implements RequestVariablesExtractor [1], its primary matcher(HttpServletRequest) method is generally used for simple boolean matching [1]. If you require extracted path variables (such as template variables), the recommended approach in modern Spring Security (especially for Spring MVC integration) is to use PathPatternRequestMatcher instead of AntPathRequestMatcher, as it is designed to align with Spring MVC's path pattern parsing and variable extraction logic [4]. Key details regarding the MatchResult and variables: 1. RequestMatcher.MatchResult: This class holds the match status (boolean) and a map of variables [5][3]. The getVariables method returns a Map<String, String> containing key-value pairs of extracted variable names and values [5][6]. 2. Default Behavior: When matcher(HttpServletRequest) is called on an AntPathRequestMatcher, it does not populate this map with path variables; it returns Collections.emptyMap [1][3]. 3. Alternative: For path variable extraction, use PathPatternRequestMatcher, which integrates directly with Spring MVC's pattern parsing [4].

Citations:


🌐 Web query:

Spring Security 6.1.4 PathPatternRequestMatcher builder patternMatch getVariables URI variables

💡 Result:

In Spring Security, the PathPatternRequestMatcher.Builder allows you to define path patterns that can capture URI variables, which are then accessible through the RequestAuthorizationContext. When using patterns like /path/{value}/, the {value} segment is captured as a URI variable [1][2][3]. To use this functionality, you create a matcher using the builder, for example: PathPatternRequestMatcher.Builder mvc = PathPatternRequestMatcher.withDefaults.basePath("/api"); http.authorizeHttpRequests(authorize -> authorize.requestMatchers(mvc.matcher("/user/{id}/")).hasAuthority("user")); When a request matches the pattern, the captured variables (such as "id") are stored in the RequestAuthorizationContext [1][2][3]. You can access these variables during authorization by using the RequestAuthorizationContext object in your custom authorization logic: http.authorizeHttpRequests(authorize -> authorize.requestMatchers(mvc.matcher("/user/{id}/")).access((authentication, context) -> { Map<String, Object> variables = context.getVariables; String userId = (String) variables.get("id"); // Apply custom logic using userId return new AuthorizationDecision(true); })); Note that the PathPatternRequestMatcher returns a MatchResult when matcher(HttpServletRequest) is called [4][5][6]. While the default implementation of some matcher methods might return empty maps, the framework-level integration with authorizeHttpRequests automatically handles the extraction and population of these variables into the RequestAuthorizationContext for you to access as shown above [1][2][3]. Top results: [1][2][3]

Citations:


Use a matcher that captures path variables.

With Spring Boot 3.1, AntPathRequestMatcher.matcher(...) returns a MatchResult, but getVariables() stays empty, so this wrapper always rejects {id} paths. Replace the delegate with PathPatternRequestMatcher or another variable-aware matcher, and add tests for a UUID path and a literal path.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/main/java/disscount/config/UuidScopedRequestMatcher.java` around
lines 38 - 47, Update UuidScopedRequestMatcher’s delegate used by matches so it
uses a variable-aware matcher such as PathPatternRequestMatcher, allowing
result.getVariables().get("id") to resolve for UUID paths. Preserve rejection of
non-matching and invalid IDs, and add tests covering both a valid UUID path and
a literal path.

}
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
package disscount.exceptions;

import org.hibernate.exception.ConstraintViolationException;
import org.springframework.dao.DataIntegrityViolationException;
import org.springframework.dao.DuplicateKeyException;
import org.springframework.http.HttpStatus;
import org.springframework.http.ProblemDetail;
import org.springframework.http.converter.HttpMessageNotReadableException;
import org.springframework.validation.FieldError;
import org.springframework.web.bind.MethodArgumentNotValidException;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.RestControllerAdvice;
import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.net.URI;
import java.sql.SQLException;
import java.time.Instant;
import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -40,11 +45,68 @@ public ProblemDetail handleForbiddenException(ForbiddenException ex) {
return problem(HttpStatus.FORBIDDEN, "forbidden", "Zabranjeno", ex.getMessage());
}

@ExceptionHandler(NotFoundException.class)
public ProblemDetail handleNotFoundException(NotFoundException ex) {
return problem(HttpStatus.NOT_FOUND, "not-found", "Nije pronađeno", ex.getMessage());
}

@ExceptionHandler(ConflictException.class)
public ProblemDetail handleConflictException(ConflictException ex) {
return problem(HttpStatus.CONFLICT, "conflict", "Sukob", ex.getMessage());
}

/**
* Hibernate reports a unique-index violation as DataIntegrityViolationException, not
* DuplicateKeyException, so both are caught and narrowed here. Not-null, check and
* foreign-key violations are bugs rather than conflicts and rethrow to the 500.
* Nothing from the driver message is logged: PostgreSQL puts the colliding value in it.
*/
@ExceptionHandler(DataIntegrityViolationException.class)
public ProblemDetail handleDuplicateKey(DataIntegrityViolationException ex) {
// Rethrowing would bypass this advice entirely and surface a container 500, so the
// non-conflict case is answered here instead.
if (!(ex instanceof DuplicateKeyException) && !isUniqueViolation(ex)) {
return handleGenericException(ex);
}

String constraint = constraintNameOf(ex);
log.warn("Duplicate key violation on constraint: {}", constraint);

boolean isUsername = constraint.toLowerCase().contains("username");

ProblemDetail detail = problem(HttpStatus.CONFLICT, "conflict", "Sukob",
isUsername ? "Korisničko ime je već zauzeto." : "Vrijednost je već zauzeta.");

if (isUsername) {
detail.setProperty("fieldErrors", Map.of("username", "Korisničko ime je već zauzeto."));
}

return detail;
}

/** SQLState 23505 is unique_violation in both PostgreSQL and the H2 the tests run on. */
private static boolean isUniqueViolation(Throwable ex) {
for (Throwable cause = ex; cause != null; cause = cause.getCause()) {
if (cause instanceof SQLException sql && "23505".equals(sql.getSQLState())) {
return true;
}
if (cause.getCause() == cause) {
break;
}
}
return false;
}

/** The constraint name only; the message around it carries the colliding value. */
private static String constraintNameOf(DataIntegrityViolationException ex) {
Throwable cause = ex.getCause();
if (cause instanceof ConstraintViolationException violation
&& violation.getConstraintName() != null) {
return violation.getConstraintName();
}
return ex.getClass().getSimpleName();
}

@ExceptionHandler(MethodArgumentNotValidException.class)
public ProblemDetail handleValidationExceptions(MethodArgumentNotValidException ex) {
Map<String, String> fieldErrors = new HashMap<>();
Expand All @@ -61,6 +123,13 @@ public ProblemDetail handleValidationExceptions(MethodArgumentNotValidException
return problemDetail;
}

/** Without this a malformed UUID is a logged 500, free for anyone to trigger. */
@ExceptionHandler(MethodArgumentTypeMismatchException.class)
public ProblemDetail handleTypeMismatch(MethodArgumentTypeMismatchException ex) {
return problem(HttpStatus.BAD_REQUEST, "bad-request", "Neispravan zahtjev",
"Neispravan format parametra: " + ex.getName());
}

@ExceptionHandler(HttpMessageNotReadableException.class)
public ProblemDetail handleNotReadable(HttpMessageNotReadableException ex) {
return problem(HttpStatus.BAD_REQUEST, "malformed-request", "Neispravan zahtjev",
Expand Down
Loading