update
Some checks failed
Tests / lint (push) Has been cancelled
Tests / test (push) Has been cancelled
Tests / test-postgres (push) Has been cancelled

This commit is contained in:
2026-08-21 09:14:51 +02:00
parent 618d3a56ad
commit c8ba8d7b14
5 changed files with 388 additions and 105 deletions

View File

@@ -1274,13 +1274,18 @@ a leer ese documento y no este código.
Dos cosas lo sujetan: Dos cosas lo sujetan:
**Un test de contrato** ([`PublicApiContractTest`](tests/Feature/Api/PublicApiContractTest.php)) **Dos tests de contrato**, uno por mitad de la API, comparten la comparación en
que llama a cada endpoint público con datos sembrados y compara la respuesta con sí ([`VerifiesOpenApiContract`](tests/VerifiesOpenApiContract.php)) porque la
el esquema del `openapi.yaml`, clave a clave. La regla es asimétrica a pregunta es la misma dos veces: si la forma que el documento promete es la que
propósito: **todo lo que la API manda tiene que estar documentado** —siempre—, y la API manda. Lo que cambia es todo lo que rodea la pregunta, no la
lo que el documento promete tiene que llegar en los endpoints de detalle, salvo comparación.
lo que está marcado `nullable`, que es como este documento dice «puede no
estar». [`PublicApiContractTest`](tests/Feature/Api/PublicApiContractTest.php) llama a
cada endpoint público con datos sembrados y compara la respuesta con el esquema
del `openapi.yaml`, clave a clave. La regla es asimétrica a propósito: **todo lo
que la API manda tiene que estar documentado** —siempre—, y lo que el documento
promete tiene que llegar en los endpoints de detalle, salvo lo que está marcado
`nullable`, que es como este documento dice «puede no estar».
Al escribirlo aparecieron cuatro desajustes que llevaban ahí un tiempo: Al escribirlo aparecieron cuatro desajustes que llevaban ahí un tiempo:
`meta_title` y `meta_description` sin documentar en proyectos y servicios, `meta_title` y `meta_description` sin documentar en proyectos y servicios,
@@ -1292,6 +1297,19 @@ era la API: el alt vive dentro de la imagen, que es donde lo necesita un `<img>`
y dos nombres para el mismo texto es cómo un frontend acaba leyendo el que y dos nombres para el mismo texto es cómo un frontend acaba leyendo el que
resulta ser el accidental. resulta ser el accidental.
[`AdminApiContractTest`](tests/Feature/Api/AdminApiContractTest.php) hace lo
mismo con `/v1/*`, autenticado como `super_admin` para que un permiso de menos
nunca se confunda con una forma equivocada, y añade una comprobación que la
pública no necesita: el **sobre de paginación** (`links`, `meta`) de cada
listado contra lo documentado, y que `/v1/roles` —el único listado que Laravel
no pagina— no lleve ninguno de los dos. Escribirlo encontró cuatro desajustes
más: `featured_image_alt` sin documentar en absoluto en la respuesta de página
(la petición de escritura sí lo tenía, con otra forma), `permissions` de
usuario documentado solo en prosa en vez de con `nullable: true`, y el sobre de
paginación real de Laravel —que manda `from`, `to`, `path` y un `links` de
primer nivel— documentado con la mitad de esas claves y sin ese `links` en
absoluto.
**Tipos generados** para el otro lado: **Tipos generados** para el otro lado:
```bash ```bash
@@ -1346,7 +1364,7 @@ php artisan site:maintenance --up
## Tests ## Tests
```bash ```bash
php artisan test # 391 tests, 1121 aserciones php artisan test # 394 tests, 1128 aserciones
``` ```
En cada push y cada pull request, [`.github/workflows/tests.yml`](.github/workflows/tests.yml) En cada push y cada pull request, [`.github/workflows/tests.yml`](.github/workflows/tests.yml)
@@ -1569,7 +1587,7 @@ docker compose -f docker-compose.test.yml run --rm test
Levanta un Postgres en memoria, corre la suite entera contra él y lo tira. Es la Levanta un Postgres en memoria, corre la suite entera contra él y lo tira. Es la
comprobación que convierte «debería funcionar» en «funciona», y conviene hacerla comprobación que convierte «debería funcionar» en «funciona», y conviene hacerla
antes del primer despliegue. **391 tests en verde contra Postgres 17.** antes del primer despliegue. **394 tests en verde contra Postgres 17.**
Usa una etapa propia de la imagen: la de producción se construye con `--no-dev` Usa una etapa propia de la imagen: la de producción se construye con `--no-dev`
y ahí no está ni el runner de tests. y ahí no está ni el runner de tests.

View File

@@ -1198,6 +1198,7 @@ paths:
data: data:
type: array type: array
items: { $ref: "#/components/schemas/User" } items: { $ref: "#/components/schemas/User" }
links: { $ref: "#/components/schemas/PaginationLinks" }
meta: { $ref: "#/components/schemas/PaginationMeta" } meta: { $ref: "#/components/schemas/PaginationMeta" }
"403": { $ref: "#/components/responses/Forbidden" } "403": { $ref: "#/components/responses/Forbidden" }
@@ -2075,8 +2076,16 @@ components:
deleted_at: { type: string, format: date-time, nullable: true } deleted_at: { type: string, format: date-time, nullable: true }
category: { $ref: "#/components/schemas/Category" } category: { $ref: "#/components/schemas/Category" }
author: { $ref: "#/components/schemas/User" } author: { $ref: "#/components/schemas/User" }
featured_image_alt:
type: string
nullable: true
description: |
What the image shows, resolved into the language above — unlike
`PageWrite`'s field of the same name, which carries every
language at once.
translations: translations:
type: object type: object
nullable: true
description: Present only when `?with=translations` was passed. description: Present only when `?with=translations` was passed.
additionalProperties: additionalProperties:
type: object type: object
@@ -2128,6 +2137,13 @@ components:
pages_count: { type: integer } pages_count: { type: integer }
created_at: { type: string, format: date-time } created_at: { type: string, format: date-time }
updated_at: { type: string, format: date-time } updated_at: { type: string, format: date-time }
translations:
type: object
nullable: true
description: Present only when `?with=translations` was passed.
additionalProperties:
type: object
additionalProperties: { type: string }
CategoryWrite: CategoryWrite:
type: object type: object
@@ -2160,6 +2176,7 @@ components:
permissions: permissions:
type: array type: array
items: { type: string } items: { type: string }
nullable: true
description: Effective local permissions. Returned by `/v1/me` only. description: Effective local permissions. Returned by `/v1/me` only.
remote_roles: remote_roles:
type: array type: array
@@ -2221,18 +2238,45 @@ components:
PaginationMeta: PaginationMeta:
type: object type: object
description: |
Laravel's own paginator shape, sent as-is — nothing on this side
trims it down, so this describes exactly what `->paginate()` sends
rather than a curated subset of it.
properties: properties:
current_page: { type: integer } current_page: { type: integer }
from: { type: integer, nullable: true, description: Null on an empty page. }
last_page: { type: integer } last_page: { type: integer }
links:
type: array
description: One entry per page number shown by a "1 2 3 … 9" control, plus prev/next.
items:
type: object
properties:
url: { type: string, nullable: true }
label: { type: string, description: "Already translated — e.g. «Anterior», not «Previous»." }
page: { type: integer, nullable: true }
active: { type: boolean }
path: { type: string }
per_page: { type: integer } per_page: { type: integer }
to: { type: integer, nullable: true, description: Null on an empty page. }
total: { type: integer } total: { type: integer }
PaginationLinks:
type: object
description: The four addresses a client actually navigates with, ready to use verbatim.
properties:
first: { type: string }
last: { type: string }
prev: { type: string, nullable: true }
next: { type: string, nullable: true }
PageCollection: PageCollection:
type: object type: object
properties: properties:
data: data:
type: array type: array
items: { $ref: "#/components/schemas/Page" } items: { $ref: "#/components/schemas/Page" }
links: { $ref: "#/components/schemas/PaginationLinks" }
meta: { $ref: "#/components/schemas/PaginationMeta" } meta: { $ref: "#/components/schemas/PaginationMeta" }
CategoryCollection: CategoryCollection:
@@ -2241,4 +2285,5 @@ components:
data: data:
type: array type: array
items: { $ref: "#/components/schemas/Category" } items: { $ref: "#/components/schemas/Category" }
links: { $ref: "#/components/schemas/PaginationLinks" }
meta: { $ref: "#/components/schemas/PaginationMeta" } meta: { $ref: "#/components/schemas/PaginationMeta" }

View File

@@ -0,0 +1,179 @@
<?php
declare(strict_types=1);
namespace Tests\Feature\Api;
use App\Models\Category;
use App\Models\Page;
use App\Models\User;
use Database\Seeders\RolePermissionSeeder;
use Illuminate\Foundation\Testing\RefreshDatabase;
use PHPUnit\Framework\Attributes\Test;
use Tests\FakesPireon;
use Tests\TestCase;
use Tests\VerifiesOpenApiContract;
/**
* The administration API answers with what the specification says it does —
* `PublicApiContractTest`'s other half.
*
* The frontend reads the public document from another repository, in a
* framework not yet chosen; whoever writes it will never see this code, only
* the specification. That case does not apply here — an admin API caller is
* inside this codebase, or at least inside this organisation — but the
* underlying failure is identical: a field renamed in a `Http\Resources\*`
* class is a change nothing else in this suite notices, because every other
* test exercises behaviour, not shape.
*
* Unlike the public listings, an admin index and its matching show endpoint
* send the same full record — none of the four resources here has a
* "lightweight for a listing" projection — so both are held to the stricter
* rule throughout: everything documented and not `nullable` must be present.
*/
class AdminApiContractTest extends TestCase
{
use FakesPireon, RefreshDatabase, VerifiesOpenApiContract;
protected User $caller;
protected function setUp(): void
{
parent::setUp();
$this->loadOpenApiSpec();
$this->seed(RolePermissionSeeder::class);
// super_admin, so a permission gap never masquerades as a shape
// mismatch: every request here must reach the resource, not a 403.
$this->caller = User::factory()->create();
$this->caller->syncRoles(['super_admin']);
$this->withHeaders($this->apiHeadersFor($this->caller));
}
/**
* @return array<string, array{path: string}>
*/
public static function paginatedEndpoints(): array
{
return [
'pages' => ['path' => '/v1/pages'],
'categories' => ['path' => '/v1/categories'],
'users' => ['path' => '/v1/users'],
];
}
#[Test]
public function every_admin_record_answers_in_the_shape_it_documents(): void
{
$page = Page::factory()->create();
$category = Category::factory()->create();
// One request, one documented path — the URL a real caller hits and
// the path key that describes it, side by side rather than derived
// from one another, because guessing the second from the first is
// exactly the kind of cleverness that breaks quietly.
$cases = [
'pages' => ['/v1/pages', '/v1/pages'],
'page' => ["/v1/pages/{$page->id}", '/v1/pages/{page}'],
'categories' => ['/v1/categories', '/v1/categories'],
'category' => ["/v1/categories/{$category->id}", '/v1/categories/{category}'],
'users' => ['/v1/users', '/v1/users'],
'user' => ["/v1/users/{$this->caller->id}", '/v1/users/{user}'],
'me' => ['/v1/me', '/v1/me'],
'roles' => ['/v1/roles', '/v1/roles'],
];
$problems = [];
foreach ($cases as $name => [$url, $documentedPath]) {
$response = $this->getJson('/api'.$url);
if ($response->status() !== 200) {
$problems[] = "{$name}: answered {$response->status()}";
continue;
}
$payload = $response->json('data');
$record = array_is_list((array) $payload) ? ($payload[0] ?? null) : $payload;
if (! is_array($record)) {
$problems[] = "{$name}: no record to compare against";
continue;
}
$schema = $this->recordSchema($documentedPath);
foreach (array_diff(array_keys($record), $this->documentedKeys($schema)) as $undocumented) {
$problems[] = "{$name}: sends `{$undocumented}`, which the specification does not describe";
}
foreach (array_diff($this->alwaysPresent($schema), array_keys($record)) as $missing) {
$problems[] = "{$name}: documents `{$missing}`, which it does not send";
}
}
$this->assertSame([], $problems, "The admin API and its specification disagree:\n".implode("\n", $problems));
}
/**
* The pagination envelope every listing here uses — `data`, `links`,
* `meta` — checked the same way the record inside it is: nothing sent
* that is not documented.
*
* Worth its own test rather than folding into the one above: the record
* comparison already proves the endpoints work at all, and a failure here
* points at a completely different place to fix — the collection schema,
* not a `Resource` class.
*/
#[Test]
public function the_pagination_envelope_matches_what_is_documented(): void
{
Page::factory()->create();
Category::factory()->create();
$problems = [];
foreach (self::paginatedEndpoints() as $name => $endpoint) {
$response = $this->getJson('/api'.$endpoint['path'])->assertOk();
$envelope = $this->responseSchema($endpoint['path']);
$documented = $this->documentedKeys($envelope);
foreach (array_diff(array_keys($response->json()), $documented) as $undocumented) {
$problems[] = "{$name}: envelope sends `{$undocumented}`, which the specification does not describe";
}
$metaSchema = $this->resolve($envelope['properties']['meta'] ?? []);
$meta = (array) $response->json('meta');
foreach (array_diff(array_keys($meta), $this->documentedKeys($metaSchema)) as $undocumented) {
$problems[] = "{$name}: meta sends `{$undocumented}`, which the specification does not describe";
}
foreach (array_diff($this->alwaysPresent($metaSchema), array_keys($meta)) as $missing) {
$problems[] = "{$name}: meta documents `{$missing}`, which it does not send";
}
}
$this->assertSame([], $problems, "The pagination envelope and its specification disagree:\n".implode("\n", $problems));
}
/**
* `/v1/roles` is the one listing here Laravel never paginates —
* `RoleController::index()` calls `->get()`, not `->paginate()` — so
* proving it carries no stray `meta`/`links` is the other half of the
* envelope test above.
*/
#[Test]
public function the_unpaginated_roles_listing_carries_no_pagination_envelope(): void
{
$response = $this->getJson('/api/v1/roles')->assertOk();
$this->assertSame(['data'], array_keys($response->json()));
}
}

View File

@@ -15,8 +15,8 @@ use App\Models\Technology;
use App\Models\Testimonial; use App\Models\Testimonial;
use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Foundation\Testing\RefreshDatabase;
use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\Attributes\Test;
use Symfony\Component\Yaml\Yaml;
use Tests\TestCase; use Tests\TestCase;
use Tests\VerifiesOpenApiContract;
/** /**
* The public API answers with what the specification says it answers with. * The public API answers with what the specification says it answers with.
@@ -34,12 +34,7 @@ use Tests\TestCase;
*/ */
class PublicApiContractTest extends TestCase class PublicApiContractTest extends TestCase
{ {
use RefreshDatabase; use RefreshDatabase, VerifiesOpenApiContract;
/**
* @var array<string, mixed>
*/
protected array $spec;
protected function setUp(): void protected function setUp(): void
{ {
@@ -47,7 +42,7 @@ class PublicApiContractTest extends TestCase
$this->withHeaders(['Accept-Language' => 'es']); $this->withHeaders(['Accept-Language' => 'es']);
$this->spec = (array) Yaml::parseFile(public_path('openapi.yaml')); $this->loadOpenApiSpec();
} }
/** /**
@@ -171,14 +166,15 @@ class PublicApiContractTest extends TestCase
continue; continue;
} }
$documented = $this->documentedKeys($endpoint['path']); $schema = $this->recordSchema($endpoint['path']);
$documented = $this->documentedKeys($schema);
foreach (array_diff(array_keys($record), $documented) as $undocumented) { foreach (array_diff(array_keys($record), $documented) as $undocumented) {
$problems[] = "{$name}: sends `{$undocumented}`, which the specification does not describe"; $problems[] = "{$name}: sends `{$undocumented}`, which the specification does not describe";
} }
if ($endpoint['detail']) { if ($endpoint['detail']) {
foreach (array_diff($this->alwaysPresent($endpoint['path']), array_keys($record)) as $missing) { foreach (array_diff($this->alwaysPresent($schema), array_keys($record)) as $missing) {
$problems[] = "{$name}: documents `{$missing}`, which it does not send"; $problems[] = "{$name}: documents `{$missing}`, which it does not send";
} }
} }
@@ -186,90 +182,4 @@ class PublicApiContractTest extends TestCase
$this->assertSame([], $problems, "The public API and its specification disagree:\n".implode("\n", $problems)); $this->assertSame([], $problems, "The public API and its specification disagree:\n".implode("\n", $problems));
} }
/**
* The field names the specification promises for one endpoint's records.
*
* @return array<int, string>
*/
protected function documentedKeys(string $path): array
{
return array_keys($this->recordSchema($path)['properties'] ?? []);
}
/**
* The schema of a single record, whichever way the endpoint wraps it.
*
* @return array<string, mixed>
*/
protected function recordSchema(string $path): array
{
$schema = $this->resolve(
$this->spec['paths'][$path]['get']['responses'][200]['content']['application/json']['schema'] ?? [],
);
$data = $this->resolve($schema['properties']['data'] ?? []);
// A listing documents its records under `items`; a detail endpoint
// describes them directly.
if (($data['type'] ?? null) === 'array') {
$data = $this->resolve($data['items'] ?? []);
}
return $data;
}
/**
* The documented fields a detail endpoint must always answer with.
*
* Anything the specification marks `nullable` is left out of this half of
* the check. That is the vocabulary this document already uses for a field
* that may not be there — a member's `role` exists only while they are
* nested under a project, and the schema says so. The other direction
* stays strict for every field either way: whatever the API sends has to
* be described, nullable or not.
*
* @return array<int, string>
*/
protected function alwaysPresent(string $path): array
{
$record = $this->recordSchema($path);
return array_keys(array_filter(
(array) ($record['properties'] ?? []),
fn (array $property) => ($property['nullable'] ?? false) !== true,
));
}
/**
* Follow `$ref` and flatten `allOf`, which is how the specification says
* "this, plus a couple of fields" without repeating a schema.
*
* @param array<string, mixed> $schema
* @return array<string, mixed>
*/
protected function resolve(array $schema): array
{
if (isset($schema['$ref'])) {
$name = basename((string) $schema['$ref']);
return $this->resolve((array) ($this->spec['components']['schemas'][$name] ?? []));
}
if (isset($schema['allOf'])) {
$merged = ['type' => 'object', 'properties' => []];
foreach ((array) $schema['allOf'] as $part) {
$resolved = $this->resolve((array) $part);
$merged['properties'] = [
...$merged['properties'],
...($resolved['properties'] ?? []),
];
}
return [...$schema, ...$merged];
}
return $schema;
}
} }

View File

@@ -0,0 +1,131 @@
<?php
declare(strict_types=1);
namespace Tests;
use Symfony\Component\Yaml\Yaml;
/**
* Compares an actual JSON response against what `public/openapi.yaml` says
* it should look like.
*
* Shared between the public and the admin contract tests because the
* question is the same one twice: does the shape the document promises match
* the shape the API sends. What differs between the two is everything
* *around* the question — how a caller authenticates, what a listing is
* allowed to leave out — not the comparison itself.
*/
trait VerifiesOpenApiContract
{
/**
* @var array<string, mixed>
*/
protected array $spec;
protected function loadOpenApiSpec(): void
{
$this->spec = (array) Yaml::parseFile(public_path('openapi.yaml'));
}
/**
* The field names a schema promises, one level deep — enough for a flat
* comparison against `array_keys()` on a decoded JSON object.
*
* @return array<int, string>
*/
protected function documentedKeys(array $schema): array
{
return array_keys($this->resolve($schema)['properties'] ?? []);
}
/**
* The documented fields that must always be present.
*
* Anything the specification marks `nullable` is left out of this half of
* the check — that is the vocabulary this document already uses for a
* field that may not be there, whether because the value itself can be
* null or because it only appears down one particular route. The other
* direction stays strict regardless: whatever is actually sent has to be
* described, nullable or not.
*
* @return array<int, string>
*/
protected function alwaysPresent(array $schema): array
{
$resolved = $this->resolve($schema);
return array_keys(array_filter(
(array) ($resolved['properties'] ?? []),
fn (array $property) => ($property['nullable'] ?? false) !== true,
));
}
/**
* The schema of one record inside a `{"data": ...}` envelope, whichever
* way the endpoint wraps it — a bare object for a detail endpoint, or the
* `items` of an array for a listing.
*
* @return array<string, mixed>
*/
protected function recordSchema(string $path, string $method = 'get', int $status = 200): array
{
$schema = $this->resolve(
$this->spec['paths'][$path][$method]['responses'][$status]['content']['application/json']['schema'] ?? [],
);
$data = $this->resolve($schema['properties']['data'] ?? []);
if (($data['type'] ?? null) === 'array') {
$data = $this->resolve($data['items'] ?? []);
}
return $data;
}
/**
* Everything documented for one path's response, `data` included — for
* comparing against the envelope as a whole rather than only the record
* inside it.
*
* @return array<string, mixed>
*/
protected function responseSchema(string $path, string $method = 'get', int $status = 200): array
{
return $this->resolve(
$this->spec['paths'][$path][$method]['responses'][$status]['content']['application/json']['schema'] ?? [],
);
}
/**
* Follow `$ref` and flatten `allOf`, which is how the specification says
* "this, plus a couple of fields" without repeating a schema.
*
* @param array<string, mixed> $schema
* @return array<string, mixed>
*/
protected function resolve(array $schema): array
{
if (isset($schema['$ref'])) {
$name = basename((string) $schema['$ref']);
return $this->resolve((array) ($this->spec['components']['schemas'][$name] ?? []));
}
if (isset($schema['allOf'])) {
$merged = ['type' => 'object', 'properties' => []];
foreach ((array) $schema['allOf'] as $part) {
$resolved = $this->resolve((array) $part);
$merged['properties'] = [
...$merged['properties'],
...($resolved['properties'] ?? []),
];
}
return [...$schema, ...$merged];
}
return $schema;
}
}