Assist_Design/docs/decisions/006-thin-controllers.md

4.5 KiB

ADR-006: Thin Controllers

Date: 2025-01-15 Status: Accepted

Context

NestJS controllers can contain varying amounts of logic:

  • Fat controllers: Business logic, validation, transformation all in controller
  • Thin controllers: HTTP handling only, delegate everything else

Decision

Controllers handle HTTP concerns only. All business logic lives in services.

// ✅ GOOD: Thin controller
@Get()
async getInvoices(
  @Request() req: RequestWithUser,
  @Query() query: InvoiceListQueryDto
): Promise<InvoiceList> {
  return this.invoiceService.getInvoices(req.user.id, query);
}

// ❌ BAD: Fat controller with business logic
@Get()
async getInvoices(@Request() req, @Query() query) {
  const clientId = await this.mappings.getClientId(req.user.id);
  const raw = await this.whmcs.getInvoices({ clientId });
  const filtered = raw.filter(inv => inv.status !== 'draft');
  return filtered.map(inv => transformInvoice(inv));
}

Rationale

Why Thin Controllers?

  1. Single responsibility: Controllers handle HTTP (request parsing, response formatting, status codes). Services handle business logic.

  2. Testability: Business logic in services can be unit tested without HTTP mocking.

  3. Reusability: Service methods can be called from:

    • Multiple controllers
    • Background jobs
    • Event handlers
    • CLI commands
  4. Consistency: Developers know controllers are just HTTP glue.

Controller Responsibilities

DO in controllers:

  • Route definition (@Get(), @Post(), etc.)
  • Request parsing (@Body(), @Query(), @Param())
  • Authentication extraction (@Request() req)
  • HTTP status codes (@HttpCode())
  • OpenAPI documentation (@ZodResponse())
  • Call ONE service method

DON'T in controllers:

  • Business logic
  • Data transformation
  • External API calls
  • Database queries
  • Multiple service orchestration
  • Error handling beyond HTTP concerns

Alternatives Considered

Approach Pros Cons
Fat controllers Everything in one place Untestable, not reusable
Thin controllers Testable, reusable, SRP More files

Consequences

Positive

  • Clear separation of concerns
  • Testable business logic
  • Reusable service methods
  • Consistent patterns

Negative

  • More files (controller + service)
  • Developers might create pass-through services

Implementation

Controller Structure

// apps/bff/src/modules/billing/billing.controller.ts
@Controller("invoices")
export class BillingController {
  constructor(private readonly invoiceService: InvoiceRetrievalService) {}

  @Get()
  @ZodResponse({ description: "List invoices", type: InvoiceListDto })
  async getInvoices(
    @Request() req: RequestWithUser,
    @Query() query: InvoiceListQueryDto
  ): Promise<InvoiceList> {
    // One service call - that's it
    return this.invoiceService.getInvoices(req.user.id, query);
  }

  @Get(":id")
  @ZodResponse({ description: "Get invoice", type: InvoiceDto })
  async getInvoice(@Request() req: RequestWithUser, @Param("id") id: string): Promise<Invoice> {
    return this.invoiceService.getInvoice(req.user.id, parseInt(id));
  }
}

Service Structure

// apps/bff/src/modules/billing/services/invoice-retrieval.service.ts
@Injectable()
export class InvoiceRetrievalService {
  constructor(
    private readonly whmcsInvoice: WhmcsInvoiceService,
    private readonly mappings: MappingsService
  ) {}

  async getInvoices(userId: string, query: InvoiceQuery): Promise<InvoiceList> {
    // Business logic here
    const clientId = await this.mappings.getWhmcsClientId(userId);

    const invoices = await this.whmcsInvoice.getInvoices(clientId, {
      status: query.status,
      limit: query.limit,
    });

    return {
      items: invoices,
      total: invoices.length,
    };
  }
}

What NOT to Do

// ❌ DON'T: Business logic in controller
@Get()
async getInvoices(@Request() req, @Query() query) {
  const clientId = await this.mappings.getClientId(req.user.id);
  const raw = await this.whmcs.api.getInvoices({ clientId });

  // Filtering, transformation, validation - all wrong here
  return raw.invoices
    .filter(inv => query.status ? inv.status === query.status : true)
    .map(inv => transformWhmcsInvoice(inv));
}