/api-contract-review

name: api-contract-review
description: Review REST API contracts for HTTP semantics, versioning, backward compatibility, and response consistency. Use when user asks "review API", "check endpoints", "REST review", or before releasing API changes.
license: MIT

API Contract Review Skill

Audit REST API design for correctness, consistency, and compatibility.

When to Use

• User asks "review this API" / "check REST endpoints"
• Before releasing API changes
• Reviewing PR with controller changes
• Checking backward compatibility

---

Quick Reference: Common Issues

| Issue | Symptom | Impact | |-------|---------|--------| | Wrong HTTP verb | POST for idempotent operation | Confusion, caching issues | | Missing versioning | `/users` instead of `/v1/users` | Breaking changes affect all clients | | Entity leak | JPA entity in response | Exposes internals, N+1 risk | | 200 with error | `{"status": 200, "error": "..."}` | Breaks error handling | | Inconsistent naming | `/getUsers` vs `/users` | Hard to learn API |

---

HTTP Verb Semantics

Verb Selection Guide

| Verb | Use For | Idempotent | Safe | Request Body | |------|---------|------------|------|--------------| | GET | Retrieve resource | Yes | Yes | No | | POST | Create new resource | No | No | Yes | | PUT | Replace entire resource | Yes | No | Yes | | PATCH | Partial update | No* | No | Yes | | DELETE | Remove resource | Yes | No | Optional |

*PATCH can be idempotent depending on implementation

Common Mistakes

// ❌ POST for retrieval
@PostMapping("/users/search")
public List<User> searchUsers(@RequestBody SearchCriteria criteria) { }

// ✅ GET with query params (or POST only if criteria is very complex)
@GetMapping("/users")
public List<User> searchUsers(
    @RequestParam String name,
    @RequestParam(required = false) String email) { }

// ❌ GET for state change
@GetMapping("/users/{id}/activate")
public void activateUser(@PathVariable Long id) { }

// ✅ POST or PATCH for state change
@PostMapping("/users/{id}/activate")
public ResponseEntity<Void> activateUser(@PathVariable Long id) { }

// ❌ POST for idempotent update
@PostMapping("/users/{id}")
public User updateUser(@PathVariable Long id, @RequestBody UserDto dto) { }

// ✅ PUT for full replacement, PATCH for partial
@PutMapping("/users/{id}")
public User replaceUser(@PathVariable Long id, @RequestBody UserDto dto) { }

@PatchMapping("/users/{id}")
public User updateUser(@PathVariable Long id, @RequestBody UserPatchDto dto) { }

---

API Versioning

Strategies

| Strategy | Example | Pros | Cons | |----------|---------|------|------| | URL path | `/v1/users` | Clear, easy routing | URL changes | | Header | `Accept: application/vnd.api.v1+json` | Clean URLs | Hidden, harder to test | | Query param | `/users?version=1` | Easy to add | Easy to forget |

Recommended: URL Path

// ✅ Versioned endpoints
@RestController
@RequestMapping("/api/v1/users")
public class UserControllerV1 { }

@RestController
@RequestMapping("/api/v2/users")
public class UserControllerV2 { }

// ❌ No versioning
@RestController
@RequestMapping("/api/users")  // Breaking changes affect everyone
public class UserController { }

Version Checklist

• [ ] All public APIs have version in path
• [ ] Internal APIs documented as internal (or versioned too)
• [ ] Deprecation strategy defined for old versions

---

Request/Response Design

DTO vs Entity

// ❌ Entity in response (leaks internals)
@GetMapping("/{id}")
public User getUser(@PathVariable Long id) {
    return userRepository.findById(id).orElseThrow();
    // Exposes: password hash, internal IDs, lazy collections
}

// ✅ DTO response
@GetMapping("/{id}")
public UserResponse getUser(@PathVariable Long id) {
    User user = userService.findById(id);
    return UserResponse.from(user);  // Only public fields
}

Response Consistency

// ❌ Inconsistent responses
@GetMapping("/users")
public List<User> getUsers() { }  // Returns array

@GetMapping("/users/{id}")
public User getUser(@PathVariable Long id) { }  // Returns object

@GetMapping("/users/count")
public int countUsers() { }  // Returns primitive

// ✅ Consistent wrapper (optional but recommended for large APIs)
@GetMapping("/users")
public ApiResponse<List<UserResponse>> getUsers() {
    return ApiResponse.success(userService.findAll());
}

// Or at minimum, consistent structure:
// - Collections: always wrapped or always raw (pick one)
// - Single items: always object
// - Counts/stats: always object { "count": 42 }

Pagination

// ❌ No pagination on collections
@GetMapping("/users")
public List<User> getAllUsers() {
    return userRepository.findAll();  // Could be millions
}

// ✅ Paginated
@GetMapping("/users")
public Page<UserResponse> getUsers(
    @RequestParam(defaultValue = "0") int page,
    @RequestParam(defaultValue = "20") int size) {
    return userService.findAll(PageRequest.of(page, size));
}

---

HTTP Status Codes

Success Codes

| Code | When to Use | Response Body | |------|-------------|---------------| | 200 OK | Successful GET, PUT, PATCH | Resource or result | | 201 Created | Successful POST (created) | Created resource + Location header | | 204 No Content | Successful DELETE, or PUT with no body | Empty |

Error Codes

| Code | When to Use | Common Mistake | |------|-------------|----------------| | 400 Bad Request | Invalid input, validation failed | Using for "not found" | | 401 Unauthorized | Not authenticated | Confusing with 403 | | 403 Forbidden | Authenticated but not allowed | Using 401 instead | | 404 Not Found | Resource doesn't exist | Using 400 | | 409 Conflict | Duplicate, concurrent modification | Using 400 | | 422 Unprocessable | Semantic error (valid syntax, invalid meaning) | Using 400 | | 500 Internal Error | Unexpected server error | Exposing stack traces |

Anti-Pattern: 200 with Error Body

// ❌ NEVER DO THIS
@GetMapping("/