Imported from Splatcrafter/sumbooklm (.devcontainer/claude/skills/api-contract-review/SKILL.md). Install upstream with npx skills add Splatcrafter/sumbooklm --skill api-contract-review. Copyright stays with the author.

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

HTTP Verb Semantics

Verb Selection Guide

*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

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

Error Codes

Anti-Pattern: 200 with Error Body

// ❌ NEVER DO THIS
@GetMapping("/{id}")
public ResponseEntity<Map<String, Object>> getUser(@PathVariable Long id) {
    try {
        User user = userService.findById(id);
        return ResponseEntity.ok(Map.of("status", "success", "data", user));
    } catch (NotFoundException e) {
        return ResponseEntity.ok(Map.of(  // Still 200!
                "status", "error",
                "message", "User not found"
        ));
    }
}

// ✅ Use proper status codes
@GetMapping("/{id}")
public ResponseEntity<UserResponse> getUser(@PathVariable Long id) {
    return userService.findById(id)
            .map(ResponseEntity::ok)
            .orElse(ResponseEntity.notFound().build());
}

Error Response Format

Consistent Error Structure

// ✅ Standard error response
public class ErrorResponse {
    private String code;        // Machine-readable: "USER_NOT_FOUND"
    private String message;     // Human-readable: "User with ID 123 not found"
    private Instant timestamp;
    private String path;
    private List<FieldError> errors;  // For validation errors
}

// In GlobalExceptionHandler
@ExceptionHandler(ResourceNotFoundException.class)
public ResponseEntity<ErrorResponse> handleNotFound(
        ResourceNotFoundException ex, HttpServletRequest request) {
    return ResponseEntity.status(HttpStatus.NOT_FOUND)
            .body(ErrorResponse.builder()
                    .code("RESOURCE_NOT_FOUND")
                    .message(ex.getMessage())
                    .timestamp(Instant.now())
                    .path(request.getRequestURI())
                    .build());
}

Security: Don't Expose Internals

// ❌ Exposes stack trace
@ExceptionHandler(Exception.class)
public ResponseEntity<String> handleAll(Exception ex) {
    return ResponseEntity.status(500)
            .body(ex.getStackTrace().toString());  // Security risk!
}

// ✅ Generic message, log details server-side
@ExceptionHandler(Exception.class)
public ResponseEntity<ErrorResponse> handleAll(Exception ex) {
    log.error("Unexpected error", ex);  // Full details in logs
    return ResponseEntity.status(500)
            .body(ErrorResponse.of("INTERNAL_ERROR", "An unexpected error occurred"));
}

Backward Compatibility

Breaking Changes (Avoid in Same Version)

Non-Breaking Changes (Safe)

• Add optional field to request
• Add field to response
• Add new endpoint
• Add new optional query parameter

Deprecation Pattern

@RestController
@RequestMapping("/api/v1/users")
public class UserControllerV1 {

    @Deprecated
    @GetMapping("/by-email")  // Old endpoint
    public UserResponse getByEmailOld(@RequestParam String email) {
        return getByEmail(email);  // Delegate to new
    }

    @GetMapping(params = "email")  // New pattern
    public UserResponse getByEmail(@RequestParam String email) {
        return userService.findByEmail(email);
    }
}

API Review Checklist

1. HTTP Semantics

• GET for retrieval only (no side effects)
• POST for creation (returns 201 + Location)
• PUT for full replacement (idempotent)
• PATCH for partial updates
• DELETE for removal (idempotent)

2. URL Design

• Versioned (/v1/, /v2/)
• Nouns, not verbs (/users, not /getUsers)
• Plural for collections (/users, not /user)
• Hierarchical for relationships (/users/{id}/orders)
• Consistent naming (kebab-case or camelCase, pick one)

3. Request Handling

• Validation with @Valid
• Clear error messages for validation failures
• Request DTOs (not entities)
• Reasonable size limits

4. Response Design

• Response DTOs (not entities)
• Consistent structure across endpoints
• Pagination for collections
• Proper status codes (not 200 for errors)

5. Error Handling

• Consistent error format
• Machine-readable error codes
• Human-readable messages
• No stack traces exposed
• Proper 4xx vs 5xx distinction

6. Compatibility

• No breaking changes in current version
• Deprecated endpoints documented
• Migration path for breaking changes

Token Optimization

For large APIs:

1. List all controllers: find . -name "*Controller.java"
2. Sample 2-3 controllers for pattern analysis
3. Check @ExceptionHandler configuration once
4. Grep for specific anti-patterns:

   # Find potential entity leaks
   grep -r "public.*Entity.*@GetMapping" --include="*.java"
   
   # Find 200 with error patterns
   grep -r "ResponseEntity.ok.*error" --include="*.java"
   
   # Find unversioned APIs
   grep -r "@RequestMapping.*api" --include="*.java" | grep -v "/v[0-9]"