Guide · Java

How to review Java pull requests: the 9 defect classes

Java pull requests in Spring Boot look safe because the framework does so much: security filters, transactions, validation. None of that checks that a record belongs to the caller, that a count followed by a save is atomic or that a JPA mapping does not run one query per row. This guide walks the nine defect classes DiffDojo trains, in the order a reviewer should weigh them, with what each one looks like in Java and Spring Boot code.

1. Security

Look for request values reaching an interpreter: a JPQL or native query built by concatenation (a sort column is the classic case, because it cannot be a bound parameter), a file path from a request, or a redirect target. Then check object-level authorization: a findById behind Spring Security still needs a check that the entity belongs to the caller, or a query scoped by owner.

2. Requirement fidelity

Read the ticket before the code. The default sort, which statuses a query includes, whether a date range includes its last day and the field a total is computed from are where Java PRs most often do something reasonable that is not what was asked.

3. Edge cases & error handling

Ask what happens on empty, missing and huge input. Typical misses: Optional.get() without a check, a RestTemplate or RestClient built without connect and read timeouts, an uncaught NumberFormatException on user text, and an int that counts something able to grow past two billion.

4. Hallucinated APIs & dependencies

Models invent plausible methods and annotations: a Spring Data derived query on a property the entity does not have, a builder method a library never had, an annotation attribute from another version. The compiler catches a missing method, but a bad derived query fails only when the application context starts (by default), and a wrong configuration key may never fail at all.

5. State & data flow

Spring beans are singletons by default, so a mutable field on a @Service or @RestController is shared by every request and every thread. Look for per-request data stored in a field, a non-thread-safe object such as SimpleDateFormat held in a field, and caches that are never invalidated.

6. Performance

Look for work that grows with the input: a lazy JPA collection read for each entity while mapping to a DTO (N+1), a repository call per id inside a loop, or List.contains inside a loop over a large input. With a handful of rows in test data none of this is noticeable, so ask what the code does at the production size the ticket implies.

7. Data correctness

Money and time are the usual suspects. double for currency, BigDecimal.equals (which also compares scale, so 2.0 and 2.00 differ) where compareTo was meant, rounding at the wrong step, and a LocalDateTime where an instant in a known time zone was needed all produce code that runs and returns the wrong number.

8. Deceptive tests

A green test proves only what it asserts. Check for tests that mock the class under test, verify that a mock was called instead of checking the result, or call the method and assert nothing.

9. Over-engineering

Not a bug, but a review comment: an interface with one implementation and no second in sight, a factory or strategy hierarchy for two branches, or a new dependency for what the JDK already does. Ask for the simple version.

The checklist

1. Does any request value reach a query string, a file path or a redirect?
2. Does the change do exactly what the ticket says, field by field?
3. Empty, missing, huge input: what happens, and do outbound calls have timeouts?
4. Does every unfamiliar method, annotation and derived query actually exist?
5. Is any mutable per-request state held in a singleton bean?
6. Is there a query or a lazy load inside a loop?
7. Are money and time handled with BigDecimal (compareTo) and the right time type?
8. Would the tests fail if the code were wrong?
9. Is there a simpler way to write this?

As of October 2026, the DiffDojo library has 23 live Java PRs to practise on, each graded on whether you catch the planted bug or approve the clean one.

All review guides · All code review examples