diff --git a/lib/Service/ExampleSet/ExampleSetPrerequisites.php b/lib/Service/ExampleSet/ExampleSetPrerequisites.php index c691af5ebb..255268a712 100644 --- a/lib/Service/ExampleSet/ExampleSetPrerequisites.php +++ b/lib/Service/ExampleSet/ExampleSetPrerequisites.php @@ -103,8 +103,8 @@ public function context(array $plan, DateTimeImmutable $today): array { }//end context() /** - * Import every required register that is not on the instance, or that this set imported and removed the data of, - * and record the import. + * Import every required register that is not on the instance, that this set imported and removed the data of, or + * whose descriptor ships a newer version than the instance holds, and record the import. * * @param array $plan The plan. * @param array $record The record; gains the imports. @@ -120,11 +120,14 @@ private function ensureRegisters(array $plan, array &$record): array { $result['outcome'][$slug] = 'kept'; // A register this set imported whose import was removed again is on the instance without its data. $dataRemoved = (array_key_exists($slug, $record['registers']) === true && $record['registers'][$slug] === []); - if ($this->registers->exists(slug: $slug) === true && $dataRemoved === false) { + $mock = (($required['mock'] ?? false) === true); + if ($this->registers->exists(slug: $slug) === true && $dataRemoved === false + && $this->registers->isBehind(slug: $slug, mock: $mock) === false + ) { continue; } - $import = match (($required['mock'] ?? false) === true) { + $import = match ($mock) { true => $this->registers->importMock(slug: $slug), false => $this->registers->import(slug: $slug), }; diff --git a/lib/Service/ExampleSet/ExampleSetRegisters.php b/lib/Service/ExampleSet/ExampleSetRegisters.php index 9a24e10304..74753177c2 100644 --- a/lib/Service/ExampleSet/ExampleSetRegisters.php +++ b/lib/Service/ExampleSet/ExampleSetRegisters.php @@ -125,6 +125,28 @@ public function importMock(string $slug): array { ); }//end importMock() + /** + * Whether the instance holds an older version of the register than + * OpenRegister's descriptor ships, so the set imports it again. A + * descriptor that cannot be read counts as not behind. + * + * @param string $slug The register slug. + * @param bool $mock True when the set requires the mock descriptor. + * + * @return bool + * + * @phpstan-impure It reads the instance, which an import changes. + * + * @spec openspec/changes/example-sets/specs/example-sets/spec.md#requirement-a-set-imports-the-openregister-registers-it-requires-req-exs-009 + */ + public function isBehind(string $slug, bool $mock): bool { + try { + return $this->container->get(RegisterDescriptorService::class)->isBehind(appId: self::APP, slug: $slug, mock: $mock); + } catch (Throwable) { + return false; + } + }//end isBehind() + /** * Run one descriptor import and return why it failed and the import job it reports. * diff --git a/lib/Service/RegisterDescriptorService.php b/lib/Service/RegisterDescriptorService.php index d8671ec8b1..07037aaa24 100644 --- a/lib/Service/RegisterDescriptorService.php +++ b/lib/Service/RegisterDescriptorService.php @@ -475,6 +475,37 @@ public function reimportMock(string $appId, string $slug): array { ] + $job; }//end reimportMock() + /** + * Whether the instance holds an older version of a register than the + * descriptor of the given kind (mock or real) ships. + * + * A register that is not on the instance, or that no descriptor of the + * kind declares, is not behind: there is nothing to bring up to date. + * + * @param string $appId The app whose descriptors are read. + * @param string $slug The register slug. + * @param bool $mock True for the mock descriptor, false for the real one. + * + * @return bool True when the shipped version is newer than the installed one. + * + * @spec openspec/changes/example-sets/specs/example-sets/spec.md#requirement-a-set-imports-the-openregister-registers-it-requires-req-exs-009 + */ + public function isBehind(string $appId, string $slug, bool $mock): bool { + $descriptor = $this->descriptorFor(appId: $appId, slug: $slug, mock: $mock); + $installed = ($this->installedRegisterVersions()[strtolower($slug)] ?? null); + if ($descriptor === null || $installed === null) { + return false; + } + + foreach ($descriptor['registers'] as $declared => $register) { + if (strtolower((string)$declared) === strtolower($slug)) { + return version_compare((string)($register['version'] ?? '0.0.1'), $installed, '>'); + } + } + + return false; + }//end isBehind() + /** * Slug (lowercased) to version for every register that exists. * diff --git a/lib/Settings/public_space_register.json b/lib/Settings/public_space_register.json index 4805a9932e..68bcab902f 100644 --- a/lib/Settings/public_space_register.json +++ b/lib/Settings/public_space_register.json @@ -3,7 +3,7 @@ "info": { "title": "Public Space Mock Register", "description": "Mock register of objects in public space (openbare ruimte): trees, street lights, play equipment, market pitches and road sections. Example data a case can link an object to; the Zuiddrecht example set imports it and fills it.", - "version": "1.0.0" + "version": "1.0.1" }, "x-openregister": { "type": "mock", @@ -16,6 +16,7 @@ "registers": { "public-space": { "slug": "public-space", + "version": "1.0.1", "title": "Openbare ruimte", "description": "Objects in public space that the municipality manages and that cases are about: trees, street lights, play equipment, market pitches and road sections.", "schemas": [ @@ -32,7 +33,7 @@ "tree": { "slug": "tree", "title": "Bomenregister", - "version": "1.0.0", + "version": "1.0.1", "summary": "A tree the municipality manages in public space.", "description": "A tree the municipality manages in public space.", "type": "object", @@ -109,7 +110,7 @@ "street-light": { "slug": "street-light", "title": "Openbare verlichting", - "version": "1.0.0", + "version": "1.0.1", "summary": "A street light or light mast the municipality manages.", "description": "A street light or light mast the municipality manages.", "type": "object", @@ -175,7 +176,7 @@ "play-equipment": { "slug": "play-equipment", "title": "Buitenruimte", - "version": "1.0.0", + "version": "1.0.1", "summary": "A piece of play equipment on a public playground.", "description": "A piece of play equipment on a public playground.", "type": "object", @@ -322,7 +323,7 @@ "road-section": { "slug": "road-section", "title": "Wegen en paden", - "version": "1.0.0", + "version": "1.0.1", "summary": "A road, cycle path or footpath section the municipality manages.", "description": "A road, cycle path or footpath section the municipality manages.", "type": "object", diff --git a/tests/Unit/Service/Configuration/ImportHandlerSchemaVersioningTest.php b/tests/Unit/Service/Configuration/ImportHandlerSchemaVersioningTest.php index 52a6ea2a1e..9b9fa03e99 100644 --- a/tests/Unit/Service/Configuration/ImportHandlerSchemaVersioningTest.php +++ b/tests/Unit/Service/Configuration/ImportHandlerSchemaVersioningTest.php @@ -257,6 +257,39 @@ public function testANewerVersionWithTheSameDefinitionRecordsNothing(): void { $this->assertSame('1.0.1', $this->written['version']); }//end testANewerVersionWithTheSameDefinitionRecordsNothing() + /** + * An import at a higher version replaces a stored property's definition: + * a location stored as `type: geo`, which no JSON Schema validator accepts, + * becomes the shipped `{"type": "object", "format": "geo:point"}`. + * + * @return void + */ + public function testAHigherVersionReplacesAStoredProperty(): void { + $stored = $this->schema( + id: 31, + version: '1.0.0', + properties: ['name' => ['type' => 'string'], 'location' => ['type' => 'geo']], + required: [] + ); + $this->schemaMapper->method('find')->willReturn($stored); + $this->changelogMapper->method('createFromArray')->willReturn(new SchemaChangelog()); + + $this->handler->importSchema( + data: [ + 'slug' => 'case', + 'title' => 'Case', + 'version' => '1.0.1', + 'properties' => ['name' => ['type' => 'string'], 'location' => ['type' => 'object', 'format' => 'geo:point']], + 'required' => [], + ], + slugsAndIdsMap: [] + ); + + $location = $this->written['properties']['location']; + $this->assertSame(expected: ['object', 'geo:point'], actual: [$location['type'], $location['format']]); + $this->assertSame(expected: '1.0.1', actual: $this->written['version']); + }//end testAHigherVersionReplacesAStoredProperty() + /** * Pass 2 of a configuration import keeps the version Pass 1 bumped to (#4163). * diff --git a/tests/Unit/Service/ExampleSet/ExampleSetRequirementsTest.php b/tests/Unit/Service/ExampleSet/ExampleSetRequirementsTest.php index 19526edcf0..e81b0b603e 100644 --- a/tests/Unit/Service/ExampleSet/ExampleSetRequirementsTest.php +++ b/tests/Unit/Service/ExampleSet/ExampleSetRequirementsTest.php @@ -45,6 +45,26 @@ * @uses \OCA\OpenRegister\Service\ExampleSet\ExampleSetViews */ class ExampleSetRequirementsTest extends ExampleSetTestCase { + /** + * A register already on the instance is imported again when the shipped + * descriptor is a newer version, so a schema fixed in the descriptor + * reaches an instance that installed the set before. + * + * @return void + */ + public function testARegisterBehindItsDescriptorIsImportedAgain(): void { + $this->store->offer(register: 'brp', schema: 'person'); + $this->registers->present['brp'] = true; + $this->registers->importable['brp'] = true; + $this->registers->behind['brp'] = true; + $this->app(app: 'openregister', fragment: $this->castRequiringBrp()); + + $report = $this->installer->install(plan: $this->catalogue->gather(set: 'demo'), today: $this->today()); + + $this->assertSame(expected: ['brp' => 'imported'], actual: $report['registers']); + $this->assertSame(expected: 1, actual: $this->registers->imports); + }//end testARegisterBehindItsDescriptorIsImportedAgain() + /** * A cast file that requires the `brp` register and writes one person into it. * diff --git a/tests/Unit/Service/ExampleSet/Fake/FakeExampleSetRegisters.php b/tests/Unit/Service/ExampleSet/Fake/FakeExampleSetRegisters.php index e221af7f22..f447b252ba 100644 --- a/tests/Unit/Service/ExampleSet/Fake/FakeExampleSetRegisters.php +++ b/tests/Unit/Service/ExampleSet/Fake/FakeExampleSetRegisters.php @@ -44,6 +44,25 @@ class FakeExampleSetRegisters extends ExampleSetRegisters { /** @var int Next job number. */ private int $next = 1; + /** + * Slugs whose descriptor ships a newer version than the instance holds. + * + * @var array + */ + public array $behind = []; + + /** + * Whether the slug is in `behind`. + * + * @param string $slug The register slug. + * @param bool $mock Whether the mock descriptor is meant. + * + * @return bool + */ + public function isBehind(string $slug, bool $mock): bool { + return isset($this->behind[$slug]); + } + /** * No container. */ diff --git a/tests/Unit/Service/RegisterDescriptorServiceTest.php b/tests/Unit/Service/RegisterDescriptorServiceTest.php index 5e0ad652d7..a2b68af2f9 100644 --- a/tests/Unit/Service/RegisterDescriptorServiceTest.php +++ b/tests/Unit/Service/RegisterDescriptorServiceTest.php @@ -348,6 +348,24 @@ public function testReimportForcesPastTheVersionGate(): void { $this->assertSame('imported', $result['outcome']); } + /** + * A register is behind when the descriptor of the asked kind ships a newer + * version than the instance holds; same version, the other kind or a + * register that is not there is not behind. + * + * @return void + */ + public function testARegisterIsBehindOnlyWhenItsDescriptorIsNewer(): void { + $this->writeDescriptor(file: 'a_mock_register.json', slug: 'flows', version: '1.0.1', type: 'mock'); + $this->writeDescriptor(file: 'flows_register.json', slug: 'flows', version: '1.0.0'); + $this->expectApp(appId: 'fixtureapp'); + $this->registerMapper->method('findAll')->willReturn([$this->register(slug: 'flows', version: '1.0.0')]); + + $this->assertTrue(condition: $this->service->isBehind(appId: 'fixtureapp', slug: 'flows', mock: true)); + $this->assertFalse(condition: $this->service->isBehind(appId: 'fixtureapp', slug: 'flows', mock: false)); + $this->assertFalse(condition: $this->service->isBehind(appId: 'fixtureapp', slug: 'other', mock: true)); + }//end testARegisterIsBehindOnlyWhenItsDescriptorIsNewer() + /** * 🔴 A MOCK IMPORTS UNDER ITS OWN CONFIGURATION IDENTITY. *