/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.

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("/{id}")
public