Skip to content

Commit c936b21

Browse files
committed
Replace '.get' call with map
This wasn't caught by tests because there were no column reference tests for 'In' rules.
1 parent 7a8896f commit c936b21

File tree

4 files changed

+32
-22
lines changed

4 files changed

+32
-22
lines changed

csv-validator-core/src/main/scala/uk/gov/nationalarchives/csv/validator/schema/Rule.scala

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,11 @@ abstract class Rule(name: String, val argProviders: ArgProvider*) extends Positi
5454
else fail(columnIndex, row, schema)
5555
}
5656

57+
def argProviderHelper(provider: ArgProvider, columnDefinition: ColumnDefinition, columnIndex: Int, row: Row, schema: Schema, cellValue: String): (Option[FilePathBase], FilePathBase) = {
58+
val ruleValue = provider.referenceValue(columnIndex, row, schema)
59+
60+
if (columnDefinition.directives.contains(IgnoreCase())) (ruleValue.map(_.toLowerCase), cellValue.toLowerCase) else (ruleValue, cellValue)
61+
}
5762

5863
def valid(cellValue: String, columnDefinition: ColumnDefinition, columnIndex: Int,
5964
row: Row, schema: Schema, mayBeLast: Option[Boolean] = None): Boolean =

csv-validator-core/src/main/scala/uk/gov/nationalarchives/csv/validator/schema/v1_0/Rule.scala

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -128,47 +128,36 @@ case class FileExistsRule(pathSubstitutions: List[(String,String)], enforceCaseS
128128

129129
case class InRule(inValue: ArgProvider) extends Rule("in", Seq(inValue): _*) {
130130
override def valid(cellValue: String, columnDefinition: ColumnDefinition, columnIndex: Int, row: Row, schema: Schema, mayBeLast: Option[Boolean] = None): Boolean = {
131-
val ruleValue = inValue.referenceValue(columnIndex, row, schema)
132-
133-
val (rv, cv) = if (columnDefinition.directives.contains(IgnoreCase())) (ruleValue.get.toLowerCase, cellValue.toLowerCase) else (ruleValue.get, cellValue)
134-
rv contains cv
131+
val (potentialRv, cv) = argProviderHelper(inValue, columnDefinition, columnIndex, row, schema, cellValue)
132+
potentialRv.exists(_.contains(cv))
135133
}
136134
}
137135

138136
case class IsRule(isValue: ArgProvider) extends Rule("is", Seq(isValue): _*) {
139137
override def valid(cellValue: String, columnDefinition: ColumnDefinition, columnIndex: Int, row: Row, schema: Schema, mayBeLast: Option[Boolean] = None): Boolean = {
140-
val ruleValue = isValue.referenceValue(columnIndex, row, schema)
141-
142-
val (rv, cv) = if (columnDefinition.directives.contains(IgnoreCase())) (ruleValue.get.toLowerCase, cellValue.toLowerCase) else (ruleValue.get, cellValue)
143-
cv == rv
138+
val (potentialRv, cv) = argProviderHelper(isValue, columnDefinition, columnIndex, row, schema, cellValue)
139+
potentialRv.contains(cv)
144140
}
145141
}
146142

147-
148143
case class NotRule(notValue: ArgProvider) extends Rule("not", Seq(notValue): _*) {
149144
override def valid(cellValue: String, columnDefinition: ColumnDefinition, columnIndex: Int, row: Row, schema: Schema,mayBeLast: Option[Boolean] = None): Boolean = {
150-
val ruleValue = notValue.referenceValue(columnIndex, row, schema)
151-
152-
val (rv, cv) = if (columnDefinition.directives.contains(IgnoreCase())) (ruleValue.get.toLowerCase, cellValue.toLowerCase) else (ruleValue.get, cellValue)
153-
cv != rv
145+
val (potentialRv, cv) = argProviderHelper(notValue, columnDefinition, columnIndex, row, schema, cellValue)
146+
!potentialRv.contains(cv)
154147
}
155148
}
156149

157150
case class StartsRule(startsValue: ArgProvider) extends Rule("starts", Seq(startsValue): _*) {
158151
override def valid(cellValue: String, columnDefinition: ColumnDefinition, columnIndex: Int, row: Row, schema: Schema, mayBeLast: Option[Boolean] = None): Boolean = {
159-
val ruleValue = startsValue.referenceValue(columnIndex, row, schema)
160-
161-
val (rv, cv) = if (columnDefinition.directives.contains(IgnoreCase())) (ruleValue.get.toLowerCase, cellValue.toLowerCase) else (ruleValue.get, cellValue)
162-
cv startsWith rv
152+
val (potentialRv, cv) = argProviderHelper(startsValue, columnDefinition, columnIndex, row, schema, cellValue)
153+
potentialRv.exists(rv => cv.startsWith(rv))
163154
}
164155
}
165156

166157
case class EndsRule(endsValue: ArgProvider) extends Rule("ends", Seq(endsValue): _*) {
167158
override def valid(cellValue: String, columnDefinition: ColumnDefinition, columnIndex: Int, row: Row, schema: Schema, mayBeLast: Option[Boolean] = None): Boolean = {
168-
val ruleValue = endsValue.referenceValue(columnIndex, row, schema)
169-
170-
val (rv, cv) = if (columnDefinition.directives.contains(IgnoreCase())) (ruleValue.get.toLowerCase, cellValue.toLowerCase) else (ruleValue.get, cellValue)
171-
cv endsWith rv
159+
val (potentialRv, cv) = argProviderHelper(endsValue, columnDefinition, columnIndex, row, schema, cellValue)
160+
potentialRv.exists(rv => cv.endsWith(rv))
172161
}
173162
}
174163

csv-validator-core/src/test/scala/uk/gov/nationalarchives/csv/validator/schema/v1_0/InRuleSpec.scala

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,20 @@ class InRuleSpec extends Specification {
4040
}
4141
}
4242

43+
"succeed if inRule's column reference does exist" in {
44+
val inRule = InRule(ColumnReference(NamedColumnIdentifier("column1")))
45+
46+
inRule.evaluate(0, Row(List(Cell("hello world")), 1), Schema(globalDirsOne, List(ColumnDefinition(NamedColumnIdentifier("column1"))))) mustEqual Validated.Valid(true)
47+
}
48+
49+
"fail if inRule's column reference doesn't exist" in {
50+
val inRule = InRule(ColumnReference(NamedColumnIdentifier("nonExistentColumn")))
51+
52+
inRule.evaluate(0, Row(List(Cell("hello world today")), 1), Schema(globalDirsOne, List(ColumnDefinition(NamedColumnIdentifier("column1"))))) must beLike {
53+
case Validated.Invalid(messages) => messages.head mustEqual """in($nonExistentColumn) fails for line: 1, column: column1, value: "hello world today""""
54+
}
55+
}
56+
4357
"succeed with @ignoreCase" in {
4458
val inRule = InRule(Literal(Some("hello world")))
4559
inRule.evaluate(0, Row(List(Cell("hello WORLD")), 1), Schema(globalDirsOne, List(ColumnDefinition(NamedColumnIdentifier("column1"), Nil, List(IgnoreCase()))))) mustEqual Validated.Valid(true)

csv-validator-core/src/test/scala/uk/gov/nationalarchives/csv/validator/schema/v1_0/OrRuleSpec.scala

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,9 @@ class OrRuleSpec extends Specification {
6767

6868
val orRule = OrRule(leftInRule, rightInRule)
6969

70-
orRule.evaluate(0, Row(List(Cell("UK")), 1), schema) must throwA[NoSuchElementException]
70+
orRule.evaluate(0, Row(List(Cell("UK")), 1), schema) must beLike {
71+
case Validated.Invalid(messages) => messages.head mustEqual """in($ConfigurableCountry) or in("France") fails for line: 1, column: Country, value: "UK""""
72+
}
7173
}
7274

7375
"succeed when 3 'or' rules valid for right rule" in {

0 commit comments

Comments
 (0)