Assist_Design/BFF_ARCHITECTURE_REVIEW.md

BFF Architecture Review - Logic Overlaps & Patterns

Date: October 8, 2025
Scope: Backend For Frontend (BFF) service architecture and validation patterns
Status: 🟡 Generally Good - Minor Improvements Recommended

---

Executive Summary

The BFF layer demonstrates good separation of concerns with proper use of validation pipes and domain schemas. However, there are a few areas where business logic could be better organized and some potential duplication in validation concerns.

Key Findings

✅ Strengths:

• Controllers use ZodValidationPipe consistently
• Domain schemas are imported and reused (no schema duplication in BFF)
• Clear service layer separation (orchestrator, validator, builder patterns)
• Infrastructure validation properly isolated in BFF services

⚠️ Concerns:

• Payment validation duplicated across services (2 locations)
• SIM business logic mixed with infrastructure concerns
• Some business rules could be moved to domain layer

---

🔍 Validation Pattern Analysis

✅ Correct Patterns Observed

1. Controller Input Validation

All controllers properly use ZodValidationPipe with domain schemas:

@Post()
@UsePipes(new ZodValidationPipe(createOrderRequestSchema))
async create(@Request() req: RequestWithUser, @Body() body: CreateOrderRequest) {

@UsePipes(new ZodValidationPipe(validateSignupRequestSchema))
async validateSignup(@Body() validateData: ValidateSignupRequest, @Req() req: Request) {

@UsePipes(new ZodValidationPipe(simTopupRequestSchema))
async topUpSim(

Assessment: ✅ Excellent pattern, consistent across all controllers

---

2. Domain Schema Imports

BFF properly imports schemas from domain package:

import {
  createOrderRequestSchema,
  sfOrderIdParamSchema,
  type CreateOrderRequest,
  type SfOrderIdParam,
} from "@customer-portal/domain/orders";

import {
  updateCustomerProfileRequestSchema,
  type UpdateCustomerProfileRequest,
} from "@customer-portal/domain/auth";

Assessment: ✅ No schema duplication in BFF layer

---

3. Infrastructure Validation in Services

BFF services handle infrastructure-dependent validation (DB, API calls):

async validateUserMapping(
  userId: string
): Promise<{ userId: string; sfAccountId?: string; whmcsClientId: number }> {
  const mapping = await this.mappings.findByUserId(userId);

  if (!mapping) {
    this.logger.warn({ userId }, "User mapping not found");
    throw new BadRequestException("User account mapping is required before ordering");
  }

  if (!mapping.whmcsClientId) {
    this.logger.warn({ userId, mapping }, "WHMCS client ID missing from mapping");
    throw new BadRequestException("WHMCS integration is required before ordering");
  }

  return {
    userId: mapping.userId,
    sfAccountId: mapping.sfAccountId || undefined,
    whmcsClientId: mapping.whmcsClientId,
  };
}

Assessment: ✅ Correct - infrastructure checks belong in BFF

---

🟡 Areas for Improvement

1. Payment Method Validation Duplication (Priority: MEDIUM)

Problem: Payment validation logic exists in two places:

Service	File	Lines
OrderValidator	apps/bff/src/modules/orders/services/order-validator.service.ts	109-122
OrderFulfillmentValidator	apps/bff/src/modules/orders/services/order-fulfillment-validator.service.ts	~130-150 (private method)

Current Implementation (OrderValidator):

async validatePaymentMethod(userId: string, whmcsClientId: number): Promise<void> {
  try {
    const pay = await this.whmcs.getPaymentMethods({ clientid: whmcsClientId });
    const paymentMethods = Array.isArray(pay.paymethods) ? pay.paymethods : [];
    if (paymentMethods.length === 0) {
      this.logger.warn({ userId }, "No WHMCS payment method on file");
      throw new BadRequestException("A payment method is required before ordering");
    }
  } catch (e: unknown) {
    const err = getErrorMessage(e);
    this.logger.error({ err }, "Payment method verification failed");
    throw new BadRequestException("Unable to verify payment method. Please try again later.");
  }
}

Impact:

• Logic duplicated in two validators
• Changes must be made in two places
• Potential for inconsistent error messages

Recommendation: Create a shared payment validation service:

// apps/bff/src/modules/orders/services/payment-validator.service.ts
@Injectable()
export class PaymentValidatorService {
  constructor(
    private readonly whmcs: WhmcsConnectionOrchestratorService,
    @Inject(Logger) private readonly logger: Logger
  ) {}

  async validatePaymentMethodExists(userId: string, whmcsClientId: number): Promise<void> {
    try {
      const pay = await this.whmcs.getPaymentMethods({ clientid: whmcsClientId });
      const paymentMethods = Array.isArray(pay.paymethods) ? pay.paymethods : [];
      
      if (paymentMethods.length === 0) {
        this.logger.warn({ userId, whmcsClientId }, "No payment method on file");
        throw new BadRequestException("A payment method is required before ordering");
      }
    } catch (e: unknown) {
      if (e instanceof BadRequestException) throw e;
      
      const err = getErrorMessage(e);
      this.logger.error({ err, userId }, "Payment method verification failed");
      throw new BadRequestException("Unable to verify payment method. Please try again later.");
    }
  }
}

// Then both validators inject and use it
constructor(private readonly paymentValidator: PaymentValidatorService) {}

async validatePaymentMethod(userId: string, whmcsClientId: number): Promise<void> {
  return this.paymentValidator.validatePaymentMethodExists(userId, whmcsClientId);
}

---

2. SIM Business Logic in BFF Service (Priority: MEDIUM)

Problem: SimValidationService contains business rules that should be in domain layer.

File: apps/bff/src/modules/subscriptions/sim-management/services/sim-validation.service.ts

Current Issues:

Issue 2a: Hardcoded Business Rules

// Check if this is a SIM service
const isSimService =
  subscription.productName.toLowerCase().includes("sim") ||
  subscription.groupName?.toLowerCase().includes("sim");

if (!isSimService) {
  throw new BadRequestException("This subscription is not a SIM service");
}

Issue: Business rule (what makes a subscription a "SIM service") is in BFF layer, not domain.

Should be:

// packages/domain/subscriptions/validation.ts
export function isSimSubscription(subscription: Subscription): boolean {
  return (
    subscription.productName.toLowerCase().includes("sim") ||
    subscription.groupName?.toLowerCase().includes("sim")
  );
}

---

Issue 2b: Magic Numbers/Test Data

// 4. Final fallback - for testing, use the known test SIM number
if (!account) {
  // Use the specific test SIM number that should exist in the test environment
  account = "02000331144508";

  this.logger.warn(
    `No SIM account identifier found for subscription ${subscriptionId}, using known test SIM number: ${account}`,
    {
      userId,
      subscriptionId,
      productName: subscription.productName,
      domain: subscription.domain,
      customFields: subscription.customFields ? Object.keys(subscription.customFields) : [],
      note: "Using known test SIM number 02000331144508 - should exist in Freebit test environment",
    }
  );
}

Issue: Hardcoded test data in service layer, should be in config.

Should be:

// config/env.validation.ts or constants file
export const TEST_SIM_ACCOUNT = process.env.TEST_SIM_ACCOUNT || "02000331144508";

// In service
if (!account && this.configService.get('NODE_ENV') === 'test') {
  account = TEST_SIM_ACCOUNT;
  this.logger.warn(`Using test SIM account: ${account}`);
}

---

Issue 2c: Custom Field Mapping Logic

private extractAccountFromCustomFields(
  customFields: Record<string, unknown>,
  subscriptionId: number
): string {
  // Common field names for SIM phone numbers in WHMCS
  const phoneFields = [
    "phone",
    "msisdn",
    "phonenumber",
    "phone_number",
    "mobile",
    "sim_phone",
    "Phone Number",
    "MSISDN",
    // ... 30+ more variations
  ];

  for (const fieldName of phoneFields) {
    const rawValue = customFields[fieldName];
    if (rawValue !== undefined && rawValue !== null && rawValue !== "") {
      const accountValue = this.formatCustomFieldValue(rawValue);
      this.logger.log(`Found SIM account in custom field '${fieldName}': ${accountValue}`);
      return accountValue;
    }
  }

  return "";
}

Issue: WHMCS field mapping is provider-specific knowledge, should be in provider layer.

Should be:

// packages/domain/subscriptions/providers/whmcs/sim-field-mapper.ts
export const WHMCS_SIM_ACCOUNT_FIELDS = [
  "phone",
  "msisdn",
  "phonenumber",
  "phone_number",
  "mobile",
  "sim_phone",
  // ... etc
] as const;

export function extractSimAccountFromWhmcsFields(
  customFields: Record<string, unknown>
): string | null {
  for (const fieldName of WHMCS_SIM_ACCOUNT_FIELDS) {
    const value = customFields[fieldName];
    if (value !== undefined && value !== null && value !== "") {
      return String(value);
    }
  }
  return null;
}

---

3. Order Business Rules in BFF Service (Priority: LOW)

Current Implementation:

validateBusinessRules(orderType: string, skus: string[]): void {
  const error = getOrderTypeValidationError(orderType, skus);
  if (error) {
    this.logger.warn({ orderType, skuCount: skus.length, error }, "Order validation failed");
    throw new BadRequestException(error);
  }

  // Log successful validation
  this.logger.log(
    { orderType, skuCount: skus.length },
    `Order business rules validated successfully for ${orderType} order`
  );
}

Assessment: This is actually OK because it delegates to domain function getOrderTypeValidationError().

The function correctly imports from domain:

import { getOrderTypeValidationError } from "@customer-portal/domain/orders/validation";

Recommendation: ✅ Keep as-is, this is the correct pattern.

---

🟢 Good Architectural Patterns

1. Service Layer Separation

The orders module demonstrates excellent separation:

orders/
├── orders.controller.ts           → HTTP layer (validation pipes only)
├── services/
│   ├── order-orchestrator.service.ts      → Workflow coordination
│   ├── order-validator.service.ts         → Validation aggregation
│   ├── order-builder.service.ts           → Data transformation
│   ├── order-fulfillment-orchestrator.service.ts  → Fulfillment workflow
│   └── order-fulfillment-validator.service.ts     → Fulfillment validation

Benefits:

• Single Responsibility Principle
• Easy to test each layer independently
• Clear separation of concerns

---

2. Orchestrator Pattern

async createOrder(userId: string, rawBody: unknown) {
  this.logger.log({ userId }, "Order creation workflow started");

  // 1) Complete validation (format + business rules)
  const { validatedBody, userMapping, pricebookId } =
    await this.orderValidator.validateCompleteOrder(userId, rawBody);

  this.logger.log(
    {
      userId,
      orderType: validatedBody.orderType,
      skuCount: validatedBody.skus.length,
    },
    "Order validation completed successfully"
  );

  // 2) Build order fields (includes address snapshot)
  const orderFields = await this.orderBuilder.buildOrderFields(
    validatedBody,
    userMapping,
    pricebookId,
    validatedBody.userId
  );

  // 3) Create Order in Salesforce via integration service
  const created = await this.salesforceOrderService.createOrder(orderFields);

  // 4) Create OrderItems from SKUs
  await this.orderItemBuilder.createOrderItemsFromSKUs(
    created.id,
    validatedBody.skus,
    pricebookId
  );

  this.logger.log(
    {
      orderId: created.id,
      skuCount: validatedBody.skus.length,
    },
    "Order creation workflow completed successfully"
  );

  return {
    sfOrderId: created.id,
    status: "Created",
    message: "Order created successfully in Salesforce",
  };
}

Benefits:

• Clear workflow steps
• Easy to understand business process
• Each step can be modified independently
• Excellent logging for debugging

---

3. Distributed Transaction Pattern

private async executeFulfillmentWithTransactions(
  sfOrderId: string,
  payload: Record<string, unknown>,
  idempotencyKey: string
): Promise<OrderFulfillmentContext> {
  const context: OrderFulfillmentContext = {
    sfOrderId,
    idempotencyKey,
    validation: null,
    steps: this.initializeSteps(
      typeof payload.orderType === "string" ? payload.orderType : "Unknown"
    ),
  };

  this.logger.log("Starting transactional fulfillment orchestration", {
    sfOrderId,
    idempotencyKey,
  });

  // Step 1: Validation (no rollback needed)
  try {
    context.validation = await this.orderFulfillmentValidator.validateFulfillmentRequest(
      sfOrderId,
      idempotencyKey
    );

    if (context.validation.isAlreadyProvisioned) {
      this.logger.log("Order already provisioned, skipping fulfillment", { sfOrderId });
      return context;
    }
  } catch (error) {
    this.logger.error("Fulfillment validation failed", {
      sfOrderId,
      error: getErrorMessage(error),
    });
    throw error;
  }

  // Step 2: Get order details (no rollback needed)
  try {
    const orderDetails = await this.orderOrchestrator.getOrder(sfOrderId);
    if (!orderDetails) {
      throw new Error("Order details could not be retrieved.");
    }
    context.orderDetails = orderDetails;
  } catch (error) {
    this.logger.error("Failed to get order details", {
      sfOrderId,
      error: getErrorMessage(error),
    });
    throw error;
  }

  // Step 3: Execute the main fulfillment workflow as a distributed transaction
  let mappingResult: OrderItemMappingResult | undefined;
  let whmcsCreateResult: { orderId: number } | undefined;
  let whmcsAcceptResult: WhmcsOrderResult | undefined;

  const fulfillmentResult =
    await this.distributedTransactionService.executeDistributedTransaction(
      // ... transaction steps
    );

Benefits:

• Idempotency built-in
• Clear rollback strategy
• Transactional integrity across services
• Proper error handling and logging

---

📋 Summary of Recommendations

Immediate Actions

1. Extract Payment Validation to Shared Service
   • Create PaymentValidatorService
   • Both OrderValidator and OrderFulfillmentValidator use it
   • Eliminates duplication, ensures consistency

Medium Priority

2. Move SIM Business Logic to Domain

   • Extract isSimSubscription() to packages/domain/subscriptions/validation.ts
   • Move WHMCS field mappings to packages/domain/subscriptions/providers/whmcs/
   • Move test constants to config

3. Extract SIM Account Extraction Logic

   • Create domain function for SIM account extraction
   • Keep infrastructure concerns (DB, logging) in BFF
   • Move business rules (what fields to check, validation logic) to domain

Low Priority (Optional)

4. Consider Creating Base Validator Service

   // apps/bff/src/core/validation/base-validator.service.ts
   export abstract class BaseValidatorService {
     constructor(
       protected readonly logger: Logger,
       protected readonly mappingsService: MappingsService
     ) {}
   
     protected async validateUserMapping(userId: string) {
       // Common validation logic used by multiple validators
     }
   }
   

---

🎯 BFF Validation Checklist

Use this checklist when adding new validation:

• Format validation: Use ZodValidationPipe in controller
• Schema source: Import from @customer-portal/domain
• Business rules: Check if rule belongs in domain layer
• Infrastructure checks: Keep in BFF service (DB, API calls)
• Shared logic: Consider extracting to shared service if used > 1 place
• Error messages: Use consistent, user-friendly messages
• Logging: Log validation failures with context
• Type safety: Use inferred types from domain schemas

---

📊 Validation Logic Distribution

Current distribution of validation logic:

Layer	Responsibility	Examples	Status
Controller	Input format validation	@UsePipes(ZodValidationPipe)	✅ Good
Domain Schemas	Data structure & format rules	createOrderRequestSchema	✅ Good
Domain Validation	Business rules (pure logic)	hasSimServicePlan()	✅ Good
BFF Services	Infrastructure validation	validateUserMapping(), validatePaymentMethod()	🟡 Minor duplication
BFF Services	Business logic	isSimService check in SimValidationService	⚠️ Should move to domain

---

🔍 Validation Flow Example (Orders)

1. HTTP Request
   ↓
2. OrdersController
   ├─ @UsePipes(ZodValidationPipe(createOrderRequestSchema))  ← Format validation
   └─ Passes validated body to OrderOrchestrator
      ↓
3. OrderOrchestrator.createOrder()
   ├─ Calls OrderValidator.validateCompleteOrder()
   │  ├─ Format validation (schema)            ← Zod schema
   │  ├─ Business validation (schema)          ← Domain validation
   │  ├─ User mapping check (infrastructure)   ← BFF validation
   │  ├─ Payment method check (infrastructure) ← BFF validation
   │  ├─ SKU validation (infrastructure)       ← BFF validation
   │  └─ Business rules (delegates to domain)  ← Domain validation
   └─ Continue with order creation

Assessment: ✅ Clean separation, proper delegation

---

📝 Notes

Comparison with Domain Layer

• Domain Layer: Pure business logic, no dependencies on infrastructure
• BFF Layer: Infrastructure-dependent validation (DB, external APIs)
• Clear Boundary: BFF delegates pure business rules to domain functions

Previous Validation Cleanup

The codebase has already:

• Eliminated password validation duplication
• Moved SKU validation to domain layer
• Consolidated order business rules

This review builds on that work to identify remaining improvements.

---

🔗 Related Documents

• VALIDATION_AUDIT_REPORT.md - Domain validation overlaps
• docs/validation/VALIDATION_PATTERNS.md - Validation patterns guide
• docs/validation/VALIDATION_CLEANUP_SUMMARY.md - Previous cleanup work