Skip to content

Commit

Permalink
fix(*): Reorder property type alternatives for improved coercion
Browse files Browse the repository at this point in the history
Previously we used an undocumented convention of ordering alternative types for a property (i.e `anyOf` rules) in order of increasing complexity (e.g. `string`, then `Date`). In Encoda we use Ajv to coerce data to the schema. Ajv attempts coercion in the order listed. This meant that if `string` was listed first, that no other coercion would be attempted.

This PR changes the order of types in various `anyOf` rules so that coercion is attempted in the "right" order.

There are associated changes to `Article.title` and `Article.description` to make coercion behave as expected.

There is an associated PR in Encoda.
  • Loading branch information
nokome committed Sep 17, 2020
1 parent 0c63c54 commit 0b15122
Show file tree
Hide file tree
Showing 12 changed files with 242 additions and 217 deletions.
332 changes: 166 additions & 166 deletions py/stencila/schema/types.py

Large diffs are not rendered by default.

36 changes: 18 additions & 18 deletions r/R/types.R
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ Cite <- function(
self[["target"]] <- check_property("Cite", "target", TRUE, missing(target), "character", target)
self[["citationMode"]] <- check_property("Cite", "citationMode", FALSE, missing(citationMode), Enum("normal", "suppressAuthor"), citationMode)
self[["content"]] <- check_property("Cite", "content", FALSE, missing(content), Array(InlineContent), content)
self[["pageEnd"]] <- check_property("Cite", "pageEnd", FALSE, missing(pageEnd), Union("character", "numeric"), pageEnd)
self[["pageStart"]] <- check_property("Cite", "pageStart", FALSE, missing(pageStart), Union("character", "numeric"), pageStart)
self[["pageEnd"]] <- check_property("Cite", "pageEnd", FALSE, missing(pageEnd), Union("numeric", "character"), pageEnd)
self[["pageStart"]] <- check_property("Cite", "pageStart", FALSE, missing(pageStart), Union("numeric", "character"), pageStart)
self[["pagination"]] <- check_property("Cite", "pagination", FALSE, missing(pagination), "character", pagination)
self[["prefix"]] <- check_property("Cite", "prefix", FALSE, missing(prefix), "character", prefix)
self[["suffix"]] <- check_property("Cite", "suffix", FALSE, missing(suffix), "character", suffix)
Expand Down Expand Up @@ -534,9 +534,9 @@ Thing <- function(
)
self$type <- as_scalar("Thing")
self[["alternateNames"]] <- check_property("Thing", "alternateNames", FALSE, missing(alternateNames), Array("character"), alternateNames)
self[["description"]] <- check_property("Thing", "description", FALSE, missing(description), Union("character", Array(Node)), description)
self[["identifiers"]] <- check_property("Thing", "identifiers", FALSE, missing(identifiers), Array(Union("character", PropertyValue)), identifiers)
self[["images"]] <- check_property("Thing", "images", FALSE, missing(images), Array(Union("character", ImageObject)), images)
self[["description"]] <- check_property("Thing", "description", FALSE, missing(description), Union(Array(BlockContent), Array(InlineContent), "character"), description)
self[["identifiers"]] <- check_property("Thing", "identifiers", FALSE, missing(identifiers), Array(Union(PropertyValue, "character")), identifiers)
self[["images"]] <- check_property("Thing", "images", FALSE, missing(images), Array(Union(ImageObject, "character")), images)
self[["name"]] <- check_property("Thing", "name", FALSE, missing(name), "character", name)
self[["url"]] <- check_property("Thing", "url", FALSE, missing(url), "character", url)
class(self) <- c(class(self), "Thing")
Expand Down Expand Up @@ -728,12 +728,12 @@ CreativeWork <- function(
self[["genre"]] <- check_property("CreativeWork", "genre", FALSE, missing(genre), Array("character"), genre)
self[["isPartOf"]] <- check_property("CreativeWork", "isPartOf", FALSE, missing(isPartOf), CreativeWorkTypes, isPartOf)
self[["keywords"]] <- check_property("CreativeWork", "keywords", FALSE, missing(keywords), Array("character"), keywords)
self[["licenses"]] <- check_property("CreativeWork", "licenses", FALSE, missing(licenses), Array(Union("character", CreativeWorkTypes)), licenses)
self[["licenses"]] <- check_property("CreativeWork", "licenses", FALSE, missing(licenses), Array(Union(CreativeWorkTypes, "character")), licenses)
self[["parts"]] <- check_property("CreativeWork", "parts", FALSE, missing(parts), Array(CreativeWorkTypes), parts)
self[["publisher"]] <- check_property("CreativeWork", "publisher", FALSE, missing(publisher), Union(Person, Organization), publisher)
self[["references"]] <- check_property("CreativeWork", "references", FALSE, missing(references), Array(Union("character", CreativeWorkTypes)), references)
self[["references"]] <- check_property("CreativeWork", "references", FALSE, missing(references), Array(Union(CreativeWorkTypes, "character")), references)
self[["text"]] <- check_property("CreativeWork", "text", FALSE, missing(text), "character", text)
self[["title"]] <- check_property("CreativeWork", "title", FALSE, missing(title), Union("character", Array(Node)), title)
self[["title"]] <- check_property("CreativeWork", "title", FALSE, missing(title), Union(Array(InlineContent), "character"), title)
self[["version"]] <- check_property("CreativeWork", "version", FALSE, missing(version), Union("character", "numeric"), version)
class(self) <- c(class(self), "CreativeWork")
self
Expand Down Expand Up @@ -843,8 +843,8 @@ Article <- function(
version = version
)
self$type <- as_scalar("Article")
self[["pageEnd"]] <- check_property("Article", "pageEnd", FALSE, missing(pageEnd), Union("character", "numeric"), pageEnd)
self[["pageStart"]] <- check_property("Article", "pageStart", FALSE, missing(pageStart), Union("character", "numeric"), pageStart)
self[["pageEnd"]] <- check_property("Article", "pageEnd", FALSE, missing(pageEnd), Union("numeric", "character"), pageEnd)
self[["pageStart"]] <- check_property("Article", "pageStart", FALSE, missing(pageStart), Union("numeric", "character"), pageStart)
self[["pagination"]] <- check_property("Article", "pagination", FALSE, missing(pagination), "character", pagination)
class(self) <- c(class(self), "Article")
self
Expand Down Expand Up @@ -2745,9 +2745,9 @@ PublicationIssue <- function(
version = version
)
self$type <- as_scalar("PublicationIssue")
self[["issueNumber"]] <- check_property("PublicationIssue", "issueNumber", FALSE, missing(issueNumber), Union("character", "numeric"), issueNumber)
self[["pageEnd"]] <- check_property("PublicationIssue", "pageEnd", FALSE, missing(pageEnd), Union("character", "numeric"), pageEnd)
self[["pageStart"]] <- check_property("PublicationIssue", "pageStart", FALSE, missing(pageStart), Union("character", "numeric"), pageStart)
self[["issueNumber"]] <- check_property("PublicationIssue", "issueNumber", FALSE, missing(issueNumber), Union("numeric", "character"), issueNumber)
self[["pageEnd"]] <- check_property("PublicationIssue", "pageEnd", FALSE, missing(pageEnd), Union("numeric", "character"), pageEnd)
self[["pageStart"]] <- check_property("PublicationIssue", "pageStart", FALSE, missing(pageStart), Union("numeric", "character"), pageStart)
self[["pagination"]] <- check_property("PublicationIssue", "pagination", FALSE, missing(pagination), "character", pagination)
class(self) <- c(class(self), "PublicationIssue")
self
Expand Down Expand Up @@ -2859,10 +2859,10 @@ PublicationVolume <- function(
version = version
)
self$type <- as_scalar("PublicationVolume")
self[["pageEnd"]] <- check_property("PublicationVolume", "pageEnd", FALSE, missing(pageEnd), Union("character", "numeric"), pageEnd)
self[["pageStart"]] <- check_property("PublicationVolume", "pageStart", FALSE, missing(pageStart), Union("character", "numeric"), pageStart)
self[["pageEnd"]] <- check_property("PublicationVolume", "pageEnd", FALSE, missing(pageEnd), Union("numeric", "character"), pageEnd)
self[["pageStart"]] <- check_property("PublicationVolume", "pageStart", FALSE, missing(pageStart), Union("numeric", "character"), pageStart)
self[["pagination"]] <- check_property("PublicationVolume", "pagination", FALSE, missing(pagination), "character", pagination)
self[["volumeNumber"]] <- check_property("PublicationVolume", "volumeNumber", FALSE, missing(volumeNumber), Union("character", "numeric"), volumeNumber)
self[["volumeNumber"]] <- check_property("PublicationVolume", "volumeNumber", FALSE, missing(volumeNumber), Union("numeric", "character"), volumeNumber)
class(self) <- c(class(self), "PublicationVolume")
self
}
Expand Down Expand Up @@ -3846,7 +3846,7 @@ GrantTypes <- Union(Grant, MonetaryGrant)
#' Union type for valid inline content.
#'
#' @export
InlineContent <- Union("NULL", "logical", "numeric", "character", CodeFragment, CodeExpression, Delete, Emphasis, ImageObject, Link, MathFragment, NontextualAnnotation, Quote, Strong, Subscript, Superscript, Cite, CiteGroup)
InlineContent <- Union(CodeFragment, CodeExpression, Delete, Emphasis, ImageObject, Link, MathFragment, NontextualAnnotation, Quote, Strong, Subscript, Superscript, Cite, CiteGroup, "numeric", "logical", "NULL", "character")


#' All type schemas that are derived from Mark
Expand All @@ -3870,7 +3870,7 @@ MediaObjectTypes <- Union(MediaObject, AudioObject, ImageObject, VideoObject)
#' Union type for all valid nodes.
#'
#' @export
Node <- Union("NULL", "logical", "numeric", "character", Array(Any()), "list", Entity)
Node <- Union(Entity, "numeric", "logical", "NULL", "character", Array(Any()), "list")


#' All type schemas that are derived from NumberValidator
Expand Down
4 changes: 2 additions & 2 deletions schema/Article.schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ properties:
'@id': schema:pageStart
description: The page on which the article starts; for example "135" or "xiii".
anyOf:
- type: string
- type: integer
- type: string
pageEnd:
'@id': schema:pageEnd
description: The page on which the article ends; for example "138" or "xvi".
anyOf:
- type: string
- type: integer
- type: string
pagination:
'@id': schema:pagination
description: |
Expand Down
4 changes: 2 additions & 2 deletions schema/Cite.schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ properties:
'@id': schema:pageStart
description: The page on which the work starts; for example "135" or "xiii".
anyOf:
- type: string
- type: integer
- type: string
pageEnd:
'@id': schema:pageEnd
description: The page on which the work ends; for example "138" or "xvi".
anyOf:
- type: string
- type: integer
- type: string
pagination:
'@id': schema:pagination
description: |
Expand Down
22 changes: 16 additions & 6 deletions schema/CreativeWork.schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,9 @@ properties:
type: array
items:
anyOf:
- $ref: CreativeWorkTypes
- type: string
format: uri
- $ref: CreativeWorkTypes
parts:
# The name "parts" seems more intuitive for users and developers than schema.orgs's "hasParts".
# We provide the latter as an alias.
Expand Down Expand Up @@ -161,8 +161,8 @@ properties:
type: array
items:
anyOf:
- type: string
- $ref: CreativeWorkTypes
- type: string
text:
'@id': schema:text
description: The textual content of this creative work.
Expand All @@ -174,14 +174,24 @@ properties:
description: The title of the creative work.
aliases:
- headline
$comment: |
Allows for the title to include inline content (e.g `Strong`, `Math`)
or a string. The title can not be block content e.g `Paragraph`.
The `minItems` restriction avoids a string being coerced into an array
with a single string item.
anyOf:
- type: string
- type: array
items:
$ref: Node
$ref: InlineContent
minItems: 2
- type: string
version:
'@id': schema:version
description: The version of the creative work.
$comment: |
In this case `string` is listed as an alternative before `number` to
avoid semantic version numbers e.g. `1.0` being parsed, and subsequently
encoded, as `1` thereby resulting in loss of information.
anyOf:
- type: string
- type: number
Expand All @@ -193,7 +203,7 @@ definitions:
# for `DateTime` as well.
anyOf:
- $ref: Date
- type: string
format: date
- type: string
format: date-time
- type: string
format: date
4 changes: 3 additions & 1 deletion schema/Date.schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ properties:
'@id': 'schema:value'
description: The date as an ISO 8601 string.
anyOf:
- type: string
format: date-time
- type: string
format: date
- type: string
format: date-time
pattern: \d{4}
required:
- value
12 changes: 7 additions & 5 deletions schema/InlineContent.schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,9 @@ $comment: |
Note that this definition currently does not include
`array` and `object` nodes (which are included in `Node`).
This seems incongruent, and may be changed in the future.
The order of these types is important because it determines the
order of attempted coercion (particularly important for primitive types).
anyOf:
- type: 'null'
- type: boolean
- type: integer
- type: number
- type: string
- $ref: CodeFragment
- $ref: CodeExpression
- $ref: Delete
Expand All @@ -25,3 +22,8 @@ anyOf:
- $ref: Superscript
- $ref: Cite
- $ref: CiteGroup
- type: integer
- type: number
- type: boolean
- type: 'null'
- type: string
11 changes: 7 additions & 4 deletions schema/Node.schema.yaml
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
title: Node
category: metadata
description: Union type for all valid nodes.
$comment: |
The order of these types is important because it determines the
order of attempted coercion (ie. parsing and reshaping to arrays)
anyOf:
- type: 'null'
- type: boolean
- type: number
- $ref: Entity
- type: integer
- type: number
- type: boolean
- type: 'null'
- type: string
- type: array
- type: object
- $ref: Entity
6 changes: 3 additions & 3 deletions schema/PublicationIssue.schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,20 @@ properties:
'@id': schema:issueNumber
description: Identifies the issue of publication; for example, "iii" or "2".
anyOf:
- type: string
- type: integer
- type: string
pageStart:
'@id': schema:pageStart
description: The page on which the issue starts; for example "135" or "xiii".
anyOf:
- type: string
- type: integer
- type: string
pageEnd:
'@id': schema:pageEnd
description: The page on which the issue ends; for example "138" or "xvi".
anyOf:
- type: string
- type: integer
- type: string
pagination:
'@id': schema:pagination
description: |
Expand Down
6 changes: 3 additions & 3 deletions schema/PublicationVolume.schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ properties:
'@id': schema:pageStart
description: The page on which the volume starts; for example "135" or "xiii".
anyOf:
- type: string
- type: integer
- type: string
pageEnd:
'@id': schema:pageEnd
description: The page on which the volume ends; for example "138" or "xvi".
anyOf:
- type: string
- type: integer
- type: string
pagination:
'@id': schema:pagination
description: |
Expand All @@ -30,5 +30,5 @@ properties:
description: |
Identifies the volume of publication or multi-part work; for example, "iii" or "2".
anyOf:
- type: string
- type: integer
- type: string
16 changes: 12 additions & 4 deletions schema/Thing.schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,19 @@ properties:
description:
'@id': schema:description
description: A description of the item.
$comment: |
Allows for the description to be an array of nodes (e.g. an array of inline content,
or a couple of paragraphs), or a string. The `minItems` restriction avoids a string
being coerced into an array with a single string item.
anyOf:
- type: string
- type: array
items:
$ref: Node
$ref: BlockContent
- type: array
items:
$ref: InlineContent
minItems: 2
- type: string
identifiers:
'@id': schema:identifier
description: Any kind of identifier for any kind of Thing.
Expand All @@ -33,17 +41,17 @@ properties:
type: array
items:
anyOf:
- type: string
- $ref: PropertyValue
- type: string
images:
'@id': schema:image
description: Images of the item.
type: array
items:
anyOf:
- $ref: ImageObject
- type: string
format: uri
- $ref: ImageObject
name:
'@id': schema:name
description: The name of the item.
Expand Down
6 changes: 3 additions & 3 deletions ts/bindings/__file_snapshots__/Person.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,16 @@ def __init__(
address: Optional[Union[str, "PostalAddress"]] = None,
affiliations: Optional[Array["Organization"]] = None,
alternateNames: Optional[Array[str]] = None,
description: Optional[Union[str, Array["Node"]]] = None,
description: Optional[Union[Array["BlockContent"], Array["InlineContent"], str]] = None,
emails: Optional[Array[str]] = None,
familyNames: Optional[Array[str]] = None,
funders: Optional[Array[Union["Organization", "Person"]]] = None,
givenNames: Optional[Array[str]] = None,
honorificPrefix: Optional[str] = None,
honorificSuffix: Optional[str] = None,
id: Optional[str] = None,
identifiers: Optional[Array[Union[str, "PropertyValue"]]] = None,
images: Optional[Array[Union[str, "ImageObject"]]] = None,
identifiers: Optional[Array[Union["PropertyValue", str]]] = None,
images: Optional[Array[Union["ImageObject", str]]] = None,
jobTitle: Optional[str] = None,
memberOf: Optional[Array["Organization"]] = None,
meta: Optional[Dict[str, Any]] = None,
Expand Down

0 comments on commit 0b15122

Please sign in to comment.