Skip to content

Service Layer Architecture

Ed Mozley edited this page Jul 5, 2026 · 7 revisions

Service Layer β€” one implementation, two interfaces

In one sentence: the UI and the REST API should call the same business logic, so a rule is written once and can never drift between them.

See also: Architecture Β· REST API β€” how it works Β· πŸ“Š Service-Layer progress tracker

The problem this solves

FreeITSM's REST API v1 was built by mirroring each module's internal (UI) endpoints. The header comment on api/v1/resources/tickets.php says it out loud:

"Mirrors the behaviour of the internal ticket endpoints so a ticket touched via the API is indistinguishable from one touched in the UI…"

"Mirrors" is the tell. It means every write operation exists twice:

Operation UI copy (session-authed) API copy (key-authed)
Create a service api/service-status/save_service.php api/v1/resources/service_status.php
Save an incident api/service-status/save_incident.php api/v1/resources/service_status.php
…and so on for every module

Two copies of the same rules is a drift surface. Change one and forget the other, and the UI and API quietly diverge. Several past bugs were exactly this (the Forms submission break, the foreign-key series). Worse, the two copies were never quite identical to begin with β€” see Watch out for #1 below.

Why only the writes are refactored

This refactor deliberately unifies write logic (create / update / delete) and leaves the read paths (lists, detail views, serializers) untouched. That's not laziness β€” it's where the risk and the payoff actually are:

  • The drift danger lives in writes. A write isn't just a SQL statement; it carries business rules and side effects β€” validation, audit-log rows, workflow dispatch, template emails, SLA recalculation, CSAT triggers, UTC stamping. That's the logic that was copied twice and that silently drifts when one copy changes. A read is essentially "SELECT + shape the output" β€” there's no side effect to keep in sync, so there's very little that can drift.
  • Reads aren't duplicated in the dangerous way. The UI read and the API read deliberately produce different shapes and always will: the API returns a frozen public JSON contract ({data:{…}}, typed fields), while the UI read returns whatever its own front-end needs. They aren't two copies of one thing β€” they're two different outputs over the same table. Forcing them through one function would mean either rewriting the UI to consume the API's JSON (huge churn, zero user benefit) or having the service return raw rows for each side to shape itself β€” which is effectively what already happens. Low payoff, real risk on battle-tested code.
  • The next feature plugs into the write path. Once a module's writes go through one service, emitting a lifecycle event (licence.created, ticket.updated) for outbound webhooks is one line from that single write path β€” firing identically for UI, API and automation. Reads emit nothing, so there's nothing to gain there.

In short: the careful, parity-harnessed work goes where a bug corrupts data or silently breaks an integration. A read drift, worst case, shows a slightly different field somewhere visible β€” so the read paths stay exactly as they are. (This is also why the API's serializers are kept untouched during a migration: it's what makes the API output provably byte-identical β€” see Watch out for #4.)

The shape of the fix (ports & adapters)

Session (UI)  ─┐                                              β”Œβ”€ UI JSON  {success:true,…}
               β”œβ”€β–Ί ActorContext ─► <Module>Service::method() ──
API key       β”€β”˜        (who + scope)      (the rules)         └─ API envelope + 201/404/422…
  • The service holds the business rules. It is written once.
  • The UI and the API become thin adapters. Each one only does two things: turn its own kind of request into a call, and turn the result back into its own kind of response.

The key insight: how one function serves session and API-key auth

This is the part that trips people up. "The UI needs a session, the API needs a key β€” how can they run the same code?"

The service never sees a session or an API key. Authentication is the adapter's job. Each adapter distils its caller down to the only two facts the rules actually need β€” who is acting and what companies they can see β€” and passes them in as an ActorContext:

final class ActorContext {
    public int    $actorId;       // the analyst this action is attributed to
    public ?array $companyScope;  // null = all companies, or [tenantId, …]
    public string $source;        // 'ui' | 'api'
    public string $locale;
    static function fromSession(PDO $c): self { /* reads $_SESSION['analyst_id'] + tenancy */ }
    static function fromApiKey(array $key): self { /* reads $key['analyst_id'] + company_scope */ }
}

The two auth systems already produce those same two facts β€” apiAuthenticate() returns analyst_id + company_scope; the session equivalent reads $_SESSION['analyst_id'] + the tenancy helpers. ActorContext is just the neutral shape they both collapse into. The service is auth-agnostic; the adapters are transport-specific.

The three rules a service obeys

  1. Signature β€” method(PDO $conn, ActorContext $ctx, array $input): <data>. No $_SESSION, $_GET, $apiKey, or any superglobal.
  2. No transport β€” never echo, header(), http_response_code(), exit, apiRespond() or apiError(). It returns plain data (usually the affected id) or throws a typed ServiceError. Serialising and HTTP framing belong to the adapter.
  3. Typed errors — ServiceError($kind, $code, $message). The adapter maps $kind to a status (validation→422, not_found→404, forbidden→403, conflict→409) and passes the $code + message through verbatim, so the API's error bodies stay identical.

Worked example β€” save_service

The service (includes/services/service_status.php) β€” the rules, once:

class ServiceStatusService {
    static function saveService(PDO $conn, ActorContext $ctx, array $in): int {
        $name = trim((string)($in['name'] ?? ''));
        if ($name === '') throw new ServiceError('validation', 'missing_field', "'name' is required.");
        // …insert/update status_services (UTC timestamps, empty desc -> NULL)…
        return $id;               // the affected id β€” NOT a serialised response
    }
}

The UI adapter β€” auth + its own response shape:

if (!isset($_SESSION['analyst_id'])) { echo json_encode(['success'=>false,'error'=>'Not authenticated']); exit; }
try {
    $id = ServiceStatusService::saveService($conn, ActorContext::fromSession($conn), $data);
    echo json_encode(['success' => true]);
} catch (Exception $e) { echo json_encode(['success'=>false, 'error'=>$e->getMessage()]); }

The API adapter β€” same call, API framing:

function apiStatusServicesCreate(PDO $conn, array $apiKey, $params, array $body): void {
    try {
        $id = ServiceStatusService::saveService($conn, ActorContext::fromApiKey($apiKey), $body);
        apiRespond(apiSerializeService($conn, apiLoadService($conn, $id)), 201);
    } catch (ServiceError $e) { apiError(serviceErrorHttpStatus($e->kind), $e->errorCode, $e->getMessage()); }
}

One implementation of the rule. Two thin edges. Notice the service returns the id and each adapter decides how much to send β€” the API re-loads and serialises with its existing, untouched serialiser; the UI returns {success:true}. (Why that matters: #4.)


Watch out for… (read this before migrating a module)

These are the traps we've hit across the modules migrated so far (service-status, morning-checks, software, calendar, forms, contracts). Most recur on every module β€” read them before you start.

Watch out for #1 β€” the two copies already disagreed

"Unify the UI and API into one path" sounds like pure de-duplication, but the two copies were never byte-identical β€” so unifying forces a choice about which behaviour wins. For service-status the pre-existing differences were:

# Spot UI did API did
1 create timestamps DB default (server-local) UTC
2 empty description stored "" stored null
3 bad service/impact id on an incident silently skipped 422
4 set incident status by name only by name or id

Decision (project-wide): converge to the API's behaviour. The API is the contract we promised external consumers we wouldn't break, so it stays identical; the UI's internal write behaviour shifts to match. Every convergence here made the UI more correct. Always enumerate these before you touch code, and get sign-off β€” a silent behaviour change to a working UI is exactly what erodes trust.

Watch out for #2 β€” timestamps and the "20 offices" test

Convergence #1 (server-local β†’ UTC) isn't cosmetic; it's the storage half of getting time right across time zones. The rule the whole app is moving toward:

Store UTC. Compute SLAs in UTC. Convert to local only at display (in the browser).

Why it matters: SLA maths subtracts two instants. If both are UTC, "4 hours elapsed" is always exactly 4 hours β€” daylight saving can't add or drop an hour, and it doesn't matter which office raised the ticket. The moment a local time sneaks into that subtraction, an SLA timer jumps twice a year. The service layer is the ideal enforcement point because every write goes through one place β€” "stamp UTC" is written once, not in 40 endpoints.

Deferred, and important: this fixes storage. The display half is still open β€” the module JS does new Date("2026-07-05 12:00:00"), which JavaScript parses as local, so UTC data can render an hour off. Fixing that means a parse-as-UTC helper across every timestamp-rendering module. Tracked on the progress page.

But "always stamp UTC" has an exception β€” respect a module's deliberate time model. The calendar stores naive server-local datetimes on purpose: it writes exactly what the browser sends and its ICS feed reads them back in the server's zone. Blindly converting it to UTC would have shifted every event. So its service keeps the naive contract β€” it validates YYYY-MM-DD HH:MM:SS, still rejects Z/offset values, and does not convert. The lesson: before you "converge to UTC", check whether the module's current behaviour is a bug (most are) or a deliberate model (a few are). Converge the bugs; preserve the models, and say which is which.

Watch out for #3 β€” error ordering is part of the contract

The old API update did existence-check first (404), then the empty-body check (422). If you move the empty-body guard into the adapter (before the service runs), you reverse that order and a request with both problems returns the wrong error. Fix: fold the empty-body rule into the service, after the existence load, so the order is preserved and the rule still lives in one place:

$current = self::loadServiceRow($conn, $id);            // 404 first, as the API did
if (!array_diff_key($in, ['id' => true])) {
    throw new ServiceError('validation', 'missing_field', 'No fields to update.');
}

Watch out for #4 β€” proving byte-identical

The promise "the API doesn't change" has to be proven, not hoped for. Two things make it hold:

  • Don't touch the serialisers. The service returns an id; the API adapter re-loads and serialises with the same functions as before. Identical input to an unchanged serialiser β†’ identical output, trivially.
  • A before/after parity harness (see #8).

Watch out for #5 β€” ServiceError::$code collides with Exception::$code

Exception already has a $code property (an int). Declaring public string $code in a subclass is a fatal error (Type of ServiceError::$code must not be defined). Name it $errorCode instead. The constructor can still accept $code β€” just assign it to $this->errorCode.

Watch out for #6 β€” deletes stop being idempotent (on purpose)

The old UI delete was idempotent: deleting a non-existent id returned success. The API delete 404s. Converging to the API means the UI now 404s too β€” so double-clicking delete shows an error the second time. This is a real, if minor, UX change; flag it, don't let it surprise you.

Watch out for #7 β€” tiny response-shape fidelity

Byte-identical means byte-identical. The old API delete responds with ['id' => $params[0], …] where $params[0] is the string captured from the URL. Keep passing $params[0] (not (int)) into that response, or the JSON type flips from "5" to 5 and the harness (rightly) fails.

Watch out for #8 β€” the parity harness and its traps

The harness seeds a temp API key and a real PHP session, drives every endpoint over HTTP, normalises volatile ids/timestamps, and diffs golden-before vs golden-after. A reusable template lives at docs/design/parity-harness-example.php β€” copy it to a scratchpad and adapt. Traps we hit, in rough order of how often they bit us:

  • Seeding both auths. API: insert a row into api_keys (act as an existing analyst) with the right permissions JSON. UI: write a session with session_id($fixed); session_start(); $_SESSION['analyst_id']=…; session_write_close(); and send Cookie: PHPSESSID=$fixed.
  • Re-capturing "before" after you've refactored. You need the golden-before on the original code. git stash push -- <the changed files> (note: not -u, or it drags in the new untracked service files), run the harness, then git stash pop. Always confirm the stash worked (grep -c <Module>Service <resource> should print 0) before capturing.
  • Sessions overwrite $_SESSION. A CLI script that seeds $_SESSION then calls an endpoint that does its own session_start() won't share state β€” drive the endpoint over HTTP with the cookie, don't try to fake it in-process.

The id-collision trap β€” this bit us on five of six modules, so it gets its own guidance. The harness normalises volatile auto-increment ids (which differ between the before and after runs) to placeholders like <CON>. The naΓ―ve way β€” a regex that swaps the id digits anywhere in the response text β€” corrupts unrelated data that happens to share those digits, producing a fake diff. Every variant we saw:

  • The created id 10 rewrote "10-15 minutes" in an unrelated demo comment.
  • A created status id equalled a sort_order value in the same payload.
  • A seeded lookup id (say 9) equalled a created contract id β€” because every table has its own auto-increment, ids from different tables collide freely.
  • A supplier id 11 matched the 11 inside a date string "2024-11-01".
  • A term-tab id 3 matched the literal 3 in the message "must be a 3-letter code".

Three defences, apply all of them:

  1. Force seeded ids to be distinctive-large. When you insert the fixtures (lookups, seed rows), give them explicit ids like 900001+ (INSERT (id, …) VALUES (900001, …)). They can't collide with real auto-increment ids, and β€” because they're identical across both runs β€” you don't need to normalise them at all.
  2. Use distinctive-large numbers for test data too (a sort_order of 90001, a notice_period_days of 90060), so no data value can equal a small created id.
  3. Normalise structurally, not textually. Instead of a blunt string-regex, json_decode the response and walk it: replace an id only where it appears as an integer value, and mask date/timestamp strings to <DATE>/<TS>. Digits inside strings are then untouchable. Keep a narrow value-regex pass only for ids embedded in error-message text (e.g. a 409 that names the conflicting row). This is the go-to for any module whose responses carry dates or numbers.
  • Capture child ids that are created implicitly. Forms clone their fields on every version fork, so the new version's field ids are new. Query them (SELECT id FROM form_fields WHERE form_id = ? ORDER BY sort_order) right after the create and after the fork, and add them to the normalise map β€” otherwise every serialized field id reads as a diff.
  • Filter list endpoints to your test rows before comparing β€” a list also returns the install's real rows, whose count/content you don't control.
  • DELETE … WHERE … (SELECT … FROM same_table) fails with MySQL error 1093. Cleaning a self-referencing table (forms' parent_form_id) can't sub-select the table it deletes from. Rewrite as a self-join: DELETE f FROM forms f LEFT JOIN forms c ON c.parent_form_id = f.id WHERE … AND c.id IS NULL, looped until no rows remain.
  • Opcache serves stale code across the git stash swap. WAMP runs with opcache.validate_timestamps=On, revalidate_freq=2, so for ~2s after a stash push/pop the web server can still execute the previous version of a swapped file β€” which silently corrupts the golden (we saw two UI cases "fail to converge" that were actually the pre-refactor code still cached). Reset opcache right after each stash operation before running the harness: drop a one-liner <?php opcache_reset(); in the web root and curl it after git stash push and again after git stash pop. A byte-identical API result alone doesn't prove the new code ran β€” it can also mean the old code ran for both captures; the opcache reset is what makes the run trustworthy.

Watch out for #9 β€” a module doesn't have to be migrated whole

The goal is to kill drift, and drift only exists where the same write logic lives in two places. So migrate the cleanly-duplicated overlaps and be willing to leave the rest alone β€” say so out loud when you do:

  • UI-only settings with no API twin. Software's dashboard widgets and legacy api-keys, and contracts' five lookup tables (statuses, types, schedules, term tabs), exist only in the UI β€” there's nothing to de-duplicate, and dragging them into the service is churn on battle-tested code for zero benefit. Leave them.
  • Sub-entities whose UI and API models genuinely differ in shape. Contracts' supplier contacts are modelled flat by the UI (a supplier_id in the body, and a contact can be moved between suppliers) but nested under a supplier by the API. Forcing those into one method risks subtle behaviour changes for little gain, so they stayed on their own code.
  • Do fold in UI-only ops when they're small and cohesive to one entity β€” e.g. calendar categories, or morning-checks' statuses. The test is "is this the same entity's settings, or a separate sub-system?"

The consistent rule isn't "migrate everything", it's "migrate the duplication, and flag every deliberate leave-out" so the scope is a decision, not an accident.

When the shapes differ but the write is genuinely the same, you have three moves β€” pick the cheapest that stays honest:

  1. Leave it out (above) β€” when the operations are different enough that unifying invites bugs (tasks' reorder.php: client-computed positions vs the API's server-side re-pack).
  2. One method, an optional parameter β€” when only a scope differs. deleteSubmission($id, $formId = null) and deleteRelationship($relId, $objectId = null): the API passes the scoping id (and gets a 404 on a miss), the UI passes null (unscoped). One body of logic, two callers.
  3. Normalise both input shapes at the service boundary β€” when the two sides address the same data differently. CMDB properties arrive as the API's {key: value} map or the UI's [{property_id, value}] id-list; the service accepts either and normalises to one internal form before validating. Neither transport had to change its payload, and there's still exactly one set of rules. Prefer this over leaving a core write unmigrated.

Watch out for #10 β€” side effects belong in the service, not the adapter

A write is more than a row: it can fire a workflow, send a templated email, recompute an SLA, trigger CSAT. Those must move into the service with the write, or a UI action and an API action stop behaving the same β€” which is the whole problem we're solving. Forms' form.submitted workflow dispatch is the model: it now runs from the single submitForm() path, so a submission made through the browser and one made through the API kick off identical automation. Two rules carried over from the original endpoints: fire the side effect after the DB commit, and wrap it so its failure can't roll back or break the write (try { … } catch (Throwable) { error_log(…) }). This is also the hook the outbound-webhooks work plugs into later β€” one events.emit(...) from the service covers UI, API and automation at once.

Watch out for #11 β€” not every API error is a 422

ServiceError's default maps to 422, which is right for almost everything. But when you lift a helper that returned a different status, you have to preserve it or you silently change the API's contract. CMDB's date-property validator used the shared apiParseDate(), which returns 400 invalid_parameter (it's a query-param parser). Turning that into a plain ServiceError('validation', …) would have downgraded it to 422 β€” an observable, harness-failing change. The fix was to teach the kindβ†’status map a new case and throw with it:

// serviceErrorHttpStatus(): case 'bad_request': return 400;
throw new ServiceError('bad_request', 'invalid_parameter', "… not a valid date/time …");

Before you lift a validator, check the exact status and error code it emitted (grep the original apiError( call) and reproduce both. The status is part of the contract, not an implementation detail.

Watch out for #12 β€” catch (Exception) will swallow your ServiceError

A subtle one. The old code often wrapped a DB probe in try { … apiError(422, …) } catch (Exception $e) { apiError(422, "not available") } β€” where apiError exits, so the "not available" branch only ever caught a real driver error. Lift that verbatim into a service and the inner apiError becomes a throw ServiceError β€” which is an Exception, so your own validation error gets caught by that catch and re-thrown as the wrong message. Catch the specific driver exception instead:

try {
    $stmt->execute([$id]);
    if (!$stmt->fetchColumn()) throw new ServiceError('validation','invalid_field', "Unknown … id");
} catch (PDOException $e) {                 // NOT catch (Exception) β€” that'd eat the ServiceError
    throw new ServiceError('validation','invalid_field', "… not available on this install.");
}

Watch out for #13 β€” company scope goes through ActorContext, not the transport helper

Tenant-scoped checks (can this caller see ticket X?) exist twice in the codebase: apiKeyCanAccessTicket() (API-key side) and analystCanAccessTicket() (session side). A service must call neither β€” it has no key and no session. Instead read the one fact both adapters already distilled into the ActorContext: companyScope (null = all companies, else the allowed tenant-id list). Write a tiny scope-based check against that and it works for both callers:

private static function ticketAccessible(PDO $conn, ActorContext $ctx, int $ticketId): bool {
    // … load the row's tenant_id …
    if (!isMultiTenant($conn) || $ctx->companyScope === null) return true;
    $tid = $tenantId ?? getDefaultTenantId($conn);
    return in_array($tid, $ctx->companyScope, true);
}

Tasks (#728) proved this: the same ticketAccessible() now gives the UI the tenant isolation on task→ticket links it never had, while the API stays byte-identical. This is the pattern for the tenant-scoped modules still to come (problems, changes, tickets).


How to build a NEW feature that has both a UI and an API

Write the logic once, in the service. Add two thin adapters. Never copy logic between api/<module>/ and api/v1/resources/.

  1. Put the rule in includes/services/<module>.php as a static method method(PDO $conn, ActorContext $ctx, array $input). Validate, write, and return the id (or throw ServiceError). No echo/header/superglobals. For a create-or-update method, return ['id' => …, 'created' => true|false] so the API adapter can pick 201 vs 200 and the UI adapter can pick its "created" vs "updated" message from one code path. Map the UI's field names to the service's canonical keys in the adapter (e.g. the calendar UI's start_datetime β†’ the service's start_at), so the service speaks one vocabulary.
  2. UI endpoint: check the session, build ActorContext::fromSession($conn), call the service, emit {success:true,…}; catch (Exception $e) β†’ {success:false, error:…}.
  3. API handler: build ActorContext::fromApiKey($apiKey), call the service, serialise + respond; catch (ServiceError $e) β†’ apiError(serviceErrorHttpStatus($e->kind), $e->errorCode, …).
  4. If it's a lifecycle event (created/updated/resolved…), that's the one place to emit a webhook event later β€” it then fires for UI, API and automation alike.

If you're tempted to paste business logic into a second file, stop β€” that second file should be calling the service.

Related

FreeITSM

Getting Started

Modules

Multi-tenancy (planned)

Blue sky thinking

Bugs resolved

Links

Clone this wiki locally