diff --git a/app/Exports/ZipExports/Models/ZipExportAttachment.php b/app/Exports/ZipExports/Models/ZipExportAttachment.php index c6615e1dc49..4f5b2f23699 100644 --- a/app/Exports/ZipExports/Models/ZipExportAttachment.php +++ b/app/Exports/ZipExports/Models/ZipExportAttachment.php @@ -43,7 +43,7 @@ public static function fromModelArray(array $attachmentArray, ZipExportFiles $fi public static function validate(ZipValidationHelper $context, array $data): array { $rules = [ - 'id' => ['nullable', 'int'], + 'id' => ['nullable', 'int', $context->uniqueIdRule('attachment')], 'name' => ['required', 'string', 'min:1'], 'link' => ['required_without:file', 'nullable', 'string'], 'file' => ['required_without:link', 'nullable', 'string', $context->fileReferenceRule()], diff --git a/app/Exports/ZipExports/Models/ZipExportBook.php b/app/Exports/ZipExports/Models/ZipExportBook.php index 0dc4e93d43c..47ab8f0a699 100644 --- a/app/Exports/ZipExports/Models/ZipExportBook.php +++ b/app/Exports/ZipExports/Models/ZipExportBook.php @@ -70,7 +70,7 @@ public static function fromModel(Book $model, ZipExportFiles $files): self public static function validate(ZipValidationHelper $context, array $data): array { $rules = [ - 'id' => ['nullable', 'int'], + 'id' => ['nullable', 'int', $context->uniqueIdRule('book')], 'name' => ['required', 'string', 'min:1'], 'description_html' => ['nullable', 'string'], 'cover' => ['nullable', 'string', $context->fileReferenceRule()], diff --git a/app/Exports/ZipExports/Models/ZipExportChapter.php b/app/Exports/ZipExports/Models/ZipExportChapter.php index 50440d61a5a..5a5fe350f3a 100644 --- a/app/Exports/ZipExports/Models/ZipExportChapter.php +++ b/app/Exports/ZipExports/Models/ZipExportChapter.php @@ -59,7 +59,7 @@ public static function fromModelArray(array $chapterArray, ZipExportFiles $files public static function validate(ZipValidationHelper $context, array $data): array { $rules = [ - 'id' => ['nullable', 'int'], + 'id' => ['nullable', 'int', $context->uniqueIdRule('chapter')], 'name' => ['required', 'string', 'min:1'], 'description_html' => ['nullable', 'string'], 'priority' => ['nullable', 'int'], diff --git a/app/Exports/ZipExports/Models/ZipExportImage.php b/app/Exports/ZipExports/Models/ZipExportImage.php index 691eb918fc6..89083b15be1 100644 --- a/app/Exports/ZipExports/Models/ZipExportImage.php +++ b/app/Exports/ZipExports/Models/ZipExportImage.php @@ -33,7 +33,7 @@ public function metadataOnly(): void public static function validate(ZipValidationHelper $context, array $data): array { $rules = [ - 'id' => ['nullable', 'int'], + 'id' => ['nullable', 'int', $context->uniqueIdRule('image')], 'name' => ['required', 'string', 'min:1'], 'file' => ['required', 'string', $context->fileReferenceRule()], 'type' => ['required', 'string', Rule::in(['gallery', 'drawio'])], diff --git a/app/Exports/ZipExports/Models/ZipExportPage.php b/app/Exports/ZipExports/Models/ZipExportPage.php index 3a876e7aaff..16e7e925539 100644 --- a/app/Exports/ZipExports/Models/ZipExportPage.php +++ b/app/Exports/ZipExports/Models/ZipExportPage.php @@ -68,7 +68,7 @@ public static function fromModelArray(array $pageArray, ZipExportFiles $files): public static function validate(ZipValidationHelper $context, array $data): array { $rules = [ - 'id' => ['nullable', 'int'], + 'id' => ['nullable', 'int', $context->uniqueIdRule('page')], 'name' => ['required', 'string', 'min:1'], 'html' => ['nullable', 'string'], 'markdown' => ['nullable', 'string'], diff --git a/app/Exports/ZipExports/ZipFileReferenceRule.php b/app/Exports/ZipExports/ZipFileReferenceRule.php index bcd3c39acf0..7d6c829cf03 100644 --- a/app/Exports/ZipExports/ZipFileReferenceRule.php +++ b/app/Exports/ZipExports/ZipFileReferenceRule.php @@ -4,7 +4,6 @@ use Closure; use Illuminate\Contracts\Validation\ValidationRule; -use ZipArchive; class ZipFileReferenceRule implements ValidationRule { diff --git a/app/Exports/ZipExports/ZipUniqueIdRule.php b/app/Exports/ZipExports/ZipUniqueIdRule.php new file mode 100644 index 00000000000..ea2b2539296 --- /dev/null +++ b/app/Exports/ZipExports/ZipUniqueIdRule.php @@ -0,0 +1,26 @@ +context->hasIdBeenUsed($this->modelType, $value)) { + $fail('validation.zip_unique')->translate(['attribute' => $attribute]); + } + } +} diff --git a/app/Exports/ZipExports/ZipValidationHelper.php b/app/Exports/ZipExports/ZipValidationHelper.php index 55c86b03b5b..7659c228bcd 100644 --- a/app/Exports/ZipExports/ZipValidationHelper.php +++ b/app/Exports/ZipExports/ZipValidationHelper.php @@ -9,6 +9,13 @@ class ZipValidationHelper { protected Factory $validationFactory; + /** + * Local store of validated IDs (in format ":". Example: "book:2") + * which we can use to check uniqueness. + * @var array + */ + protected array $validatedIds = []; + public function __construct( public ZipExportReader $zipReader, ) { @@ -31,6 +38,23 @@ public function fileReferenceRule(): ZipFileReferenceRule return new ZipFileReferenceRule($this); } + public function uniqueIdRule(string $type): ZipUniqueIdRule + { + return new ZipUniqueIdRule($this, $type); + } + + public function hasIdBeenUsed(string $type, int $id): bool + { + $key = $type . ':' . $id; + if (isset($this->validatedIds[$key])) { + return true; + } + + $this->validatedIds[$key] = true; + + return false; + } + /** * Validate an array of relation data arrays that are expected * to be for the given ZipExportModel. diff --git a/lang/en/validation.php b/lang/en/validation.php index bc01ac47b94..fdfc3d9a9b9 100644 --- a/lang/en/validation.php +++ b/lang/en/validation.php @@ -107,6 +107,7 @@ 'zip_file' => 'The :attribute needs to reference a file within the ZIP.', 'zip_model_expected' => 'Data object expected but ":type" found.', + 'zip_unique' => 'The :attribute must be unique for the object type within the ZIP.', // Custom validation lines 'custom' => [ diff --git a/tests/Exports/ZipExportValidatorTests.php b/tests/Exports/ZipExportValidatorTests.php new file mode 100644 index 00000000000..4cacea95ec6 --- /dev/null +++ b/tests/Exports/ZipExportValidatorTests.php @@ -0,0 +1,74 @@ +filesToRemove as $file) { + unlink($file); + } + + parent::tearDown(); + } + + protected function getValidatorForData(array $zipData, array $files = []): ZipExportValidator + { + $upload = ZipTestHelper::zipUploadFromData($zipData, $files); + $path = $upload->getRealPath(); + $this->filesToRemove[] = $path; + $reader = new ZipExportReader($path); + return new ZipExportValidator($reader); + } + + public function test_ids_have_to_be_unique() + { + $validator = $this->getValidatorForData([ + 'book' => [ + 'id' => 4, + 'name' => 'My book', + 'pages' => [ + [ + 'id' => 4, + 'name' => 'My page', + 'markdown' => 'hello', + 'attachments' => [ + ['id' => 4, 'name' => 'Attachment A', 'link' => 'https://example.com'], + ['id' => 4, 'name' => 'Attachment B', 'link' => 'https://example.com'] + ], + 'images' => [ + ['id' => 4, 'name' => 'Image A', 'type' => 'gallery', 'file' => 'cat'], + ['id' => 4, 'name' => 'Image b', 'type' => 'gallery', 'file' => 'cat'], + ], + ], + ['id' => 4, 'name' => 'My page', 'markdown' => 'hello'], + ], + 'chapters' => [ + ['id' => 4, 'name' => 'Chapter 1'], + ['id' => 4, 'name' => 'Chapter 2'] + ] + ] + ], ['cat' => $this->files->testFilePath('test-image.png')]); + + $results = $validator->validate(); + $this->assertCount(4, $results); + + $expectedMessage = 'The id must be unique for the object type within the ZIP.'; + $this->assertEquals($expectedMessage, $results['book.pages.0.attachments.1.id']); + $this->assertEquals($expectedMessage, $results['book.pages.0.images.1.id']); + $this->assertEquals($expectedMessage, $results['book.pages.1.id']); + $this->assertEquals($expectedMessage, $results['book.chapters.1.id']); + } +}