diff --git a/fixtures/3/rights_lang_issues.json b/fixtures/3/rights_lang_issues.json new file mode 100644 index 0000000..58bd69b --- /dev/null +++ b/fixtures/3/rights_lang_issues.json @@ -0,0 +1 @@ +{"@context":["http://www.w3.org/ns/anno.jsonld","http://iiif.io/api/presentation/3/context.json"],"id":"https://www.rpm-api.io/records/5e78f06706f6190017c039f9?_format=iiif","type":"Manifest","items":[{"id":"https://www.rpm-api.io/records/5e78f06706f6190017c039f9/canvas/0","type":"Canvas","items":[{"id":"https://www.rpm-api.io/records/5e78f06706f6190017c039f9/canvas/0/annotationpage/0","type":"AnnotationPage","items":[{"id":"https://www.rpm-api.io/records/5e78f06706f6190017c039f9/canvas/0/annotation/0","type":"Annotation","motivation":"painting","body":{"id":"https://iiif.rpm-api.io/iiif/2/d300bde0b6954976b3a9534160768aa6/full/max/0/default.jpg","type":"Image","format":"image/jpeg","label":{"@none":["Map of Brighthelmston"]},"service":[{"id":"https://iiif.rpm-api.io/iiif/2/d300bde0b6954976b3a9534160768aa6","type":"ImageService2","profile":"http://iiif.io/api/image/2/level2.json"}]},"target":"https://www.rpm-api.io/records/5e78f06706f6190017c039f9/canvas/0"}]}],"label":{"@none":["Map of Brighthelmston"]},"thumbnail":[{"id":"https://iiif.rpm-api.io/iiif/2/d300bde0b6954976b3a9534160768aa6/full/90,/0/default.jpg","type":"Image"}]}],"label":{"@none":["Map of Brighthelmston"]},"metadata":[{"label":{"@none":["Title"]},"value":{"@none":["Map of Brighthelmston"]}},{"label":{"@none":["Department"]},"value":{"@none":["Fine Art"]}},{"label":{"@none":["Category"]},"value":{"@none":["Coloured Lithograph"]}},{"label":{"@none":["Creator"]},"value":{"@none":["Yeakell; Gardner; Arthur Wallis"]}},{"label":{"@none":["Place Created"]},"value":{"@none":["Brighton"]}},{"label":{"@none":["Date Created"]},"value":{"@none":["1779"]}},{"label":{"@none":["Materials"]},"value":{"@none":["Paper"]}},{"label":{"@none":["Description"]},"value":{"@none":[" \tThis is a coloured version which has been mounted. It shows the Old Town of Brighton and is annotated and shows the gardens at the back of houses and the open spaces and market gardens in some detail. At the top and bottom of the map are two engraved illustrations one of which is of the town from the sea. The references mention all the main buildigs. "]}},{"label":{"@none":["Licence"]},"value":{"@none":["https://creativecommons.org/publicdomain/zero/1.0/"]}},{"label":{"@none":["Accession Id"]},"value":{"@none":["FATMP000096"]}}],"rights":"https://creativecommons.org/publicdomain/zero/1.0/"} \ No newline at end of file diff --git a/iiif-presentation-validator.py b/iiif-presentation-validator.py index 83989c0..79ee83c 100755 --- a/iiif-presentation-validator.py +++ b/iiif-presentation-validator.py @@ -9,6 +9,7 @@ from gzip import GzipFile from io import BytesIO from jsonschema.exceptions import ValidationError, SchemaError +import traceback try: # python3 @@ -73,14 +74,17 @@ def check_manifest(self, data, version, url=None, warnings=[]): 'received': data, 'okay': 0, 'error': str(e), - 'url': url + 'url': url, + 'warnings': [] } except Exception as e: + traceback.print_exc() infojson = { 'received': data, 'okay': 0, 'error': 'Presentation Validator bug: "{}". Please create a Validator Issue, including a link to the manifest.'.format(e), - 'url': url + 'url': url, + 'warnings': [] } else: diff --git a/schema/error_processor.py b/schema/error_processor.py index 29e259f..4bf4ddf 100755 --- a/schema/error_processor.py +++ b/schema/error_processor.py @@ -92,7 +92,7 @@ def diagnoseWhichOneOf(self, error_path, IIIFJsonPath): print(err) raise - # One of the oneOf possibilities is a reference to anouther part of the schema + # One of the oneOf possibilities is a reference to another part of the schema # this won't bother the validator but will complicate the diagnoise so replace # it with the actual schema json (and copy all references) if isinstance(possibility, dict) and "$ref" in possibility: @@ -116,12 +116,19 @@ def diagnoseWhichOneOf(self, error_path, IIIFJsonPath): # its not another oneOf error if addErrors: # if error is also a oneOf then diagnoise again - if err.absolute_schema_path[-1] == 'oneOf' and err.absolute_schema_path != error_path: + if err.absolute_schema_path[-1] == 'oneOf' and err.absolute_schema_path != error_path and 'rights' not in err.absolute_schema_path: error_path.append(oneOfIndex) # this is is related to one of the original oneOfs at index oneOfIndex 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 + result = (self.diagnoseWhichOneOf(error_path, IIIFJsonPath)) # so recursivly discovery real error + + if isinstance(result, ValidationError): + store_errs.append(result) + else: + store_errs.extend(result) + #print ('would add: {} by addErrors is {}'.format(err.message, addErrors)) - store_errs.append(err) + else: + store_errs.append(err) # if All errors are relevant to the current type add them to the list if addErrors: valid_errors += store_errs @@ -131,10 +138,12 @@ def diagnoseWhichOneOf(self, error_path, IIIFJsonPath): # this may hide errors as we are only selecting the first one but better to get one to work on and then can re-run # Also need to convert back the error paths to the full path error_path.reverse() - valid_errors[0].absolute_schema_path.extendleft(error_path) IIIFJsonPath.reverse() - valid_errors[0].absolute_path.extendleft(IIIFJsonPath) - return valid_errors[0] + for error in valid_errors: + error.absolute_schema_path.extendleft(error_path) + error.absolute_path.extendleft(IIIFJsonPath) + # print ('Err {}, path {}'.format(error.message, error.path)) + return valid_errors else: # Failed to find the source of the error so most likely its a problem with the type # and it didn't match any of the possible oneOf types @@ -171,10 +180,14 @@ def getSchemaPortion(self, schemaPath): """ schemaEl = self.schema for pathPart in schemaPath: - if isinstance(schemaEl[pathPart], dict) and "$ref" in schemaEl[pathPart]: - schemaEl = self.resolver.resolve(schemaEl[pathPart]['$ref'])[1] - else: - schemaEl = schemaEl[pathPart] + try: + if isinstance(schemaEl[pathPart], dict) and "$ref" in schemaEl[pathPart]: + schemaEl = self.resolver.resolve(schemaEl[pathPart]['$ref'])[1] + else: + schemaEl = schemaEl[pathPart] + except KeyError as error: + # print (schemaEl) + raise KeyError return schemaEl @@ -218,6 +231,7 @@ def parse(self, error_path, schemaEl, iiif_asset, IIIFJsonPath, parent=None, jso return self.parse(error_path, self.resolver.resolve(schemaEl['$ref'])[1], iiif_asset, IIIFJsonPath, parent, jsonPath) + #print ("Path: {} ".format(error_path)) pathEl = error_path.pop(0) # Check current type to see if its a match if pathEl == 'type' and parent == 'properties': @@ -242,7 +256,7 @@ def parse(self, error_path, schemaEl, iiif_asset, IIIFJsonPath, parent=None, jso return False # This is the case where additionalProperties has falied but need to check # if the type didn't match anyway - elif pathEl == 'additionalProperties' and 'type' in schemaEl['properties'] and 'pattern' in schemaEl['properties']['type']: + elif pathEl == 'additionalProperties' and 'properties' in schemaEl and 'type' in schemaEl['properties'] and 'pattern' in schemaEl['properties']['type']: value = schemaEl['properties']['type']['pattern'] if not self.isTypeMatch(jsonPath + '.type', iiif_asset, value, IIIFJsonPath): return False diff --git a/schema/schemavalidator.py b/schema/schemavalidator.py index 5e703ee..ddfe556 100755 --- a/schema/schemavalidator.py +++ b/schema/schemavalidator.py @@ -58,17 +58,26 @@ 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) + if isinstance(err, ValidationError): + relevantErrors.append(err) + else: + relevantErrors.extend(err) #else: # print ('Dismissing schema: {} path: {}'.format(err.absolute_schema_path, err.absolute_path)) i += 1 - errors = relevantErrors + # Remove dupes + seen_titles = set() + errors = [] + for errorDup in relevantErrors: + errorPath = errorParser.pathToJsonPath(errorDup.path) + if errorPath not in seen_titles: + errors.append(errorDup) + seen_titles.add(errorPath) errorCount = 1 # Now create some useful messsages to pass on for err in errors: diff --git a/tests/test_validator.py b/tests/test_validator.py index 76192a6..b9ba60c 100644 --- a/tests/test_validator.py +++ b/tests/test_validator.py @@ -251,12 +251,43 @@ def test_version3errors(self): 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 test_lang_rights(self): + v = val_mod.Validator() + + filename = 'fixtures/3/rights_lang_issues.json' + errorPaths = [ + '/label', + '/items[0]/label[0]', + '/items[0]/', + '/rights', + '/metadata[0]/label/', + '/metadata[0]/value/', + '/metadata[1]/label/', + '/metadata[1]/value/', + '/metadata[2]/label/', + '/metadata[2]/value/', + '/metadata[3]/label/', + '/metadata[3]/value/', + '/metadata[4]/label/', + '/metadata[4]/value/', + '/metadata[5]/label/', + '/metadata[5]/value/', + '/metadata[6]/label/', + '/metadata[6]/value/', + '/metadata[7]/label/', + '/metadata[7]/value/', + '/metadata[8]/label/', + '/metadata[8]/value/', + '/metadata[9]/label/', + '/metadata[9]/value/' + ] + 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))