Skip to content
This repository has been archived by the owner on May 15, 2024. It is now read-only.

Backwards compatibility validator #3

Merged
merged 6 commits into from
May 25, 2021
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
6 changes: 6 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#
# https://help.github.com/articles/dealing-with-line-endings/
#
# These are explicitly windows files and should use crlf
*.bat text eol=crlf

29 changes: 29 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
name: Build

on: [push]

jobs:
test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
with:
fetch-depth: 0
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because the app logic relies on git history; fetch-depth: 0 means fetch the entire history, not just a single commit

- uses: actions/setup-java@v2
with:
distribution: 'adopt'
java-version: '11'
- run: ./gradlew check --console=plain

compatibility:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
with:
fetch-depth: 0
- uses: actions/setup-java@v2
with:
distribution: 'adopt'
java-version: '11'
- run: ./gradlew assemble --console=plain
- run: java -jar app/build/libs/app.jar
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Ignore Gradle project-specific cache directory
.gradle

# Ignore Gradle build output directory
build
30 changes: 29 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,33 @@
Store reference data as flat files with the following characteristics:

- each entry has a unique ID
- the unique ID is in the _first_ column
- entries are not deleted (instead, add a `start_date`/`end_date` column pair)
- data structure changes are backwards-compatible in the same file
- data structure changes are backwards-compatible in the same file:
- cannot remove existing columns
- cannot rename existing columns
- can add new columns

## Checking backwards compatibility

To build, test, assemble the project:
```
$ ./gradlew build
<snip>
BUILD SUCCESSFUL in 2s
10 actionable tasks: 10 executed
```

To run the tool:
```
$ java -jar app/build/libs/app.jar
🔍 Checking if register files are backwards compatible with 'main'...
OFFENCES.csv: ✅ pass
nomis-ethnicity.CSV: ✅ pass
nomis-gender.csv: ✅ pass
nomis-locations.csv: ✅ pass
nomis-suffix.csv: ✅ pass
nomis-titles.csv: ✅ pass
pcc-regions-for-probation-v0.csv: ✅ pass
probation-regions-v0.csv: ✅ pass
```
29 changes: 29 additions & 0 deletions app/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
plugins {
id("org.jetbrains.kotlin.jvm") version "1.4.31"
application
}

repositories {
mavenCentral()
}

dependencies {
implementation(platform("org.jetbrains.kotlin:kotlin-bom"))
implementation("org.jetbrains.kotlin:kotlin-stdlib-jdk8")
implementation("com.opencsv:opencsv:5.4")
testImplementation("org.jetbrains.kotlin:kotlin-test")
testImplementation("org.jetbrains.kotlin:kotlin-test-junit")
}

application {
mainClass.set("uk.gov.justice.hmpps.referencedata.AppKt")
}

tasks.withType<Jar> {
manifest.attributes["Main-Class"] = application.mainClass
duplicatesStrategy = DuplicatesStrategy.EXCLUDE

from(sourceSets.main.get().output)
dependsOn(configurations.runtimeClasspath)
from({ configurations.runtimeClasspath.get().filter { it.name.endsWith("jar") }.map { zipTree(it) } })
}
39 changes: 39 additions & 0 deletions app/src/main/kotlin/uk/gov/justice/hmpps/referencedata/App.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package uk.gov.justice.hmpps.referencedata

import java.io.FileReader
import java.io.StringReader
import kotlin.system.exitProcess

data class Result(val register: Register, val compatibility: Compatibility) {
fun hasErrors(): Boolean {
return compatibility.errors.isNotEmpty()
}
}

fun main(vararg args: String) {
val ref = args.firstOrNull() ?: "origin/main"
val registers = VersionedRegisters.fromPreviousCommit(ref)
val results = registers.map { register ->
val previousVersion = StringReader(register.content)
val currentVersion = FileReader(register.path)
Result(register, CheckCompatibility(previousVersion, currentVersion).check())
}

println("🔍 Checking if register files are backwards compatible with '$ref'...")
results.forEach(::printResult)

if (results.any(Result::hasErrors)) {
exitProcess(1)
}
}

fun printResult(r: Result) {
print("${r.register.path}:")
val errors = r.compatibility.errors
if (errors.isEmpty()) {
println(" ✅ pass")
} else {
println()
errors.forEach { println(" ❗ error: $it") }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package uk.gov.justice.hmpps.referencedata

import com.opencsv.CSVReader
import java.io.Reader

data class Compatibility(val errors: List<String>)

class CheckCompatibility(releasedVersion: Reader, currentVersion: Reader) {
private var vReleased: MutableList<List<String>>
private var vCurrent: MutableList<List<String>>
private var requiredHeaders: List<String>
private var newHeaders: List<String>

init {
vReleased = readAll(releasedVersion)
vCurrent = readAll(currentVersion)
requiredHeaders = vReleased.removeFirst()
newHeaders = vCurrent.removeFirst()
}

fun check(): Compatibility {
val errors = mutableListOf<String>()
errors.addAll(verifyHeaders())
errors.addAll(verifyContent())
return Compatibility(errors)
}

private fun verifyHeaders(): List<String> {
val errors = arrayListOf<String>()

val removedHeaders = requiredHeaders.subtract(newHeaders)
for (h in removedHeaders) {
errors.add("previously used column cannot be removed: $h")
}

return errors
}

private fun verifyContent(): List<String> {
val errors = arrayListOf<String>()

val releasedIds = vReleased.map { row -> row.first() }
val currentIds = vCurrent.map { row -> row.first() }

val removedIds = releasedIds.subtract(currentIds)
for (removedId in removedIds) {
errors.add("previously used row cannot be removed: row with ID `$removedId` is missing")
}

return errors
}

private fun readAll(r: Reader): MutableList<List<String>> {
val csvr = CSVReader(r)
val rows = mutableListOf<List<String>>()

var l = csvr.readNext()
while (l != null) {
rows.add(l.toList())
l = csvr.readNext()
}

return rows
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package uk.gov.justice.hmpps.referencedata

import java.io.File
import java.lang.RuntimeException
import java.util.concurrent.TimeUnit

data class Register(val path: String, val content: String)

class VersionedRegisters {
companion object {
fun fromPreviousCommit(shaOrRef: String): List<Register> {
val c = ProcessBuilder("git", "ls-tree", "-r", "--full-tree", "--name-only", shaOrRef)
val p = c.start()
p.waitFor(10, TimeUnit.SECONDS)
if (p.exitValue() != 0) {
throw RuntimeException("Command ${c.command()} failed with exit status ${p.exitValue()}")
}

val registries = mutableListOf<Register>()
for (path in p.inputStream.bufferedReader().lines()) {
if (!path.endsWith(".csv", true)) {
continue
}
if (path.contains("test/resources/")) {
continue
}
registries.add(Register(path, checkoutFile(shaOrRef, path)))
}

return registries
}

private fun checkoutFile(shaOrRef: String, path: String): String {
val tempfile = File.createTempFile("reference-data", "")
tempfile.deleteOnExit()

val p = ProcessBuilder("git", "show", "$shaOrRef:$path")
.redirectOutput(tempfile)
.start()
p.waitFor(10, TimeUnit.SECONDS)

return tempfile.readText()
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package uk.gov.justice.hmpps.referencedata

import java.io.FileReader
import java.io.Reader
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertTrue

class CheckCompatibilityTest {
@Test
fun addingNewColumnIsAllowed() {
val compatibility = checkCompatibility("/new_column_v1.csv", "/new_column_v2.csv")
assertTrue(compatibility.errors.isEmpty())
}

@Test
fun addingNewRowIsAllowed() {
val compatibility = checkCompatibility("/new_row_v1.csv", "/new_row_v2.csv")
assertTrue(compatibility.errors.isEmpty())
}

@Test
fun changingAContentFieldIsAllowed() {
val compatibility = checkCompatibility("/change_content_v1.csv", "/change_content_v2.csv")
assertTrue(compatibility.errors.isEmpty())
}

@Test
fun removingAColumnIsDisallowed() {
val compatibility = checkCompatibility("/rename_column_v1.csv", "/rename_column_v2.csv")
assertEquals(
listOf("previously used column cannot be removed: valid_untll"),
compatibility.errors
)
}

@Test
fun removingARowIsDisallowed() {
val compatibility = checkCompatibility("/remove_row_v1.csv", "/remove_row_v2.csv")
assertEquals(
listOf("previously used row cannot be removed: row with ID `A` is missing"),
compatibility.errors
)
}

@Test
fun changingAPrimaryKeyIsDisallowed() {
val compatibility = checkCompatibility("/change_primary_key_v1.csv", "/change_primary_key_v2.csv")
assertEquals(
listOf("previously used row cannot be removed: row with ID `A` is missing"),
compatibility.errors
)
}

private fun checkCompatibility(before: String, after: String): Compatibility =
CheckCompatibility(fixture(before), fixture(after)).check()

private fun fixture(filename: String): Reader = FileReader(javaClass.getResource(filename).file)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package uk.gov.justice.hmpps.referencedata

import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertTrue

class VersionedRegistersTest {
private val firstRepoCommitWithRegister: String = "b8478787640aebddc15d9b80dd3ea6cde6541b44"
private val commitWithTestFixtures: String = "2cd90c5ddcf897ab8ac3173ea2c4fd1aa490888b"

@Test
fun retrievesPreviousVersionsOfRegisters() {
val registers = VersionedRegisters.fromPreviousCommit(firstRepoCommitWithRegister)
assertTrue(registers.isNotEmpty())

val filenames = registers.map(Register::path)
assertEquals(listOf("probation-regions.csv"), filenames)

val content = registers.first().content.trim().split("\n")
assertEquals("id,name", content.first())
assertEquals("L,\"Greater Manchester\"", content.last())
}

@Test
fun doesNotRetrieveNonRegisterFiles() {
val registers = VersionedRegisters.fromPreviousCommit(firstRepoCommitWithRegister)
assertTrue(registers.isNotEmpty())

val filenames = registers.map(Register::path)
assertFalse(filenames.contains("README.md"))
}

@Test
fun doesNotRetrieveTestRegisters() {
val registers = VersionedRegisters.fromPreviousCommit(commitWithTestFixtures)
assertTrue(registers.isNotEmpty())

val filenames = registers.map(Register::path)
assertFalse(filenames.any { path -> path.contains("new_column_v1.csv") })
}
}
3 changes: 3 additions & 0 deletions app/src/test/resources/change_content_v1.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
id,name
A,"North East"
B,"North Wrst"
3 changes: 3 additions & 0 deletions app/src/test/resources/change_content_v2.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
id,name
A,"North East"
B,"North West"
3 changes: 3 additions & 0 deletions app/src/test/resources/change_primary_key_v1.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
id,name
A,"North East"
B,"North West"
3 changes: 3 additions & 0 deletions app/src/test/resources/change_primary_key_v2.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
id,name
X,"North East"
B,"North West"
3 changes: 3 additions & 0 deletions app/src/test/resources/new_column_v1.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
id,name
A,"North East"
B,"North West"
3 changes: 3 additions & 0 deletions app/src/test/resources/new_column_v2.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
id,name,valid_until
A,"North East",
B,"North West",2021-05-01
3 changes: 3 additions & 0 deletions app/src/test/resources/new_row_v1.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
id,name
A,"North East"
B,"North West"
4 changes: 4 additions & 0 deletions app/src/test/resources/new_row_v2.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
id,name
A,"North East"
B,"North West"
C,"Yorkshire and the Humber"
3 changes: 3 additions & 0 deletions app/src/test/resources/remove_row_v1.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
id,name
A,"North East"
B,"North West"
Loading