Extracting a module without extracting a service

Everybody agreed billing should be separate. What “separate” meant was never specified, and the two candidate meanings — a module with a boundary, or a deployable service — have almost nothing in common except the word. This is the first one, done properly, on the explicit understanding that the second one was not happening.

The symptom

$ grep -rl 'App\Billing' src/ --exclude-dir=Billing | wc -l
88
$ grep -rlE 'App\(Orders|Catalogue|Shipping)' src/Billing | wc -l
41

# 88 classes reach into billing.
# billing reaches back into 41.

$ grep -rc 'use App\Billing\' src/Http/Controllers/*.php | sort -t: -k2 -rn | head -3
OrderController.php:7
InvoiceController.php:6
RefundController.php:5

A controller importing seven classes from another module is not using a module, it is using an implementation. Every one of those imports is a change that can break billing and a change billing can break, and there are 129 of them in both directions.

Why it happens

A boundary that exists in conversation costs nothing to cross. Nobody sets out to reach into another module’s internals; they need a value, the class that has it is right there, and the import is one keystroke in an IDE that offers it helpfully.

The fix

Drawing the line before moving anything

in billing:
  invoices, credit notes, payment allocation, tax
  calculation, the ledger

not in billing:
  the order (billing reads it, does not own it)
  the customer (same)
  the product catalogue

ambiguous, and decided one at a time:
  price          → catalogue. billing consumes it.
  discount       → billing. it changes what is owed.
  currency       → shared kernel. genuinely both.
  refund         → billing owns the money, orders owns
                   the decision. split into two things.
  payment method → billing.
  invoice email  → billing composes it, notifications
                   sends it.

The six ambiguous ones took an afternoon and were the whole exercise. Refund splitting into a decision and a movement of money is the one that mattered most: it had been a single class doing both, which is why the boundary could not be drawn around it.

Inverting the dependencies that point the wrong way

// billing reaching into orders, 41 times
final class InvoiceBuilder
{
    public function build(Order $order): Invoice   // ← AppOrdersOrder
    { /* ... */ }
}

// after: billing declares what it needs
namespace AppBillingContracts;

interface Billable
{
    public function billableReference(): string;
    /** @return list<BillableLine> */
    public function billableLines(): array;
    public function billingCurrency(): Currency;
}

// and orders implements it — the dependency now points
// from orders INTO billing, which is one direction.

Four of the 41 needed genuine inversion; the rest were reading a value that belonged on the interface anyway. Declaring what billing needs rather than accepting what orders happens to have is the change, and the interface is three methods rather than the forty an Order has.

A public API of six methods

namespace AppBilling;

final readonly class BillingApi
{
    public function raiseInvoice(Billable $subject): InvoiceReference {}
    public function creditNote(InvoiceReference $ref, Money $amount): CreditNoteReference {}
    public function allocatePayment(InvoiceReference $ref, Money $amount): void {}
    public function outstandingFor(string $reference): Money {}
    public function invoiceSummary(InvoiceReference $ref): InvoiceSummary {}
    public function taxFor(Money $net, TaxJurisdiction $where): Money {}
}

// everything else in AppBilling became internal.
// 88 call sites collapsed to 6 methods.

The collapse from 88 imports to 6 methods is the measurement that says the boundary was real. Where a call site could not be expressed through the six, it was a sign the line was drawn wrong — that happened twice, and both were the refund split arriving late.

Enforcing it, as a ratchet

layers:
  - name: Billing
    collectors: [{ type: directory, value: src/Billing/.* }]
  - name: BillingApi
    collectors: [{ type: className, value: App\Billing\BillingApi }]
  - name: Rest
    collectors: [{ type: directory, value: src/(?!Billing).* }]

ruleset:
  Rest: [BillingApi]        # not Billing
  Billing: [Rest]           # via contracts only, checked
                            # separately

$ vendor/bin/deptrac --formatter=table
  Violations   41

$ vendor/bin/deptrac --formatter=baseline > deptrac-baseline.yaml
$ vendor/bin/deptrac
  Violations   0    (41 skipped)

# and the number goes down. it does not go up: a new
# violation is not in the baseline and fails the build.

The baseline is what makes this adoptable on a working codebase, and the discipline is that the file only ever shrinks. Ours went 41 → 18 → 4 over three months, in the ordinary course of touching those files, with no dedicated cleanup work at all.

The database, which is the hard part

tables, by owner:

  billing owns    invoices, invoice_lines, credit_notes,
                  payments, allocations, tax_rates
  orders owns     orders, order_lines
  shared          currencies (read by both, written by
                  neither at runtime)

and the joins that crossed the line:
  invoices.order_id → orders.id      KEPT
  orders.invoice_id → invoices.id    REMOVED, redundant

  a report joining invoices to order_lines:
    rewritten to go through the API, and it got slower.
    accepted: it runs nightly.

Keeping the foreign key from invoices to orders is a deliberate violation of the strictest reading, and the alternative is losing referential integrity for a purity that buys nothing while both tables are in one database. The report getting slower is the honest cost, and it was quantified before being accepted.

Events for the two places that needed to react

// orders does not tell billing to do anything
$this->events->dispatch(new OrderCompleted($order->id()));

// billing decides whether it cares
final class RaiseInvoiceOnOrderCompleted
{
    public function __invoke(OrderCompleted $event): void
    {
        $this->api->raiseInvoice($this->billables->find($event->orderId));
    }
}

// synchronous, in-process, same transaction. this is not
// a message queue and does not pretend to be.

Keeping them synchronous and in the same transaction is the decision that stops this being a distributed system by accident. An invoice that is raised in the same commit as the order completing is a guarantee finance can rely on, and making it asynchronous would trade that for a resilience nobody asked for on a monolith that either serves the request or does not.

Two events, both of them cases where the calling module genuinely should not know what happens next. Everywhere else a direct call through the API is clearer, and the temptation to make everything an event is the failure mode that turns a modular monolith into an unreadable one.

What was deliberately not done

not done, and written down:

  a separate deployable    the team is four people.
                           two deployables is two
                           pipelines, two on-call
                           surfaces, one team.
  a separate database      the reports join across.
                           this is the change that
                           would cost months.
  a queue between them     adds eventual consistency to
                           an invoice, which finance
                           would notice.
  a separate repository    the boundary is enforced by
                           a test. a second repo enforces
                           it by making changes hard,
                           which is not the same thing.

revisit if: billing gets its own team, or the release
cadences genuinely diverge.

Verifying it worked

$ vendor/bin/deptrac
  Violations 0, skipped 41 (baseline)

$ grep -rl 'App\Billing' src/ --exclude-dir=Billing | wc -l
6                        # was 88 — all via BillingApi

$ vendor/bin/phpunit
Tests: 1,412 passed

# three months later
$ wc -l < deptrac-baseline.yaml
4                        # was 41
$ git log --oneline --since=3.months -- src/Billing | wc -l
38                       # and no new violations

The baseline shrinking without a cleanup project is the outcome that says the boundary is holding — thirty-eight commits touched billing and none of them added a violation, because the build would have rejected it. That is the difference between a rule and a diagram.

What this costs

Indirection that is only worth it if the boundary holds. Six API methods where there used to be direct access means a change that touches billing and its callers is now two changes, and the interface is a thing that has to be designed rather than discovered.

The nightly report is slower, measurably, because it goes through the API rather than joining. That was accepted with a number attached — 4 seconds to 31 — and it is the kind of cost that accumulates: a second report with the same shape would make this a worse trade, and the decision record says to revisit if that happens.

The largest risk is that the boundary is now good enough that nobody feels pressure to extract the service, and the boundary was justified partly as preparation for extraction. That is either a success or a rationalisation depending on whether the extraction ever becomes necessary, and it probably will not.