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

Improves validation performance of types defined in the spec #286

Merged
merged 1 commit into from
Sep 25, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,6 @@ internal class IonSchemaSystemImpl(
private val warnCallback: (() -> String) -> Unit
) : IonSchemaSystem {

private val schemaCores = mapOf(
IonSchemaVersion.v1_0 to SchemaCore(this, IonSchemaVersion.v1_0),
IonSchemaVersion.v2_0 to SchemaCore(this, IonSchemaVersion.v2_0)
)

internal fun getBuiltInTypesSchema(version: IonSchemaVersion) = schemaCores[version]!!

private val schemaContentCache = SchemaContentCache(this::loadSchemaContent)

// Set to be used to detect cycle in import dependencies
Expand Down Expand Up @@ -101,7 +94,7 @@ internal class IonSchemaSystemImpl(

private fun createSchema(referenceManager: DeferredReferenceManager, version: IonSchemaVersion, schemaId: String?, isl: List<IonValue>): SchemaInternal {
return when (version) {
IonSchemaVersion.v1_0 -> SchemaImpl_1_0(referenceManager, this, schemaCores[version]!!, isl.iterator(), schemaId)
IonSchemaVersion.v1_0 -> SchemaImpl_1_0(referenceManager, this, isl.iterator(), schemaId)
IonSchemaVersion.v2_0 -> SchemaImpl_2_0(referenceManager, this, isl, schemaId)
}
}
Expand Down
155 changes: 0 additions & 155 deletions ion-schema/src/main/kotlin/com/amazon/ionschema/internal/SchemaCore.kt

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import com.amazon.ionschema.internal.util.markReadOnly
internal class SchemaImpl_1_0 private constructor(
referenceManager: DeferredReferenceManager,
private val schemaSystem: IonSchemaSystemImpl,
private val schemaCore: SchemaCore,
schemaContent: Iterator<IonValue>,
override val schemaId: String?,
preloadedImports: Map<String, Import>,
Expand All @@ -52,10 +51,9 @@ internal class SchemaImpl_1_0 private constructor(
internal constructor(
referenceManager: DeferredReferenceManager,
schemaSystem: IonSchemaSystemImpl,
schemaCore: SchemaCore,
schemaContent: Iterator<IonValue>,
schemaId: String?
) : this(referenceManager, schemaSystem, schemaCore, schemaContent, schemaId, emptyMap(), mutableMapOf())
) : this(referenceManager, schemaSystem, schemaContent, schemaId, emptyMap(), mutableMapOf())

override val isl: IonDatagram

Expand Down Expand Up @@ -255,16 +253,16 @@ internal class SchemaImpl_1_0 private constructor(
typeMap[type.name] = type
}

override fun getType(name: String) = schemaCore.getType(name) ?: types[name]
override fun getType(name: String) = BuiltInTypes[name] ?: types[name]

override fun getInScopeType(name: String) = (schemaCore.getType(name) ?: types[name])
override fun getInScopeType(name: String) = BuiltInTypes[name] ?: types[name]

override fun getDeclaredType(name: String) = declaredTypes[name]

override fun getDeclaredTypes(): Iterator<TypeInternal> = declaredTypes.values.iterator()

override fun getTypes(): Iterator<TypeInternal> =
(schemaCore.getTypes().asSequence() + types.values.asSequence())
(BuiltInTypes.asSequence() + types.values.asSequence())
.filter { it is ImportedType || it is TypeImpl }
.iterator()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,7 @@ internal class SchemaImpl_2_0 internal constructor(
var foundFooter = false
var foundAnyType = false

importedTypes += schemaSystem.getBuiltInTypesSchema(ionSchemaLanguageVersion)
.getDeclaredTypes()
.asSequence()
.associateBy { it.name }
importedTypes += BuiltInTypes.asMap()

schemaContent.mapTo(isl) { it.clone() }
.markReadOnly()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,134 @@

package com.amazon.ionschema.internal

import com.amazon.ion.IonType
import com.amazon.ion.IonValue
import com.amazon.ion.system.IonSystemBuilder
import com.amazon.ionschema.Violation
import com.amazon.ionschema.Violations
import com.amazon.ionschema.internal.util.schemaTypeName

/**
* Marker interface for the non-scalar and non-container types defined
* in the Ion Schema Specification--types that are defined in [SchemaCore]
* by referring to other Core/Ion types.
* Represents a type that is defined in the Ion Schema Specification.
*
* Builtin types include 'lob', 'number', and 'any', but not 'document'.
* All implementations should be as lightweight as possible (e.g. no delegation to other [TypeInternal] instances)
* because these are the most commonly used types that are used as part of building almost every user-defined type.
*/
internal sealed class TypeBuiltin : TypeInternal

private val ionSystem = IonSystemBuilder.standard().build()

/**
* The `$any` type gets special treatment because it is a no-op.
*/
private object UniversalType : TypeBuiltin() {
override val schemaId: String?
get() = null
override fun getBaseType(): TypeBuiltin = this
override fun isValidForBaseType(value: IonValue): Boolean = true

Check warning on line 42 in ion-schema/src/main/kotlin/com/amazon/ionschema/internal/TypeBuiltin.kt

View check run for this annotation

Codecov / codecov/patch

ion-schema/src/main/kotlin/com/amazon/ionschema/internal/TypeBuiltin.kt#L40-L42

Added lines #L40 - L42 were not covered by tests
override val name: String = "\$any"
override val isl: IonValue = ionSystem.newSymbol("\$any")
override fun validate(value: IonValue, issues: Violations) = Unit

Check warning on line 45 in ion-schema/src/main/kotlin/com/amazon/ionschema/internal/TypeBuiltin.kt

View check run for this annotation

Codecov / codecov/patch

ion-schema/src/main/kotlin/com/amazon/ionschema/internal/TypeBuiltin.kt#L45

Added line #L45 was not covered by tests
}

/**
* The `any` type gets special treatment because it is the default type for ISL 1.0
*/
private object AnyType : TypeBuiltin() {
override val schemaId: String?
get() = null

Check warning on line 53 in ion-schema/src/main/kotlin/com/amazon/ionschema/internal/TypeBuiltin.kt

View check run for this annotation

Codecov / codecov/patch

ion-schema/src/main/kotlin/com/amazon/ionschema/internal/TypeBuiltin.kt#L53

Added line #L53 was not covered by tests
override fun getBaseType(): TypeBuiltin = this
override fun isValidForBaseType(value: IonValue): Boolean = true
override val name: String = "any"
override val isl: IonValue = ionSystem.newSymbol("any")
override fun validate(value: IonValue, issues: Violations) {
if (value.isNullValue) {
val typeName = value.type.schemaTypeName()
issues.add(Violation(isl, "type_mismatch", message = "expected type any, found null $typeName"))
}
}
}

/**
* The `nothing` type gets special treatment because all data is invalid.
*/
private object EmptyType : TypeBuiltin() {
override val schemaId: String?
get() = null

Check warning on line 71 in ion-schema/src/main/kotlin/com/amazon/ionschema/internal/TypeBuiltin.kt

View check run for this annotation

Codecov / codecov/patch

ion-schema/src/main/kotlin/com/amazon/ionschema/internal/TypeBuiltin.kt#L71

Added line #L71 was not covered by tests
override fun getBaseType(): TypeBuiltin = this
override fun isValidForBaseType(value: IonValue): Boolean = false
override val name: String = "nothing"
override val isl: IonValue = ionSystem.newSymbol("nothing")
override fun validate(value: IonValue, issues: Violations) {
val typeName = value.type.schemaTypeName()
issues.add(Violation(isl, "type_mismatch", message = "expected type nothing, found $typeName"))
}
}

/**
* An implementation of [TypeBuiltin] that is valid for a set of [IonType]s and optionally the corresponding typed nulls,
* as defined in the Ion Schema specification.
*/
internal interface TypeBuiltin : TypeInternal
private class SpecType(override val name: String, private val ionTypes: Set<IonType>, private val nullsAllowed: Boolean) : TypeBuiltin() {
override val schemaId: String?
get() = null

Check warning on line 88 in ion-schema/src/main/kotlin/com/amazon/ionschema/internal/TypeBuiltin.kt

View check run for this annotation

Codecov / codecov/patch

ion-schema/src/main/kotlin/com/amazon/ionschema/internal/TypeBuiltin.kt#L88

Added line #L88 was not covered by tests

override fun getBaseType(): TypeBuiltin = this

override fun isValidForBaseType(value: IonValue): Boolean = value.type in ionTypes

override val isl: IonValue = ionSystem.newSymbol(name)

override fun validate(value: IonValue, issues: Violations) {
val typeCheckFailed = value.type !in ionTypes
val nullCheckFailed = !nullsAllowed and value.isNullValue
if (typeCheckFailed || nullCheckFailed) {
val maybeNull = if (value.isNullValue) "null " else ""
val typeName = value.type.schemaTypeName()
issues.add(Violation(isl, "type_mismatch", message = "expected type $name, found $maybeNull$typeName"))
}
}
}

/**
* Object for holding all instances of [TypeBuiltin].
*/
internal object BuiltInTypes {
private val allTypes: MutableMap<String, TypeBuiltin> = mutableMapOf()

init {
// Micro DSL so that we can declaratively create the Spec types.
infix fun String.isNonNull(types: Set<IonType>) {
allTypes[this] = SpecType(this, types, nullsAllowed = false)
}
infix fun String.isMaybeNull(types: Set<IonType>) {
allTypes[this] = SpecType(this, types, nullsAllowed = true)
}

// Creates all the SpecTypes that correspond directly to _one_ IonType.
IonType.values().forEach {
val typeName = it.schemaTypeName()
if (it != IonType.NULL) typeName isNonNull setOf(it)
if (it != IonType.DATAGRAM) "$$typeName" isMaybeNull setOf(it)
}

"text" isNonNull setOf(IonType.STRING, IonType.SYMBOL)
"\$text" isMaybeNull setOf(IonType.STRING, IonType.SYMBOL)
"number" isNonNull setOf(IonType.INT, IonType.FLOAT, IonType.DECIMAL)
"\$number" isMaybeNull setOf(IonType.INT, IonType.FLOAT, IonType.DECIMAL)
"lob" isNonNull setOf(IonType.BLOB, IonType.CLOB)
"\$lob" isMaybeNull setOf(IonType.BLOB, IonType.CLOB)
allTypes["any"] = AnyType
allTypes["\$any"] = UniversalType
allTypes["nothing"] = EmptyType
}

/** Gets one of the spec types by name */
operator fun get(name: String): TypeBuiltin? = allTypes[name]

/** Gets all the spec types as a map */
fun asMap(): Map<String, TypeBuiltin> = allTypes

/** Gets all the spec types as a sequence */
fun asSequence() = allTypes.values.asSequence()
}
Loading
Loading