From 1651ddf7239992a0756253b82741baa12680d09c Mon Sep 17 00:00:00 2001 From: Glen Robson Date: Fri, 27 Mar 2020 01:12:06 +0000 Subject: [PATCH 1/2] Fixing issue 97 500 error A number of issues here. The error finder seemed to struggle when the error was in the root. Also decided to limit the allowable fields in the manifest to catch old fields like sequence --- fixtures/3/old_format_label.json | 52 ++++++++++++++++++++++++++++++++ schema/error_processor.py | 17 +++++++---- schema/iiif_3_0.json | 51 +++++++++++++++++-------------- schema/schemavalidator.py | 3 ++ tests/test_validator.py | 11 ++++++- 5 files changed, 104 insertions(+), 30 deletions(-) create mode 100644 fixtures/3/old_format_label.json diff --git a/fixtures/3/old_format_label.json b/fixtures/3/old_format_label.json new file mode 100644 index 0000000..24c4226 --- /dev/null +++ b/fixtures/3/old_format_label.json @@ -0,0 +1,52 @@ +{ + "@context": [ + "http://www.w3.org/ns/anno.jsonld", + "http://iiif.io/api/presentation/3/context.json" + ], + "id": "https://example.com/manifest", + "label": "Old table label which doesn't have a language", + "type": "Manifest", + "sequences": [ + { + "id": "https://example.com/sequence", + "label": "Current order", + "type": "Sequence", + "viewingDirection": "left-to-right", + "canvases": [ + { + "type": "Canvas", + "id": "https://example.com/canvas1", + "label": "Image 1", + "height": 3820, + "width": 5426, + "content": [ + { + "type": "AnnotationPage", + "id": "https://example.com/anno1", + "items": [ + { + "id": "https://example.com/image1", + "type": "Annotation", + "motivation": "painting", + "target": "https://example.com/canvas1", + "body": { + "type": "Image", + "id": "https://example.com/image/full.jpg", + "format": "image/jpeg", + "height": 3820, + "width": 5426, + "service": { + "@context": "http://iiif.io/api/image/2/context.json", + "id": "https://example.com/image1/api/1", + "profile": "http://iiif.io/api/image/2/level2.json" + } + } + } + ] + } + ] + } + ] + } + ] +} diff --git a/schema/error_processor.py b/schema/error_processor.py index df60944..f1567a0 100755 --- a/schema/error_processor.py +++ b/schema/error_processor.py @@ -115,10 +115,10 @@ def diagnoseWhichOneOf(self, error_path, IIIFJsonPath): # if this oneOf possiblity is still relevant add it to the list and check # its not another oneOf error if addErrors: - # if error is also a oneOf then diagnoise aggain - if err.absolute_schema_path[-1] == 'oneOf': + # if error is also a oneOf then diagnoise again + if err.absolute_schema_path[-1] == 'oneOf' and err.absolute_schema_path != error_path: error_path.append(oneOfIndex) # this is is related to one of the original oneOfs at index oneOfIndex - error_path.append('oneOf') # but we found another oneOf test at this location + error_path.extend(err.absolute_schema_path) # but we found another oneOf test at this location store_errs.append(self.diagnoseWhichOneOf(error_path, IIIFJsonPath)) # so recursivly discovery real error #print ('would add: {} by addErrors is {}'.format(err.message, addErrors)) store_errs.append(err) @@ -142,7 +142,7 @@ def diagnoseWhichOneOf(self, error_path, IIIFJsonPath): path = parse(self.pathToJsonPath(IIIFJsonPath)) instance = path.find(self.iiif_asset)[0].value IIIFJsonPath.append('type') - print (IIIFJsonPath) + #print (IIIFJsonPath) return ValidationError(message='Failed to find out which oneOf test matched your data. This is likely due to an issue with the type and it not being valid value at this level. SchemaPath: {}'.format(self.pathToJsonPath(error_path)), path=[], schema_path=error_path, schema=self.getSchemaPortion(error_path), instance=instance) @@ -213,6 +213,11 @@ def parse(self, error_path, schemaEl, iiif_asset, IIIFJsonPath, parent=None, jso if len(error_path) <= 0: return True + if isinstance(schemaEl, dict) and "$ref" in schemaEl: + #print ('Found ref, trying to resolve: {}'.format(schemaEl['$ref'])) + return self.parse(error_path, self.resolver.resolve(schemaEl['$ref'])[1], iiif_asset, IIIFJsonPath, parent, jsonPath) + + pathEl = error_path.pop(0) # Check current type to see if its a match if pathEl == 'type' and parent == 'properties': @@ -227,7 +232,7 @@ def parse(self, error_path, schemaEl, iiif_asset, IIIFJsonPath, parent=None, jso value.append(option['pattern']) else: value.append(option['const']) - print ('Using values: {}'.format(value)) + #print ('Using values: {}'.format(value)) if not self.isTypeMatch(jsonPath + '.type', iiif_asset, value, IIIFJsonPath): return False # Check child type to see if its a match @@ -294,7 +299,7 @@ def isTypeMatch(self, iiifPath, iiif_asset, schemaType, IIIFJsonPath): #print ('Found type {} and schemaType {}'.format(typeValue, schemaType)) if isinstance(schemaType, list): for typeOption in schemaType: - print ('Testing {} = {}'.format(typeOption, typeValue)) + #print ('Testing {} = {}'.format(typeOption, typeValue)) if re.match(typeOption, typeValue): return True return False diff --git a/schema/iiif_3_0.json b/schema/iiif_3_0.json index 05fca06..2f95e84 100644 --- a/schema/iiif_3_0.json +++ b/schema/iiif_3_0.json @@ -62,6 +62,23 @@ "duration": { "type": "number", "minimum": 0 + }, + "external": { + "type": "array", + "items": { + "allOf": [ + { "$ref": "#/types/class" }, + { + "type": "object", + "properties": { + "format": { "$ref": "#/types/format" }, + "profile": { + "type": "string" + } + } + } + ] + } } }, @@ -92,21 +109,7 @@ } }, "seeAlso": { - "type": "array", - "items": { - "allOf": [ - { "$ref": "#/types/class" }, - { - "type": "object", - "properties": { - "format": { "$ref": "#/types/format" }, - "profile": { - "type": "string" - } - } - } - ] - } + "$ref": "#/types/external" }, "partOf": { "type": "array", @@ -424,6 +427,7 @@ { "$ref": "#/types/class" }, { "type": "object", + "additionalProperties": false, "properties": { "@context": { "oneOf": [ @@ -441,6 +445,8 @@ } ] }, + "id": {}, + "label": {}, "type": { "type": "string", "pattern": "^Manifest" @@ -448,15 +454,14 @@ "metadata": { "$ref": "#/classes/metadata" }, "summary": { "$ref": "#/types/lngString" }, "requiredStatement": { "$ref": "#/types/keyValueString" }, + "rendering": { "$ref": "#/types/external" }, + "service": { "$ref": "#/classes/service" }, + "viewingDirection": { "$ref": "#/classes/viewingDirection" }, "rights": { "$ref": "#/classes/rights" }, - "height": { - "type": "integer" - }, - "width": { - "type": "integer" - }, - "duration": { - "$ref": "#/types/duration" + "start": {}, + "logo": { + "type": "array", + "items": { "$ref": "#/classes/resource" } }, "navDate": { "$ref": "#/classes/navDate" }, "provider": { "$ref": "#/classes/provider" }, diff --git a/schema/schemavalidator.py b/schema/schemavalidator.py index 1187efc..5e703ee 100755 --- a/schema/schemavalidator.py +++ b/schema/schemavalidator.py @@ -58,12 +58,15 @@ def validate(data, version, url): # if a oneOf possibility is of type Collection but we have passed a Manifest # then its safe to ignore the validation error. for err in errors: + #print (err) if errorParser.isValid(list(err.absolute_schema_path), list(err.absolute_path)): # if it is valid we want a good error message so diagnose which oneOf is # relevant for the error we've found. if err.absolute_schema_path[-1] == 'oneOf': err = errorParser.diagnoseWhichOneOf(list(err.absolute_schema_path), list(err.absolute_path)) relevantErrors.append(err) + #else: + # print ('Dismissing schema: {} path: {}'.format(err.absolute_schema_path, err.absolute_path)) i += 1 errors = relevantErrors errorCount = 1 diff --git a/tests/test_validator.py b/tests/test_validator.py index 86cafaf..143e9b0 100644 --- a/tests/test_validator.py +++ b/tests/test_validator.py @@ -247,10 +247,19 @@ def test_version3errors(self): response = self.helperRunValidation(v, filename) self.helperTestValidationErrors(filename, response, errorPaths) + filename = 'fixtures/3/old_format_label.json' + errorPaths = [ + '/label', + '/label/' + # '/' Validator doesn't pick up extra sequence when it comes across the label error first + ] + response = self.helperRunValidation(v, filename) + self.helperTestValidationErrors(filename, response, errorPaths) + def helperTestValidationErrors(self, filename, response, errorPaths): self.assertEqual(response['okay'], 0, 'Expected {} to fail validation but it past.'.format(filename)) - self.assertEqual(len(response['errorList']), len(errorPaths), 'Expected {} validation errors but found {} for file {}'.format(len(errorPaths), len(response['errorList']), filename)) + self.assertEqual(len(response['errorList']), len(errorPaths), 'Expected {} validation errors but found {} for file {}\n{}'.format(len(errorPaths), len(response['errorList']), filename, response['errorList'])) for error in response['errorList']: foundPath = False From 52b2c022c0ac1c45fa86a73fb35746de4fa196e3 Mon Sep 17 00:00:00 2001 From: Glen Robson Date: Fri, 27 Mar 2020 01:26:15 +0000 Subject: [PATCH 2/2] Pinning configparser to working version --- setup.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index b556122..f9445f8 100644 --- a/setup.py +++ b/setup.py @@ -54,7 +54,8 @@ def run(self): ':python_version>="3.0"': ["Pillow>=3.2.0"], ':python_version<"3.0"': [ "Pillow==6.2.2", - "zipp==1.2.0" + "zipp==1.2.0", + "configparser==4.0.2" ], }, test_suite="tests",