Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing issue that causes the 500 error #98

Merged
merged 2 commits into from
Mar 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
52 changes: 52 additions & 0 deletions fixtures/3/old_format_label.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
]
}
]
}
]
}
]
}
17 changes: 11 additions & 6 deletions schema/error_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)

Expand Down Expand Up @@ -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':
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
51 changes: 28 additions & 23 deletions schema/iiif_3_0.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
}
]
}
}
},

Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -424,6 +427,7 @@
{ "$ref": "#/types/class" },
{
"type": "object",
"additionalProperties": false,
"properties": {
"@context": {
"oneOf": [
Expand All @@ -441,22 +445,23 @@
}
]
},
"id": {},
"label": {},
"type": {
"type": "string",
"pattern": "^Manifest"
},
"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" },
Expand Down
3 changes: 3 additions & 0 deletions schema/schemavalidator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
11 changes: 10 additions & 1 deletion tests/test_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down