From ac1dd467ee531f22cb1e53303ec5e451395b85c4 Mon Sep 17 00:00:00 2001 From: Oguzhan Unlu Date: Thu, 18 Jan 2018 18:26:43 +0300 Subject: [PATCH] igluctl: switch severity level to skip checks (close #232) --- .../ctl/Command.scala | 42 +++--- .../ctl/LintCommand.scala | 78 +++++++--- .../iglu/ctl/CommandSpec.scala | 6 +- .../iglu/ctl/GenerateCommandSpec.scala | 133 +++++++++--------- .../jsonschema/SanityLinter.scala | 102 ++++++++------ .../jsonschema/SanityLinterSpec.scala | 108 +++++++++++--- 6 files changed, 287 insertions(+), 182 deletions(-) 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..48dcbb34 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,10 @@ 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 provided linters, given comma separated\t\tDefault: None" ) } } 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 becf0348..06419426 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 @@ -31,14 +31,13 @@ import com.github.fge.jsonschema.core.report.{ ListProcessingReport, ProcessingM // Schema DDL import com.snowplowanalytics.iglu.schemaddl.jsonschema.{ Schema, SanityLinter } -import com.snowplowanalytics.iglu.schemaddl.jsonschema.SanityLinter.SeverityLevel import com.snowplowanalytics.iglu.schemaddl.jsonschema.json4s.Json4sToSchema._ // This library 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[SanityLinter.Linter]) extends Command.CtlCommand { import LintCommand._ /** @@ -51,29 +50,38 @@ 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 - val stubFile: File = new File(inputDir.getAbsolutePath) - val stubCommand = GenerateCommand(stubFile, stubFile) - val (_, schemas) = splitValidations(validatedJsons.map(_.extractSelfDescribingSchema)) - val (schemaVerWarnings, schemaVerErrors) = stubCommand.validateSchemaVersions(schemas) - val lintSchemaVer: Boolean = (schemaVerWarnings, schemaVerErrors) match { - case (Nil, Nil) => true - case (warnings, Nil) => - println(warnings.mkString("\n")) - true - case (Nil, errors) => - println(errors.mkString("\n")) - false - case _ => - // this case is here only to trick compiler, - // GenerateCommand.validateSchemaVersions doesn't return warnings together with errors - true - } + if (linters.contains(SanityLinter.lintSchemaVersions)) { + val stubFile: File = new File(inputDir.getAbsolutePath) + val stubCommand = GenerateCommand(stubFile, stubFile) + val (_, schemas) = splitValidations(validatedJsons.map(_.extractSelfDescribingSchema)) + val (schemaVerWarnings, schemaVerErrors) = stubCommand.validateSchemaVersions(schemas) + val lintSchemaVer: Boolean = (schemaVerWarnings, schemaVerErrors) match { + case (Nil, Nil) => true + case (warnings, Nil) => + println(warnings.mkString("\n")) + true + case (Nil, errors) => + println(errors.mkString("\n")) + false + case _ => + // this case is here only to trick compiler, + // GenerateCommand.validateSchemaVersions doesn't return warnings together with errors + true + } - if (!lintSchemaVer) { - sys.exit(1) + if (!lintSchemaVer) { + sys.exit(1) + } else { + val reports = jsons.map { file => + val report = file.map(check) + flattenReport(report) + } + reports.foldLeft(Total(0, 0, 0))((acc, cur) => acc.add(cur)).exit() + } } else { val reports = jsons.map { file => val report = file.map(check) @@ -81,6 +89,7 @@ case class LintCommand(inputDir: File, skipWarnings: Boolean, severityLevel: Sev } reports.foldLeft(Total(0, 0, 0))((acc, cur) => acc.add(cur)).exit() } + } /** @@ -93,7 +102,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 => SanityLinter.lint(schema, 0, linters) } val fullValidation = syntaxCheck |+| pathCheck |+| lintCheck.getOrElse("Doesn't contain JSON Schema".failureNel) @@ -228,4 +237,29 @@ object LintCommand { case _ => true } else true + + def validateSkippedLinters(skipChecks: String): Either[String, List[SanityLinter.Linter]] = { + val skippedLinters = skipChecks.split(",") + + val linterValidationErrors = for { + sl <- skippedLinters + linter = SanityLinter.allLinters.get(sl) match { + case Some(l) => l + case None => Nil + } + if linter == Nil + } yield s"Unknown linter $sl" + + if (linterValidationErrors.nonEmpty) { + Left(linterValidationErrors.mkString("\n")) + } else { + val lintersToUse = skippedLinters.foldLeft(Right(SanityLinter.allLinters.values.toList)) { (linters, cur) => + (linters, SanityLinter.allLinters.get(cur)) match { + case (Right(linters), Some(l)) => Right(linters.diff(List(l))) + case (Right(linters), None) => Right(linters) + } + } + 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..272a4630 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 // specs2 import org.specs2.Specification @@ -33,10 +33,10 @@ class CommandSpec extends Specification { def is = s2""" 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 = { 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 2d723fcd..edfccba3 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 @@ -43,29 +43,29 @@ class GenerateCommandSpec extends Specification { def is = s2""" """|CREATE SCHEMA IF NOT EXISTS atomic; | |CREATE TABLE IF NOT EXISTS atomic.com_amazon_aws_lambda_java_context_1 ( - | "schema_vendor" VARCHAR(128) ENCODE RUNLENGTH NOT NULL, - | "schema_name" VARCHAR(128) ENCODE RUNLENGTH NOT NULL, - | "schema_format" VARCHAR(128) ENCODE RUNLENGTH NOT NULL, - | "schema_version" VARCHAR(128) ENCODE RUNLENGTH NOT NULL, - | "root_id" CHAR(36) ENCODE RAW NOT NULL, - | "root_tstamp" TIMESTAMP ENCODE LZO NOT NULL, - | "ref_root" VARCHAR(255) ENCODE RUNLENGTH NOT NULL, - | "ref_tree" VARCHAR(1500) ENCODE RUNLENGTH NOT NULL, - | "ref_parent" VARCHAR(255) ENCODE RUNLENGTH NOT NULL, - | "aws_request_id" VARCHAR(4096) ENCODE LZO, - | "client_context.client.app_package_name" VARCHAR(4096) ENCODE LZO, - | "client_context.client.app_title" VARCHAR(4096) ENCODE LZO, - | "client_context.client.app_version_code" VARCHAR(4096) ENCODE LZO, - | "client_context.client.app_version_name" VARCHAR(4096) ENCODE LZO, - | "client_context.custom" VARCHAR(4096) ENCODE LZO, - | "client_context.environment" VARCHAR(4096) ENCODE LZO, - | "function_name" VARCHAR(4096) ENCODE LZO, - | "identity.identity_id" VARCHAR(4096) ENCODE LZO, - | "identity.identity_pool_id" VARCHAR(4096) ENCODE LZO, - | "log_group_name" VARCHAR(4096) ENCODE LZO, - | "log_stream_name" VARCHAR(4096) ENCODE LZO, - | "memory_limit_in_mb" BIGINT ENCODE LZO, - | "remaining_time_millis" BIGINT ENCODE LZO, + | "schema_vendor" VARCHAR(128) ENCODE ZSTD NOT NULL, + | "schema_name" VARCHAR(128) ENCODE ZSTD NOT NULL, + | "schema_format" VARCHAR(128) ENCODE ZSTD NOT NULL, + | "schema_version" VARCHAR(128) ENCODE ZSTD NOT NULL, + | "root_id" CHAR(36) ENCODE RAW NOT NULL, + | "root_tstamp" TIMESTAMP ENCODE ZSTD NOT NULL, + | "ref_root" VARCHAR(255) ENCODE ZSTD NOT NULL, + | "ref_tree" VARCHAR(1500) ENCODE ZSTD NOT NULL, + | "ref_parent" VARCHAR(255) ENCODE ZSTD NOT NULL, + | "aws_request_id" VARCHAR(4096) ENCODE ZSTD, + | "client_context.client.app_package_name" VARCHAR(4096) ENCODE ZSTD, + | "client_context.client.app_title" VARCHAR(4096) ENCODE ZSTD, + | "client_context.client.app_version_code" VARCHAR(4096) ENCODE ZSTD, + | "client_context.client.app_version_name" VARCHAR(4096) ENCODE ZSTD, + | "client_context.custom" VARCHAR(4096) ENCODE ZSTD, + | "client_context.environment" VARCHAR(4096) ENCODE ZSTD, + | "function_name" VARCHAR(4096) ENCODE ZSTD, + | "identity.identity_id" VARCHAR(4096) ENCODE ZSTD, + | "identity.identity_pool_id" VARCHAR(4096) ENCODE ZSTD, + | "log_group_name" VARCHAR(4096) ENCODE ZSTD, + | "log_stream_name" VARCHAR(4096) ENCODE ZSTD, + | "memory_limit_in_mb" BIGINT ENCODE ZSTD, + | "remaining_time_millis" BIGINT ENCODE ZSTD, | FOREIGN KEY (root_id) REFERENCES atomic.events(event_id) |) |DISTSTYLE KEY @@ -166,39 +166,32 @@ 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 = { val resultContent = """|CREATE TABLE IF NOT EXISTS java_context ( - | "aws_request_id" VARCHAR(128) ENCODE LZO, - | "client_context.client.app_package_name" VARCHAR(128) ENCODE LZO, - | "client_context.client.app_title" VARCHAR(128) ENCODE LZO, - | "client_context.client.app_version_code" VARCHAR(128) ENCODE LZO, - | "client_context.client.app_version_name" VARCHAR(128) ENCODE LZO, - | "client_context.custom" VARCHAR(128) ENCODE LZO, - | "client_context.environment" VARCHAR(128) ENCODE LZO, - | "function_name" VARCHAR(128) ENCODE LZO, - | "identity.identity_id" VARCHAR(128) ENCODE LZO, - | "identity.identity_pool_id" VARCHAR(128) ENCODE LZO, - | "log_group_name" VARCHAR(128) ENCODE LZO, - | "log_stream_name" VARCHAR(128) ENCODE LZO, - | "memory_limit_in_mb" BIGINT ENCODE LZO, - | "remaining_time_millis" BIGINT ENCODE LZO + | "aws_request_id" VARCHAR(128) ENCODE ZSTD, + | "client_context.client.app_package_name" VARCHAR(128) ENCODE ZSTD, + | "client_context.client.app_title" VARCHAR(128) ENCODE ZSTD, + | "client_context.client.app_version_code" VARCHAR(128) ENCODE ZSTD, + | "client_context.client.app_version_name" VARCHAR(128) ENCODE ZSTD, + | "client_context.custom" VARCHAR(128) ENCODE ZSTD, + | "client_context.environment" VARCHAR(128) ENCODE ZSTD, + | "function_name" VARCHAR(128) ENCODE ZSTD, + | "identity.identity_id" VARCHAR(128) ENCODE ZSTD, + | "identity.identity_pool_id" VARCHAR(128) ENCODE ZSTD, + | "log_group_name" VARCHAR(128) ENCODE ZSTD, + | "log_stream_name" VARCHAR(128) ENCODE ZSTD, + | "memory_limit_in_mb" BIGINT ENCODE ZSTD, + | "remaining_time_millis" BIGINT ENCODE ZSTD |); | |COMMENT ON TABLE java_context IS 'Source: java-context.json';""".stripMargin @@ -302,29 +295,29 @@ class GenerateCommandSpec extends Specification { def is = s2""" """|CREATE SCHEMA IF NOT EXISTS snowplow; | |CREATE TABLE IF NOT EXISTS snowplow.com_amazon_aws_ec2_instance_identity_document_1 ( - | "schema_vendor" VARCHAR(128) ENCODE RUNLENGTH NOT NULL, - | "schema_name" VARCHAR(128) ENCODE RUNLENGTH NOT NULL, - | "schema_format" VARCHAR(128) ENCODE RUNLENGTH NOT NULL, - | "schema_version" VARCHAR(128) ENCODE RUNLENGTH NOT NULL, - | "root_id" CHAR(36) ENCODE RAW NOT NULL, - | "root_tstamp" TIMESTAMP ENCODE LZO NOT NULL, - | "ref_root" VARCHAR(255) ENCODE RUNLENGTH NOT NULL, - | "ref_tree" VARCHAR(1500) ENCODE RUNLENGTH NOT NULL, - | "ref_parent" VARCHAR(255) ENCODE RUNLENGTH NOT NULL, - | "account_id" VARCHAR(4096) ENCODE LZO, - | "architecture" VARCHAR(4096) ENCODE LZO, - | "availability_zone" VARCHAR(4096) ENCODE LZO, - | "billing_products" VARCHAR(5000) ENCODE LZO, - | "devpay_product_codes" VARCHAR(5000) ENCODE LZO, - | "image_id" CHAR(12) ENCODE LZO, - | "instance_id" VARCHAR(19) ENCODE LZO, - | "instance_type" VARCHAR(4096) ENCODE LZO, - | "kernel_id" CHAR(12) ENCODE LZO, - | "pending_time" TIMESTAMP ENCODE LZO, - | "private_ip" VARCHAR(15) ENCODE LZO, - | "ramdisk_id" CHAR(12) ENCODE LZO, - | "region" VARCHAR(4096) ENCODE LZO, - | "version" VARCHAR(4096) ENCODE LZO, + | "schema_vendor" VARCHAR(128) ENCODE ZSTD NOT NULL, + | "schema_name" VARCHAR(128) ENCODE ZSTD NOT NULL, + | "schema_format" VARCHAR(128) ENCODE ZSTD NOT NULL, + | "schema_version" VARCHAR(128) ENCODE ZSTD NOT NULL, + | "root_id" CHAR(36) ENCODE RAW NOT NULL, + | "root_tstamp" TIMESTAMP ENCODE ZSTD NOT NULL, + | "ref_root" VARCHAR(255) ENCODE ZSTD NOT NULL, + | "ref_tree" VARCHAR(1500) ENCODE ZSTD NOT NULL, + | "ref_parent" VARCHAR(255) ENCODE ZSTD NOT NULL, + | "account_id" VARCHAR(4096) ENCODE ZSTD, + | "architecture" VARCHAR(4096) ENCODE ZSTD, + | "availability_zone" VARCHAR(4096) ENCODE ZSTD, + | "billing_products" VARCHAR(5000) ENCODE ZSTD, + | "devpay_product_codes" VARCHAR(5000) ENCODE ZSTD, + | "image_id" CHAR(12) ENCODE ZSTD, + | "instance_id" VARCHAR(19) ENCODE ZSTD, + | "instance_type" VARCHAR(4096) ENCODE ZSTD, + | "kernel_id" CHAR(12) ENCODE ZSTD, + | "pending_time" TIMESTAMP ENCODE ZSTD, + | "private_ip" VARCHAR(15) ENCODE ZSTD, + | "ramdisk_id" CHAR(12) ENCODE ZSTD, + | "region" VARCHAR(4096) ENCODE ZSTD, + | "version" VARCHAR(4096) ENCODE ZSTD, | FOREIGN KEY (root_id) REFERENCES snowplow.events(event_id) |) |DISTSTYLE KEY @@ -770,7 +763,7 @@ class GenerateCommandSpec extends Specification { def is = s2""" val resultContent = """|CREATE TABLE IF NOT EXISTS 1_0_0 ( - | "name" VARCHAR(4096) ENCODE LZO NOT NULL, + | "name" VARCHAR(4096) ENCODE ZSTD NOT NULL, | "age" DOUBLE PRECISION ENCODE RAW |); | 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..91fb98ef 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,22 +106,26 @@ 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 = { + // Current level validations - val validations = severityLevel.linters.map(linter => linter(schema)) + val validations = linters.map(linter => linter(schema)) .foldMap(_.toValidationNel) val rootTypeCheck = - if(severityLevel == SecondLevel || severityLevel == ThirdLevel) + if (linters.contains(lintRootType)) (height match { case 0 => (schema.`type`, schema.properties) match { @@ -140,30 +144,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,9 +175,23 @@ object SanityLinter { validations |+| rootTypeCheck |+| properties |+| items |+| additionalItems |+| additionalProperties |+| patternProperties } - // Linter functions + // Linters + + /** + * Placeholder linter to be understood through --skip-checks, + * SanityLinter.lint() contains its logic + */ + val lintRootType: Linter = (_: Schema) => { + propertySuccess + } - // First Severity Level + /** + * Placeholder Linter value to be understood through --skip-checks, + * igluctl.LintCommand.process() contains its logic + */ + val lintSchemaVersions: Linter = (_: Schema) => { + propertySuccess + } /** * Check that number's `minimum` property isn't greater than `maximum` @@ -292,8 +310,6 @@ object SanityLinter { } } - // Second Severity Level - /** * Check that schema with type `number` or `integer` contains both minimum * and maximum properties @@ -326,8 +342,6 @@ object SanityLinter { } } - // Third Severity Level - /** * Check that non-required properties have type null */ @@ -359,26 +373,22 @@ object SanityLinter { } } - 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( + "rootType" -> lintRootType, + "schemaVersions" -> lintSchemaVersions, + "minimumMaximum" -> lintMinimumMaximum, + "minMaxLength" -> lintMinMaxLength, + "maxLengthRange" -> lintMaxLengthRange, + "minMaxItems" -> lintMinMaxItems, + "numberProperties" -> lintNumberProperties, + "stringProperties" -> lintStringProperties, + "objectProperties" -> lintObjectProperties, + "arrayProperties" -> lintArrayProperties, + "possibleKeys" -> lintPossibleKeys, + "unknownFormats" -> lintUnknownFormats, + "minMaxPresent" -> lintMinMaxPresent, + "maxLength" -> lintMaxLength, + "optionalFields" -> lintOptionalFields, + "descriptionPresent" -> lintDescriptionPresent + ) } 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..050c30a0 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.allLinters 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 errors $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 schema doesn't contain description property $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"))) + SanityLinter.lint(schema, 0, allLinters.values.toList) must beEqualTo( + Failure(NonEmptyList( + "Properties [minLength] require string or absent type", + "Object Schema doesn't contain description property", + "Object Schema doesn't have 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]"))) + SanityLinter.lint(schema, 0, allLinters.values.toList) must beEqualTo( + Failure(NonEmptyList( + "Schema doesn't contain description property", + "Schema doesn't begin with type object", + "Schema doesn't contain description property", + "Schema doesn't contain description property", + "Schema doesn't contain description property", + "Object Schema doesn't contain description property", + "Schema doesn't contain description property", + "Schema doesn't contain description property", + "minimum property [5] is greater than maximum [0]" + )) + ) } 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"))) + SanityLinter.lint(schema, 0, allLinters.values.toList) must beEqualTo( + Failure(NonEmptyList( + "Properties [twoKey] is required, but not listed in properties", + "Schema doesn't contain description property", + "Schema doesn't begin with type object", + "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( + SanityLinter.lint(schema, 0, allLinters.values.toList) must beEqualTo( Failure(NonEmptyList( + "It is recommended to express absence of property via nullable type", + "Object Schema doesn't contain description property", + "String Schema doesn't contain description property", "String Schema doesn't contain maxLength nor enum properties nor appropriate format", "Numeric Schema doesn't contain minimum and maximum properties", + "Number Schema doesn't contain description property", + "String Schema doesn't contain description property", "String Schema doesn't contain maxLength nor enum properties nor appropriate format", + "String Schema doesn't contain description property", "String Schema doesn't contain maxLength nor enum properties nor appropriate format", + "String Schema doesn't contain description property", "String Schema doesn't contain maxLength nor enum properties nor appropriate format", - "Numeric Schema doesn't contain minimum and maximum properties" + "Numeric Schema doesn't contain minimum and maximum properties", + "Number Schema doesn't contain description property" )) ) } @@ -173,10 +207,24 @@ class SanityLinterSpec extends Specification { def is = s2""" """.stripMargin )).get - SanityLinter.lint(schema, SanityLinter.FirstLevel, 0) must beEqualTo( + SanityLinter.lint(schema, 0, allLinters.values.toList) must beEqualTo( Failure(NonEmptyList( + "It is recommended to express absence of property via nullable type", + "Object Schema doesn't contain description property", "Properties [maximum] require number, integer or absent type", - "Properties [minimum] require number, integer or absent type" + "String Schema doesn't contain description property", + "String Schema doesn't contain maxLength nor enum properties nor appropriate format", + "Numeric Schema doesn't contain minimum and maximum properties", + "Number Schema doesn't contain description property", + "String Schema doesn't contain description property", + "String Schema doesn't contain maxLength nor enum properties nor appropriate format", + "String Schema doesn't contain description property", + "String Schema doesn't contain maxLength nor enum properties nor appropriate format", + "Properties [minimum] require number, integer or absent type", + "String Schema doesn't contain description property", + "String Schema doesn't contain maxLength nor enum properties nor appropriate format", + "Numeric Schema doesn't contain minimum and maximum properties", + "Number Schema doesn't contain description property" )) ) } @@ -202,10 +250,14 @@ class SanityLinterSpec extends Specification { def is = s2""" """.stripMargin )).get - SanityLinter.lint(schema, SanityLinter.SecondLevel, 0) must beEqualTo( + SanityLinter.lint(schema, 0, allLinters.values.toList) must beEqualTo( Failure(NonEmptyList( + "Array Schema doesn't contain description property", "Schema doesn't begin with type object", - "String Schema doesn't contain maxLength nor enum properties nor appropriate format" + "Object Schema doesn't contain description property", + "String Schema doesn't contain description property", + "String Schema doesn't contain maxLength nor enum properties nor appropriate format", + "Schema doesn't contain description property" )) ) } @@ -228,12 +280,12 @@ class SanityLinterSpec extends Specification { def is = s2""" """.stripMargin )).get - SanityLinter.lint(schema, SanityLinter.ThirdLevel, 0) must beEqualTo( + SanityLinter.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", + "Object Schema doesn't contain description property", "String Schema doesn't contain description property", + "String Schema doesn't contain maxLength nor enum properties nor appropriate format", "Numeric Schema doesn't contain minimum and maximum properties", "Number Schema doesn't contain description property" )) @@ -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"))) + + SanityLinter.lint(schema, 0, allLinters.values.toList) must beEqualTo( + Failure(NonEmptyList( + "It is recommended to express absence of property via nullable type", + "Object Schema doesn't contain description property", + "String Schema doesn't contain description property", + "Format [camelCase] is not supported. Available options are: date-time, date, email, hostname, ipv4, ipv6, uri", + "String Schema doesn't contain maxLength nor enum properties nor appropriate format", + "Numeric Schema doesn't contain minimum and maximum properties", + "Number Schema doesn't contain description property" + )) + ) } def e9 = { @@ -272,7 +335,13 @@ 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"))) + SanityLinter.lint(schema, 0, allLinters.values.toList) must beEqualTo( + Failure(NonEmptyList( + "maxLength [65536] is greater than Redshift VARCHAR(max), 65535", + "String Schema doesn't contain description property", + "Schema doesn't begin with type object" + )) + ) } def e10 = { @@ -317,7 +386,7 @@ def e10 = { """.stripMargin )).get - SanityLinter.lint(schema, SanityLinter.ThirdLevel, 0) must beEqualTo( + SanityLinter.lint(schema, 0, allLinters.values.toList) must beEqualTo( Failure(NonEmptyList( "It is recommended to express absence of property via nullable type", "String Schema doesn't contain description property", @@ -326,6 +395,5 @@ def e10 = { "Number Schema doesn't contain description property" )) ) - } }