Skip to content

Commit

Permalink
igluctl: switch severity level to skip checks (close #232)
Browse files Browse the repository at this point in the history
  • Loading branch information
@oguzhanunlu
oguzhanunlu committed Jan 29, 2018
1 parent ba94844 commit 5f850e2
Show file tree
Hide file tree
Showing 6 changed files with 224 additions and 109 deletions.
58 changes: 37 additions & 21 deletions 0-common/igluctl/src/main/scala/com.snowplowanalytics.iglu/ctl/Command.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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,
Expand All @@ -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
}
Expand All @@ -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 =
Expand Down Expand Up @@ -138,27 +138,27 @@ object Command {
opt[File]("output")
action { (x, c) => c.copy(output = Some(x)) }
valueName "<path>"
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 "<name>"
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 "<owner>"
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 "<name>"
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 "<n>"
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) }
Expand Down Expand Up @@ -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))}
Expand All @@ -245,7 +245,7 @@ object Command {
opt[String]("region")
action { (x, c) => c.copy(region = Some(x))}
valueName "<name>"
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 {
Expand All @@ -270,10 +270,26 @@ 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 "<linters>"
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 rootType : Check that root of schema has `type` object or `properties`\n" +
"\t\t\t minimumMaximum : Check that number's `minimum` property isn't greater than `maximum`\n" +
"\t\t\t minMaxLength : Check that string's `minLength` property isn't greater than `maxLength`\n" +
"\t\t\t maxLengthRange : Check that string's `maxLength` property isn't greater than Redshift VARCHAR(max), 65535\n" +
"\t\t\t minMaxItems : Check that array's `minItems` property isn't greater than `maxItems`\n" +
"\t\t\t numberProperties : Check that Schema with non-numeric type doesn't contain numeric properties\n" +
"\t\t\t stringProperties : Check that Schema with non-string type doesn't contain string properties\n" +
"\t\t\t objectProperties : Check that Schema with non-object type doesn't contain object properties\n" +
"\t\t\t arrayProperties : Check that Schema with non-object type doesn't contain object properties\n" +
"\t\t\t possibleKeys : Check that all required keys listed in properties\n" +
"\t\t\t unknownFormats : Check that schema contains known formats\n" +
"\t\t\t minMaxPresent : Check that schema with type `number` or `integer` contains both minimum and maximum properties\n" +
"\t\t\t maxLength : Check that schema with type `string` contains `maxLength` property or has other possibility to extract length\n" +
"\t\t\t optionalFields : Check that non-required properties have type null\n" +
"\t\t\t descriptionPresent : Check that each field contains a description property"
)
}
}
Expand Down
33 changes: 29 additions & 4 deletions 0-common/igluctl/src/main/scala/com.snowplowanalytics.iglu/ctl/LintCommand.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@ 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
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._

/**
Expand All @@ -51,6 +51,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
Expand Down Expand Up @@ -82,7 +83,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)

Expand Down Expand Up @@ -217,4 +218,28 @@ 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).map(l => s"Unknown linter $l")

if (linterValidationErrors.nonEmpty) {
Left(linterValidationErrors.mkString("\n"))
} 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)
}
}
}
18 changes: 15 additions & 3 deletions 0-common/igluctl/src/test/scala/com/snowplowanalytics/iglu/ctl/CommandSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, lintOptionalFields, lintRootType }

// specs2
import org.specs2.Specification
Expand All @@ -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 = {
Expand All @@ -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 optionalFields,rootType".split(" "), Command())
.flatMap(_.toCommand)

val skippedChecks = List(lintOptionalFields, lintRootType)

lint must beSome(LintCommand(new File("."), false, allLinters.values.toList.diff(skippedChecks)))
}
}
11 changes: 2 additions & 9 deletions 0-common/igluctl/src/test/scala/com/snowplowanalytics/iglu/ctl/GenerateCommandSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
Loading

0 comments on commit 5f850e2

Please sign in to comment.