diff --git a/0-common/igluctl/src/main/scala/com.snowplowanalytics.iglu/ctl/Command.scala b/0-common/igluctl/src/main/scala/com.snowplowanalytics.iglu/ctl/Command.scala index 3d788aae..043558fc 100644 --- a/0-common/igluctl/src/main/scala/com.snowplowanalytics.iglu/ctl/Command.scala +++ b/0-common/igluctl/src/main/scala/com.snowplowanalytics.iglu/ctl/Command.scala @@ -22,12 +22,12 @@ import java.util.UUID // scopt import scopt.OptionParser -// Schema DDL -import com.snowplowanalytics.iglu.schemaddl.jsonschema.SanityLinter.{ SeverityLevel, FirstLevel, SecondLevel, ThirdLevel } - // This library import PushCommand._ +// Schema DDL +import com.snowplowanalytics.iglu.schemaddl.jsonschema.SanityLinter.{ Linter, allLinters } + /** * Common command container */ @@ -55,7 +55,7 @@ case class Command( // lint skipWarnings: Boolean = false, - severityLevel: SeverityLevel = FirstLevel, + linters: List[Linter] = allLinters.values.toList, // s3 bucket: Option[String] = None, @@ -73,7 +73,7 @@ case class Command( case Some("static s3cp") => Some(S3cpCommand(input.get, bucket.get, s3path, accessKeyId, secretAccessKey, profile, region)) case Some("lint") => - Some(LintCommand(input.get, skipWarnings, severityLevel)) + Some(LintCommand(input.get, skipWarnings, linters)) case _ => None } @@ -91,11 +91,11 @@ object Command { } } - implicit val severityLevelRead: scopt.Read[SeverityLevel] = scopt.Read.reads { - case "1" => FirstLevel - case "2" => SecondLevel - case "3" => ThirdLevel - case l => throw new IllegalArgumentException(s"Error: $l is invalid severity level") + implicit val lintersRead: scopt.Read[List[Linter]] = scopt.Read.reads { s => + LintCommand.validateSkippedLinters(s) match { + case Left(err) => throw new IllegalArgumentException(err) + case Right(linters) => linters + } } private def subcommand(sub: String)(unit: Unit, root: Command): Command = @@ -138,27 +138,27 @@ object Command { opt[File]("output") action { (x, c) => c.copy(output = Some(x)) } valueName "" - text "Directory to put generated data\t\tDefault: current dir", + text "Directory to put generated data\t\t\t\tDefault: current dir", opt[String]("dbschema") action { (x, c) => c.copy(schema = Some(x)) } valueName "" - text "Redshift schema name\t\t\t\tDefault: atomic", + text "Redshift schema name\t\t\t\t\t\tDefault: atomic", opt[String]("set-owner") action { (x, c) => c.copy(owner = Some(x)) } valueName "" - text "Redshift table owner\t\t\t\tDefault: None", + text "Redshift table owner\t\t\t\t\t\tDefault: None", opt[String]("db") action { (x, c) => c.copy(db = x) } valueName "" - text "DB to which we need to generate DDL\t\tDefault: redshift", + text "DB to which we need to generate DDL\t\t\t\tDefault: redshift", opt[Int]("varchar-size") action { (x, c) => c.copy(varcharSize = x) } valueName "" - text "Default size for varchar data type\t\tDefault: 4096", + text "Default size for varchar data type\t\t\t\tDefault: 4096", opt[Unit]("with-json-paths") action { (_, c) => c.copy(withJsonPaths = true) } @@ -225,7 +225,7 @@ object Command { opt[String]("s3path") action { (x, c) => c.copy(s3path = Some(x))} - text "Path in the bucket to upload Schemas\t\tDefault: bucket root", + text "Path in the bucket to upload Schemas\t\t\t\tDefault: bucket root", opt[String]("accessKeyId") optional() action { (x, c) => c.copy(accessKeyId = Some(x))} @@ -245,7 +245,7 @@ object Command { opt[String]("region") action { (x, c) => c.copy(region = Some(x))} valueName "" - text "AWS S3 region\t\t\t\tDefault: us-west-2\n", + text "AWS S3 region\t\t\t\t\t\tDefault: us-west-2\n", checkConfig { (c: Command) => (c.secretAccessKey, c.accessKeyId, c.profile) match { @@ -270,10 +270,17 @@ object Command { action { (_, c) => c.copy(skipWarnings = true) } text "Don't output messages with log level less than ERROR", - opt[SeverityLevel]("severityLevel") - action { (x, c) => c.copy(severityLevel = x) } - text "Severity level\t\t\t\tDefault: 1" - + opt[List[Linter]]("skip-checks") + action { (x, c) => c.copy(linters = x) } + valueName "" + text "Lint without specified linters, given comma separated\tDefault: None\n" + + "\t\t\t All linters and their explanations are below.\n" + + "\t\t\t rootObject : Check that root of schema has object type and contains properties\n" + + "\t\t\t unknownFormats : Check that schema doesn't contain unknown formats\n" + + "\t\t\t numericMinMax : Check that schema with numeric type contains both minimum and maximum properties\n" + + "\t\t\t stringLength : Check that schema with string type contains maxLength property or other ways to extract max length\n" + + "\t\t\t optionalNull : Check that non-required fields have null type\n" + + "\t\t\t description : Check that property contains description\n" ) } } diff --git a/0-common/igluctl/src/main/scala/com.snowplowanalytics.iglu/ctl/LintCommand.scala b/0-common/igluctl/src/main/scala/com.snowplowanalytics.iglu/ctl/LintCommand.scala index 28350290..6e5f48ca 100644 --- a/0-common/igluctl/src/main/scala/com.snowplowanalytics.iglu/ctl/LintCommand.scala +++ b/0-common/igluctl/src/main/scala/com.snowplowanalytics.iglu/ctl/LintCommand.scala @@ -30,8 +30,8 @@ import java.io.File import com.github.fge.jsonschema.core.report.{ ListProcessingReport, ProcessingMessage } // Schema DDL -import com.snowplowanalytics.iglu.schemaddl.jsonschema.{ Schema, SanityLinter } -import com.snowplowanalytics.iglu.schemaddl.jsonschema.SanityLinter.SeverityLevel +import com.snowplowanalytics.iglu.schemaddl.jsonschema.Schema +import com.snowplowanalytics.iglu.schemaddl.jsonschema.SanityLinter.{ allLinters, Linter, lint } import com.snowplowanalytics.iglu.schemaddl.jsonschema.json4s.Json4sToSchema._ // This library @@ -39,7 +39,7 @@ import GenerateCommand.{Result, Errors, VersionSuccess, Warnings} import FileUtils.{ getJsonFilesStream, JsonFile, filterJsonSchemas } import Utils.{ extractSchema, splitValidations } -case class LintCommand(inputDir: File, skipWarnings: Boolean, severityLevel: SeverityLevel) extends Command.CtlCommand { +case class LintCommand(inputDir: File, skipWarnings: Boolean, linters: List[Linter]) extends Command.CtlCommand { import LintCommand._ /** @@ -52,6 +52,7 @@ case class LintCommand(inputDir: File, skipWarnings: Boolean, severityLevel: Sev if (failures.nonEmpty) { println("JSON Parsing errors:") println(failures.mkString("\n")) + sys.exit(1) } // lint schema versions @@ -84,7 +85,7 @@ case class LintCommand(inputDir: File, skipWarnings: Boolean, severityLevel: Sev val pathCheck = extractSchema(jsonFile).map(_ => ()).validation.toValidationNel val syntaxCheck = validateSchema(jsonFile.content, skipWarnings) - val lintCheck = Schema.parse(jsonFile.content).map { schema => SanityLinter.lint(schema, severityLevel, 0) } + val lintCheck = Schema.parse(jsonFile.content).map { schema => lint(schema, 0, linters) } val fullValidation = syntaxCheck |+| pathCheck |+| lintCheck.getOrElse("Doesn't contain JSON Schema".failureNel) @@ -219,4 +220,33 @@ object LintCommand { case _ => true } else true + + /** + * Validates if user provided --skip-checks with a valid string + * @param skipChecks command line input for --skip-checks + * @return Either error concatenated error messages or valid list of linters + */ + def validateSkippedLinters(skipChecks: String): Either[String, List[Linter]] = { + val skippedLinters = skipChecks.split(",") + val linterValidationErrors = skippedLinters.filterNot(allLinters.isDefinedAt) + if (linterValidationErrors.nonEmpty) { + Left(s"Unknown linters [${linterValidationErrors.mkString(",")}] ") + } else { + val allowedToBeExcluded: List[String] = List("rootObject", "unknownFormats", "numericMinMax", + "stringLength", "optionalNull", "description") + val forbiddenExcludes = skippedLinters.diff(allowedToBeExcluded) + if (forbiddenExcludes.nonEmpty) { + Left(s"Linters [${forbiddenExcludes.mkString(",")}] can NOT be excluded.") + } else { + val lintersToUse = skippedLinters.foldLeft(allLinters.values.toList) { (linters, cur) => + (linters, allLinters.get(cur)) match { + case (l: List[Linter], Some(linter)) => l.diff(List(linter)) + case (l: List[Linter], None) => l + case (l: List[_], _) => throw new IllegalArgumentException(s"$l is NOT a list of linters") + } + } + Right(lintersToUse) + } + } + } } diff --git a/0-common/igluctl/src/test/scala/com/snowplowanalytics/iglu/ctl/CommandSpec.scala b/0-common/igluctl/src/test/scala/com/snowplowanalytics/iglu/ctl/CommandSpec.scala index 80640531..1d4546f0 100644 --- a/0-common/igluctl/src/test/scala/com/snowplowanalytics/iglu/ctl/CommandSpec.scala +++ b/0-common/igluctl/src/test/scala/com/snowplowanalytics/iglu/ctl/CommandSpec.scala @@ -17,7 +17,7 @@ import java.io.File import java.util.UUID // Schema DDL -import com.snowplowanalytics.iglu.schemaddl.jsonschema.SanityLinter._ +import com.snowplowanalytics.iglu.schemaddl.jsonschema.SanityLinter.{ allLinters, lintUnknownFormats, lintRootObject } // specs2 import org.specs2.Specification @@ -28,15 +28,16 @@ class CommandSpec extends Specification { def is = s2""" correctly extract lint command class $e1 correctly extract static push command class $e2 correctly extract static s3cp command class $e3 + correctly extract lint command class (--skip-checks) $e4 """ def e1 = { val lint = Command .cliParser - .parse("lint . --severityLevel 2".split(" "), Command()) + .parse("lint .".split(" "), Command()) .flatMap(_.toCommand) - lint must beSome(LintCommand(new File("."), false, SecondLevel)) + lint must beSome(LintCommand(new File("."), false, allLinters.values.toList)) } def e2 = { @@ -57,4 +58,15 @@ class CommandSpec extends Specification { def is = s2""" staticS3cp must beSome(S3cpCommand(new File(".."), "anton-enrichment-test", Some("schemas"), None, None, None, Some("us-east-1"))) } + + def e4 = { + val lint = Command + .cliParser + .parse("lint . --skip-checks unknownFormats,rootObject".split(" "), Command()) + .flatMap(_.toCommand) + + val skippedChecks = List(lintUnknownFormats, lintRootObject) + + lint must beSome(LintCommand(new File("."), false, allLinters.values.toList.diff(skippedChecks))) + } } diff --git a/0-common/igluctl/src/test/scala/com/snowplowanalytics/iglu/ctl/GenerateCommandSpec.scala b/0-common/igluctl/src/test/scala/com/snowplowanalytics/iglu/ctl/GenerateCommandSpec.scala index 60bfa102..e9311f07 100644 --- a/0-common/igluctl/src/test/scala/com/snowplowanalytics/iglu/ctl/GenerateCommandSpec.scala +++ b/0-common/igluctl/src/test/scala/com/snowplowanalytics/iglu/ctl/GenerateCommandSpec.scala @@ -166,20 +166,13 @@ class GenerateCommandSpec extends Specification { def is = s2""" val jsonFile = JsonFile(sourceSchema, new File("1-0-0")) val stubFile: File = new File(".") - val command = GenerateCommand(stubFile, stubFile) + val command = GenerateCommand(stubFile, stubFile, noHeader = true) val output = command.transformSelfDescribing(List(jsonFile)) val expected = GenerateCommand.DdlOutput(List(TextFile(new File("com.amazon.aws.lambda/java_context_1.sql"), resultContent))) - def dropHeader(o: DdlOutput): DdlOutput = { - val textFile = o.ddls.head.file - val shortDdl = o.ddls.head.content.split("\n").toList.drop(4).mkString("\n") - val shortTextFiles = List(TextFile(textFile, shortDdl)) - o.copy(ddls = shortTextFiles) - } - - dropHeader(output) must beEqualTo(expected) + output must beEqualTo(expected) } def e2 = { diff --git a/0-common/schema-ddl/src/main/scala/com.snowplowanalytics/iglu.schemaddl/jsonschema/SanityLinter.scala b/0-common/schema-ddl/src/main/scala/com.snowplowanalytics/iglu.schemaddl/jsonschema/SanityLinter.scala index eb3a4690..cb751c62 100644 --- a/0-common/schema-ddl/src/main/scala/com.snowplowanalytics/iglu.schemaddl/jsonschema/SanityLinter.scala +++ b/0-common/schema-ddl/src/main/scala/com.snowplowanalytics/iglu.schemaddl/jsonschema/SanityLinter.scala @@ -106,28 +106,34 @@ object SanityLinter { } /** - * Main working function, traversing JSON Schema - * It lints all properties on current level, then tries to extract all - * subschemas from properties like `items`, `additionalItems` etc and - * recursively lint them as well - * - * @param schema parsed JSON AST - * @return non-empty list of summed failures (all, including nested) or - * unit in case of success - */ - def lint(schema: Schema, severityLevel: SeverityLevel, height: Int): LintSchema = { + * + * Main working function, traversing JSON Schema + * It lints all properties on current level, then tries to extract all + * subschemas from properties like `items`, `additionalItems` etc and + * recursively lint them as well + * + * @param schema parsed JSON AST + * @param height depth of linting + * @param linters of linters to be used + * @return non-empty list of summed failures (all, including nested) or + * unit in case of success + */ + def lint(schema: Schema, height: Int, linters: List[Linter]): LintSchema = { + + val lintersToUse = if (linters.contains(lintRootObject)) linters.diff(List(lintRootObject)) else linters + // Current level validations - val validations = severityLevel.linters.map(linter => linter(schema)) + val validations = lintersToUse.map(linter => linter(schema)) .foldMap(_.toValidationNel) + // lintRootObject val rootTypeCheck = - if(severityLevel == SecondLevel || severityLevel == ThirdLevel) + if (linters.contains(lintRootObject)) (height match { case 0 => (schema.`type`, schema.properties) match { - case (Some(Object), None) => "Object Schema doesn't have properties".failure case (Some(Object), Some(Properties(_))) => propertySuccess - case (_, _) => "Schema doesn't begin with type object".failure + case (_, _) => "Root of schema should have type object and contain properties".failure } case _ => propertySuccess }).toValidationNel @@ -140,30 +146,30 @@ object SanityLinter { val properties = schema.properties match { case Some(props) => - props.value.values.foldLeft(schemaSuccess)((a, s) => a |+| lint(s, severityLevel, height+1)) + props.value.values.foldLeft(schemaSuccess)((a, s) => a |+| lint(s, height+1, linters)) case None => schemaSuccess } val patternProperties = schema.patternProperties match { case Some(PatternProperties(props)) => - props.values.foldLeft(schemaSuccess)((a, s) => a |+| lint(s, severityLevel, height+1)) + props.values.foldLeft(schemaSuccess)((a, s) => a |+| lint(s, height+1, linters)) case _ => schemaSuccess } val additionalProperties = schema.additionalProperties match { - case Some(AdditionalPropertiesSchema(s)) => lint(s, severityLevel, height+1) + case Some(AdditionalPropertiesSchema(s)) => lint(s, height+1, linters) case _ => schemaSuccess } val items = schema.items match { - case Some(ListItems(s)) => lint(s, severityLevel, height+1) + case Some(ListItems(s)) => lint(s, height+1, linters) case Some(TupleItems(i)) => - i.foldLeft(schemaSuccess)((a, s) => a |+| lint(s, severityLevel, height+1)) + i.foldLeft(schemaSuccess)((a, s) => a |+| lint(s, height+1, linters)) case None => schemaSuccess } val additionalItems = schema.additionalItems match { - case Some(AdditionalItemsSchema(s)) => lint(s, severityLevel, height+1) + case Some(AdditionalItemsSchema(s)) => lint(s, height+1, linters) case _ => schemaSuccess } @@ -171,41 +177,51 @@ object SanityLinter { validations |+| rootTypeCheck |+| properties |+| items |+| additionalItems |+| additionalProperties |+| patternProperties } - // Linter functions + // Linters - // First Severity Level + /** + * Placeholder linter to be understood through --skip-checks, + * SanityLinter.lint() contains its logic + * + * Check that root of schema has object type and contains properties + */ + val lintRootObject: Linter = (_: Schema) => { + throw new IllegalStateException("Illegal use of lintRootObject") + } /** - * Check that number's `minimum` property isn't greater than `maximum` + * Check that schema with numeric type has minimum property not greater than maximum */ - val lintMinimumMaximum: Linter = (schema: Schema) => { + val lintNumericMinimumMaximum: Linter = (schema: Schema) => { (schema.minimum, schema.maximum) match { case (Some(min), Some(max)) => (max.getAsDecimal >= min.getAsDecimal) - .or(s"minimum property [${min.getAsDecimal}] is greater than maximum [${max.getAsDecimal}]") + .or(s"Schema with numeric type has minimum property [${min.getAsDecimal}] greater than maximum [${max.getAsDecimal}]") case _ => propertySuccess } } /** - * Check that string's `minLength` property isn't greater than `maxLength` + * Check that schema with string type has minLength property not greater than maxLength */ - val lintMinMaxLength: Linter = (schema: Schema) => { + val lintStringMinMaxLength: Linter = (schema: Schema) => { (schema.minLength, schema.maxLength) match { case (Some(min), Some(max)) => - (max.value >= min.value).or(s"minLength property [${min.value}] is greater than maxLength [${max.value}]") + (max.value >= min.value) + .or(s"Schema with string type has minLength property [${min.value}] greater than maxLength [${max.value}]") case _ => propertySuccess } } /** - * Check that string's `maxLength` property isn't greater than Redshift VARCHAR(max), 65535 + * Check that schema with string type has maxLength property not greater than Redshift VARCHAR(max) 65535 * See http://docs.aws.amazon.com/redshift/latest/dg/r_Character_types.html */ - val lintMaxLengthRange: Linter = (schema: Schema) => { + val lintStringMaxLengthRange: Linter = (schema: Schema) => { if (schema.withType(String)) { schema.maxLength match { - case Some(max) if max.value > 65535 => s"maxLength [${max.value}] is greater than Redshift VARCHAR(max), 65535".failure + case Some(max) if max.value > 65535 => + s"Schema with string type has maxLength property [${max.value}] greater than Redshift VARCHAR(max) 65535".failure case _ => propertySuccess } } @@ -213,125 +229,120 @@ object SanityLinter { } /** - * Check that array's `minItems` property isn't greater than `maxItems` + * Check that schema with array type has minItems property not greater than maxItems */ - val lintMinMaxItems: Linter = (schema: Schema) => { + val lintArrayMinMaxItems: Linter = (schema: Schema) => { (schema.minItems, schema.maxItems) match { case (Some(min), Some(max)) => - (max.value >= min.value).or(s"minItems property [${min.value}] is greater than maxItems [${max.value}]") + (max.value >= min.value) + .or(s"Schema with array type has minItems property [${min.value}] greater than maxItems [${max.value}]") case _ => propertySuccess } } /** - * Check that Schema with non-numeric type doesn't contain numeric properties + * Check that schema with non-numeric type doesn't contain numeric properties */ - val lintNumberProperties: Linter = (schema: Schema) => { + val lintNumericProperties: Linter = (schema: Schema) => { val numberProperties = schema.allProperties.collect { case Some(p: NumberProperties.NumberProperty) => p } val fruitless = numberProperties.nonEmpty && (schema.withoutType(Number) && schema.withoutType(Integer)) - (!fruitless).or(s"Properties [${numberProperties.map(_.keyName).mkString(",")}] require number, integer or absent type") + (!fruitless).or(s"Numeric properties [${numberProperties.map(_.keyName).mkString(",")}] require number, integer or absent type") } /** - * Check that Schema with non-string type doesn't contain string properties + * Check that schema with non-string type doesn't contain string properties */ val lintStringProperties: Linter = (schema: Schema) => { val stringProperties = schema.allProperties.collect { case Some(p: StringProperties.StringProperty) => p } val fruitless = stringProperties.nonEmpty && schema.withoutType(String) - (!fruitless).or(s"Properties [${stringProperties.map(_.keyName).mkString(",")}] require string or absent type") + (!fruitless).or(s"String properties [${stringProperties.map(_.keyName).mkString(",")}] require string or absent type") } /** - * Check that Schema with non-object type doesn't contain object properties + * Check that schema with non-object type doesn't contain object properties */ val lintObjectProperties: Linter = (schema: Schema) => { val objectProperties = schema.allProperties.collect { case Some(p: ObjectProperties.ObjectProperty) => p } val fruitless = objectProperties.map(_.keyName).nonEmpty && schema.withoutType(Object) - (!fruitless).or(s"Properties [${objectProperties.map(_.keyName).mkString(",")}] require object or absent type") + (!fruitless).or(s"Object properties [${objectProperties.map(_.keyName).mkString(",")}] require object or absent type") } /** - * Check that Schema with non-object type doesn't contain object properties + * Check that schema with non-array type doesn't contain array properties */ val lintArrayProperties: Linter = (schema: Schema) => { val arrayProperties = schema.allProperties.collect { case Some(p: ArrayProperties.ArrayProperty) => p } val fruitless = arrayProperties.nonEmpty && schema.withoutType(Array) - (!fruitless).or(s"Properties [${arrayProperties.map(_.keyName).mkString(",")}] require array or absent type") + (!fruitless).or(s"Array properties [${arrayProperties.map(_.keyName).mkString(",")}] require array or absent type") } /** - * Check that all required keys listed in properties + * Check that all required properties exist in properties * @todo take `patternProperties` in account */ - val lintPossibleKeys: Linter = (schema: Schema) => { + val lintRequiredPropertiesExist: Linter = (schema: Schema) => { (schema.additionalProperties, schema.required, schema.properties, schema.patternProperties) match { case (Some(AdditionalPropertiesAllowed(false)), Some(Required(required)), Some(Properties(properties)), None) => val allowedKeys = properties.keySet val requiredKeys = required.toSet val diff = requiredKeys -- allowedKeys - diff.isEmpty.or(s"Properties [${diff.mkString(",")}] is required, but not listed in properties") + diff.isEmpty.or(s"Required properties [${diff.mkString(",")}] doesn't exist in properties") case _ => propertySuccess } } /** - * Check that schema contains known formats + * Check that schema doesn't contain unknown formats */ val lintUnknownFormats: Linter = (schema: Schema) => { schema.format match { - case Some(CustomFormat(format)) => s"Format [$format] is not supported. Available options are: date-time, date, email, hostname, ipv4, ipv6, uri".failure + case Some(CustomFormat(format)) => s"Unknown format [$format] detected. Known formats are: date-time, date, email, hostname, ipv4, ipv6, uri".failure case _ => propertySuccess } } - // Second Severity Level - /** - * Check that schema with type `number` or `integer` contains both minimum - * and maximum properties + * Check that schema with numeric type contains both minimum and maximum properties */ - val lintMinMaxPresent: Linter = (schema: Schema) => { + val lintNumericMinMax: Linter = (schema: Schema) => { if (schema.withType(Number) || schema.withType(Integer)) { (schema.minimum, schema.maximum) match { case (Some(_), Some(_)) => propertySuccess - case (None, Some(_)) => "Numeric Schema doesn't contain minimum property".failure - case (Some(_), None) => "Numeric Schema doesn't contain maximum property".failure - case _ => "Numeric Schema doesn't contain minimum and maximum properties".failure + case (None, Some(_)) => "Schema with numeric type doesn't contain minimum property".failure + case (Some(_), None) => "Schema with numeric type doesn't contain maximum property".failure + case _ => "Schema with numeric type doesn't contain minimum and maximum properties".failure } } else propertySuccess } /** - * Check that schema with type `string` contains `maxLength` property or has - * other possibility to extract length + * Check that schema with string type contains maxLength property or other ways to extract max length */ - val lintMaxLength: Linter = (schema: Schema) => { + val lintStringLength: Linter = (schema: Schema) => { if (schema.withType(String) && schema.enum.isEmpty && schema.maxLength.isEmpty) { if (schema.withFormat(Ipv4Format) || schema.withFormat(Ipv6Format) || schema.withFormat(DateTimeFormat)) { propertySuccess } else { - "String Schema doesn't contain maxLength nor enum properties nor appropriate format".failure + "Schema with string type doesn't contain maxLength property or other ways to extract max length".failure } } else { propertySuccess } } - // Third Severity Level - /** - * Check that non-required properties have type null + * Check that non-required fields have null type */ - val lintOptionalFields: Linter = (schema: Schema) => { + val lintOptionalNull: Linter = (schema: Schema) => { (schema.required, schema.properties) match { case (Some(Required(required)), Some(Properties(properties))) => val allowedKeys = properties.keySet @@ -341,44 +352,36 @@ object SanityLinter { key <- optionalKeys if !properties(key).withType(Null) } yield key - optKeysWithoutTypeNull.isEmpty.or("It is recommended to express absence of property via nullable type") + optKeysWithoutTypeNull.isEmpty.or("Optional field doesn't allow null type") case _ => propertySuccess } } /** - * Check that each 'field' contains a description property + * Check that property contains description */ - val lintDescriptionPresent: Linter = (schema: Schema) => { + val lintDescription: Linter = (schema: Schema) => { schema.description match { case Some(_) => propertySuccess - case None => schema.`type` match { - case Some(t) => s"$t Schema doesn't contain description property".failure - case None => s"Schema doesn't contain description property".failure - } + case None => s"Schema doesn't contain description property".failure } } - trait SeverityLevel { - def linters: List[Linter] - } - - case object FirstLevel extends SeverityLevel { - val linters = List( - // Check if some min cannot be greater than corresponding max - lintMinimumMaximum, lintMinMaxLength, lintMinMaxItems, - // Check if type of Schema corresponds with its validation properties - lintNumberProperties, lintStringProperties, lintObjectProperties, lintArrayProperties, - // Other checks - lintPossibleKeys, lintUnknownFormats, lintMaxLengthRange - ) - } - - case object SecondLevel extends SeverityLevel { - val linters = FirstLevel.linters ++ List(lintMinMaxPresent, lintMaxLength) - } - - case object ThirdLevel extends SeverityLevel { - val linters = FirstLevel.linters ++ SecondLevel.linters ++ List(lintDescriptionPresent, lintOptionalFields) - } + val allLinters: Map[String, Linter] = Map( + "rootObject" -> lintRootObject, + "numericMinimumMaximum" -> lintNumericMinimumMaximum, + "stringMinMaxLength" -> lintStringMinMaxLength, + "stringMaxLengthRange" -> lintStringMaxLengthRange, + "arrayMinMaxItems" -> lintArrayMinMaxItems, + "numericProperties" -> lintNumericProperties, + "stringProperties" -> lintStringProperties, + "objectProperties" -> lintObjectProperties, + "arrayProperties" -> lintArrayProperties, + "requiredPropertiesExist" -> lintRequiredPropertiesExist, + "unknownFormats" -> lintUnknownFormats, + "numericMinMax" -> lintNumericMinMax, + "stringLength" -> lintStringLength, + "optionalNull" -> lintOptionalNull, + "description" -> lintDescription + ) } diff --git a/0-common/schema-ddl/src/test/scala/com/snowplowanalytics/iglu/schemaddl/jsonschema/SanityLinterSpec.scala b/0-common/schema-ddl/src/test/scala/com/snowplowanalytics/iglu/schemaddl/jsonschema/SanityLinterSpec.scala index ec996c25..e37a5244 100644 --- a/0-common/schema-ddl/src/test/scala/com/snowplowanalytics/iglu/schemaddl/jsonschema/SanityLinterSpec.scala +++ b/0-common/schema-ddl/src/test/scala/com/snowplowanalytics/iglu/schemaddl/jsonschema/SanityLinterSpec.scala @@ -23,19 +23,20 @@ import org.specs2.Specification // This libary import json4s.Json4sToSchema._ +import SanityLinter._ class SanityLinterSpec extends Specification { def is = s2""" Check SanityLinter specification recognize minLength and object type incompatibility $e1 recognize minimum/maximum incompatibility inside deeply nested Schema (can be unwanted behavior) $e2 recognize impossibility to fulfill required property $e3 - recognize errors for second severity level $e4 + recognize schema doesn't contain description property $e4 recognize error in the middle of object $e5 - recognize root of schema has type non-object for second severity level $e6 - recognize non-required properties don't have type null for third severity level $e7 + recognize root of schema has type non-object $e6 + recognize non-required properties don't have type null $e7 recognize unknown formats $e8 recognize maxLength is greater than Redshift VARCHAR(max) $e9 - recognize schema doesn't contain description property for third severity level $e10 + recognize skipped checks (description) $e10 """ def e1 = { @@ -47,7 +48,13 @@ class SanityLinterSpec extends Specification { def is = s2""" |} """.stripMargin)).get - SanityLinter.lint(schema, SanityLinter.FirstLevel, 0) must beEqualTo(Failure(NonEmptyList("Properties [minLength] require string or absent type"))) + lint(schema, 0, allLinters.values.toList) must beEqualTo( + Failure(NonEmptyList( + "Schema doesn't contain description property", + "String properties [minLength] require string or absent type", + "Root of schema should have type object and contain properties" + )) + ) } def e2 = { @@ -78,7 +85,19 @@ class SanityLinterSpec extends Specification { def is = s2""" |} """.stripMargin)).get - SanityLinter.lint(schema, SanityLinter.FirstLevel, 0) must beEqualTo(Failure(NonEmptyList("minimum property [5] is greater than maximum [0]"))) + lint(schema, 0, allLinters.values.toList) must beEqualTo( + Failure(NonEmptyList( + "Schema doesn't contain description property", + "Root of schema should have type object and contain properties", + "Schema doesn't contain description property", + "Schema doesn't contain description property", + "Schema doesn't contain description property", + "Schema doesn't contain description property", + "Schema doesn't contain description property", + "Schema with numeric type has minimum property [5] greater than maximum [0]", + "Schema doesn't contain description property" + )) + ) } def e3 = { @@ -94,7 +113,14 @@ class SanityLinterSpec extends Specification { def is = s2""" """.stripMargin )).get - SanityLinter.lint(schema, SanityLinter.FirstLevel, 0) must beEqualTo(Failure(NonEmptyList("Properties [twoKey] is required, but not listed in properties"))) + lint(schema, 0, allLinters.values.toList) must beEqualTo( + Failure(NonEmptyList( + "Schema doesn't contain description property", + "Required properties [twoKey] doesn't exist in properties", + "Root of schema should have type object and contain properties", + "Schema doesn't contain description property" + )) + ) } def e4 = { @@ -128,14 +154,22 @@ class SanityLinterSpec extends Specification { def is = s2""" """.stripMargin )).get - SanityLinter.lint(schema, SanityLinter.SecondLevel, 0) must beEqualTo( + lint(schema, 0, allLinters.values.toList) must beEqualTo( Failure(NonEmptyList( - "String Schema doesn't contain maxLength nor enum properties nor appropriate format", - "Numeric Schema doesn't contain minimum and maximum properties", - "String Schema doesn't contain maxLength nor enum properties nor appropriate format", - "String Schema doesn't contain maxLength nor enum properties nor appropriate format", - "String Schema doesn't contain maxLength nor enum properties nor appropriate format", - "Numeric Schema doesn't contain minimum and maximum properties" + "Schema doesn't contain description property", + "Optional field doesn't allow null type", + "Schema doesn't contain description property", + "Schema with string type doesn't contain maxLength property or other ways to extract max length", + "Schema doesn't contain description property", + "Schema with numeric type doesn't contain minimum and maximum properties", + "Schema doesn't contain description property", + "Schema with string type doesn't contain maxLength property or other ways to extract max length", + "Schema doesn't contain description property", + "Schema with string type doesn't contain maxLength property or other ways to extract max length", + "Schema doesn't contain description property", + "Schema with string type doesn't contain maxLength property or other ways to extract max length", + "Schema doesn't contain description property", + "Schema with numeric type doesn't contain minimum and maximum properties" )) ) } @@ -173,10 +207,24 @@ class SanityLinterSpec extends Specification { def is = s2""" """.stripMargin )).get - SanityLinter.lint(schema, SanityLinter.FirstLevel, 0) must beEqualTo( + lint(schema, 0, allLinters.values.toList) must beEqualTo( Failure(NonEmptyList( - "Properties [maximum] require number, integer or absent type", - "Properties [minimum] require number, integer or absent type" + "Schema doesn't contain description property", + "Optional field doesn't allow null type", + "Schema doesn't contain description property", + "Schema with string type doesn't contain maxLength property or other ways to extract max length", + "Numeric properties [maximum] require number, integer or absent type", + "Schema doesn't contain description property", + "Schema with numeric type doesn't contain minimum and maximum properties", + "Schema doesn't contain description property", + "Schema with string type doesn't contain maxLength property or other ways to extract max length", + "Schema doesn't contain description property", + "Schema with string type doesn't contain maxLength property or other ways to extract max length", + "Schema doesn't contain description property", + "Schema with string type doesn't contain maxLength property or other ways to extract max length", + "Numeric properties [minimum] require number, integer or absent type", + "Schema doesn't contain description property", + "Schema with numeric type doesn't contain minimum and maximum properties" )) ) } @@ -202,10 +250,14 @@ class SanityLinterSpec extends Specification { def is = s2""" """.stripMargin )).get - SanityLinter.lint(schema, SanityLinter.SecondLevel, 0) must beEqualTo( + lint(schema, 0, allLinters.values.toList) must beEqualTo( Failure(NonEmptyList( - "Schema doesn't begin with type object", - "String Schema doesn't contain maxLength nor enum properties nor appropriate format" + "Schema doesn't contain description property", + "Root of schema should have type object and contain properties", + "Schema doesn't contain description property", + "Schema doesn't contain description property", + "Schema with string type doesn't contain maxLength property or other ways to extract max length", + "Schema doesn't contain description property" )) ) } @@ -228,14 +280,14 @@ class SanityLinterSpec extends Specification { def is = s2""" """.stripMargin )).get - SanityLinter.lint(schema, SanityLinter.ThirdLevel, 0) must beEqualTo( + lint(schema, 0, allLinters.values.toList) must beEqualTo( Failure(NonEmptyList( - "Object Schema doesn't contain description property", - "It is recommended to express absence of property via nullable type", - "String Schema doesn't contain maxLength nor enum properties nor appropriate format", - "String Schema doesn't contain description property", - "Numeric Schema doesn't contain minimum and maximum properties", - "Number Schema doesn't contain description property" + "Schema doesn't contain description property", + "Optional field doesn't allow null type", + "Schema doesn't contain description property", + "Schema with string type doesn't contain maxLength property or other ways to extract max length", + "Schema doesn't contain description property", + "Schema with numeric type doesn't contain minimum and maximum properties" )) ) } @@ -259,7 +311,18 @@ class SanityLinterSpec extends Specification { def is = s2""" """.stripMargin )).get - SanityLinter.lint(schema, SanityLinter.FirstLevel, 0) must beEqualTo(Failure(NonEmptyList("Format [camelCase] is not supported. Available options are: date-time, date, email, hostname, ipv4, ipv6, uri"))) + + lint(schema, 0, allLinters.values.toList) must beEqualTo( + Failure(NonEmptyList( + "Schema doesn't contain description property", + "Optional field doesn't allow null type", + "Schema doesn't contain description property", + "Schema with string type doesn't contain maxLength property or other ways to extract max length", + "Unknown format [camelCase] detected. Known formats are: date-time, date, email, hostname, ipv4, ipv6, uri", + "Schema doesn't contain description property", + "Schema with numeric type doesn't contain minimum and maximum properties" + )) + ) } def e9 = { @@ -272,10 +335,16 @@ class SanityLinterSpec extends Specification { def is = s2""" |} """.stripMargin)).get - SanityLinter.lint(schema, SanityLinter.FirstLevel, 0) must beEqualTo(Failure(NonEmptyList("maxLength [65536] is greater than Redshift VARCHAR(max), 65535"))) + lint(schema, 0, allLinters.values.toList) must beEqualTo( + Failure(NonEmptyList( + "Schema doesn't contain description property", + "Schema with string type has maxLength property [65536] greater than Redshift VARCHAR(max) 65535", + "Root of schema should have type object and contain properties" + )) + ) } -def e10 = { + def e10 = { val schema = Schema.parse(parse( """ |{ @@ -317,15 +386,12 @@ def e10 = { """.stripMargin )).get - SanityLinter.lint(schema, SanityLinter.ThirdLevel, 0) must beEqualTo( + val skippedLinters = List(lintDescription) + + lint(schema, 0, allLinters.values.toList.diff(skippedLinters)) must beEqualTo( Failure(NonEmptyList( - "It is recommended to express absence of property via nullable type", - "String Schema doesn't contain description property", - "String Schema doesn't contain description property", - "String Schema doesn't contain description property", - "Number Schema doesn't contain description property" + "Optional field doesn't allow null type" )) ) - } }