From 3f8e1a1951dd43971b3a45d30332b4906aeb753a Mon Sep 17 00:00:00 2001 From: Graham Date: Sat, 29 Aug 2020 22:11:47 +0100 Subject: [PATCH] Add FlatFileStoreTest This commit fixes some bugs in FlatFileStore at the same time. Signed-off-by: Graham --- .../java/dev/openrs2/cache/FlatFileStore.kt | 23 +- .../dev/openrs2/cache/FlatFileStoreTest.kt | 276 ++++++++++++++++++ .../empty-archive/255/.gitempty | 0 .../cache/flat-file-store/empty/.gitempty | 0 .../flat-file-store/multiple-groups/0/0.dat | 0 .../flat-file-store/multiple-groups/0/1.dat | 0 .../flat-file-store/multiple-groups/255/0.dat | 1 + .../multiple-groups/255/65536.dat | 1 + .../single-group-overwritten/255/0.dat | 1 + .../flat-file-store/single-group/255/0.dat | 1 + .../dev/openrs2/util/io/PathExtensions.kt | 7 +- 11 files changed, 302 insertions(+), 8 deletions(-) create mode 100644 cache/src/test/java/dev/openrs2/cache/FlatFileStoreTest.kt create mode 100644 cache/src/test/resources/dev/openrs2/cache/flat-file-store/empty-archive/255/.gitempty create mode 100644 cache/src/test/resources/dev/openrs2/cache/flat-file-store/empty/.gitempty create mode 100644 cache/src/test/resources/dev/openrs2/cache/flat-file-store/multiple-groups/0/0.dat create mode 100644 cache/src/test/resources/dev/openrs2/cache/flat-file-store/multiple-groups/0/1.dat create mode 100644 cache/src/test/resources/dev/openrs2/cache/flat-file-store/multiple-groups/255/0.dat create mode 100644 cache/src/test/resources/dev/openrs2/cache/flat-file-store/multiple-groups/255/65536.dat create mode 100644 cache/src/test/resources/dev/openrs2/cache/flat-file-store/single-group-overwritten/255/0.dat create mode 100644 cache/src/test/resources/dev/openrs2/cache/flat-file-store/single-group/255/0.dat diff --git a/cache/src/main/java/dev/openrs2/cache/FlatFileStore.kt b/cache/src/main/java/dev/openrs2/cache/FlatFileStore.kt index 0b7c81cc..4dd5afd1 100644 --- a/cache/src/main/java/dev/openrs2/cache/FlatFileStore.kt +++ b/cache/src/main/java/dev/openrs2/cache/FlatFileStore.kt @@ -48,7 +48,12 @@ public class FlatFileStore private constructor( } override fun list(archive: Int): List { - Files.newDirectoryStream(archivePath(archive)).use { stream -> + val path = archivePath(archive) + if (!Files.isDirectory(path)) { + throw FileNotFoundException() + } + + Files.newDirectoryStream(path).use { stream -> return stream.filter { Files.isRegularFile(it) && GROUP_NAME.matches(it.fileName.toString()) } .map { Integer.parseInt(it.fileName.toString().removeSuffix(GROUP_EXTENSION)) } .sorted() @@ -57,11 +62,16 @@ public class FlatFileStore private constructor( } override fun create(archive: Int) { - Files.createDirectory(archivePath(archive)) + Files.createDirectories(archivePath(archive)) } override fun read(archive: Int, group: Int): ByteBuf { - FileChannel.open(groupPath(archive, group)).use { channel -> + val path = groupPath(archive, group) + if (!Files.isRegularFile(path)) { + throw FileNotFoundException() + } + + FileChannel.open(path).use { channel -> val size = channel.size() if (size > Store.MAX_GROUP_SIZE) { throw StoreCorruptException("Group too large") @@ -87,6 +97,9 @@ public class FlatFileStore private constructor( override fun remove(archive: Int) { val path = archivePath(archive) + if (!Files.exists(path)) { + return + } Files.newDirectoryStream(path).use { stream -> stream.filter { Files.isRegularFile(it) && GROUP_NAME.matches(it.fileName.toString()) } @@ -109,8 +122,8 @@ public class FlatFileStore private constructor( } public companion object { - private val ARCHIVE_NAME = Regex("[1-9][0-9]*") - private val GROUP_NAME = Regex("[1-9][0-9]*[.]dat") + private val ARCHIVE_NAME = Regex("[0-9]+") + private val GROUP_NAME = Regex("[0-9]+[.]dat") private const val GROUP_EXTENSION = ".dat" public fun open(root: Path, alloc: ByteBufAllocator = ByteBufAllocator.DEFAULT): Store { diff --git a/cache/src/test/java/dev/openrs2/cache/FlatFileStoreTest.kt b/cache/src/test/java/dev/openrs2/cache/FlatFileStoreTest.kt new file mode 100644 index 00000000..9c12520a --- /dev/null +++ b/cache/src/test/java/dev/openrs2/cache/FlatFileStoreTest.kt @@ -0,0 +1,276 @@ +package dev.openrs2.cache + +import com.google.common.jimfs.Configuration +import com.google.common.jimfs.Jimfs +import dev.openrs2.buffer.use +import dev.openrs2.util.io.recursiveCopy +import dev.openrs2.util.io.recursiveEquals +import io.netty.buffer.Unpooled +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import java.io.FileNotFoundException +import java.nio.file.Path +import java.nio.file.Paths +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +object FlatFileStoreTest { + private val IGNORE_GIT_EMPTY = { path: Path -> path.fileName.toString() != ".gitempty" } + private val root = Paths.get(FlatFileStoreTest::class.java.getResource("flat-file-store").toURI()) + + @Test + fun testBounds() { + readTest("empty") { store -> + assertThrows { + store.exists(-1) + } + + assertThrows { + store.exists(256) + } + + assertThrows { + store.list(-1) + } + + assertThrows { + store.list(256) + } + + assertThrows { + store.exists(-1, 0) + } + + assertThrows { + store.exists(256, 0) + } + + assertThrows { + store.exists(0, -1) + } + + assertThrows { + store.read(-1, 0).release() + } + + assertThrows { + store.read(256, 0).release() + } + + assertThrows { + store.read(0, -1).release() + } + } + + writeTest("empty") { store -> + assertThrows { + store.create(-1) + } + + assertThrows { + store.create(256) + } + + assertThrows { + store.write(-1, 0, Unpooled.EMPTY_BUFFER) + } + + assertThrows { + store.write(256, 0, Unpooled.EMPTY_BUFFER) + } + + assertThrows { + store.write(0, -1, Unpooled.EMPTY_BUFFER) + } + + assertThrows { + store.remove(-1) + } + + assertThrows { + store.remove(256) + } + + assertThrows { + store.remove(-1, 0) + } + + assertThrows { + store.remove(256, 0) + } + + assertThrows { + store.remove(0, -1) + } + } + } + + @Test + fun testExists() { + readTest("multiple-groups") { store -> + assertTrue(store.exists(0)) + assertFalse(store.exists(1)) + assertFalse(store.exists(254)) + assertTrue(store.exists(255)) + + assertTrue(store.exists(0, 0)) + assertTrue(store.exists(0, 1)) + assertFalse(store.exists(0, 2)) + + assertFalse(store.exists(1, 0)) + assertFalse(store.exists(254, 0)) + + assertTrue(store.exists(255, 0)) + assertFalse(store.exists(255, 1)) + assertFalse(store.exists(255, 65535)) + assertTrue(store.exists(255, 65536)) + assertFalse(store.exists(255, 65537)) + } + } + + @Test + fun testList() { + readTest("empty") { store -> + assertEquals(emptyList(), store.list()) + } + + readTest("multiple-groups") { store -> + assertEquals(listOf(0, 255), store.list()) + + assertEquals(listOf(0, 1), store.list(0)) + assertEquals(listOf(0, 65536), store.list(255)) + + assertThrows { + store.list(1) + } + + assertThrows { + store.list(254) + } + } + } + + @Test + fun testCreate() { + writeTest("empty-archive") { store -> + store.create(255) + } + + overwriteTest("empty-archive", "empty-archive") { store -> + store.create(255) + } + } + + @Test + fun testRead() { + readTest("multiple-groups") { store -> + store.read(0, 0).use { actual -> + assertEquals(Unpooled.EMPTY_BUFFER, actual) + } + + store.read(0, 1).use { actual -> + assertEquals(Unpooled.EMPTY_BUFFER, actual) + } + + assertThrows { + store.read(0, 2).release() + } + + assertThrows { + store.read(1, 0).release() + } + + Unpooled.wrappedBuffer("OpenRS2".toByteArray()).use { expected -> + store.read(255, 0).use { actual -> + assertEquals(expected, actual) + } + + store.read(255, 65536).use { actual -> + assertEquals(expected, actual) + } + } + } + } + + @Test + fun testWrite() { + writeTest("single-group") { store -> + Unpooled.wrappedBuffer("OpenRS2".toByteArray()).use { buf -> + store.write(255, 0, buf) + } + } + + overwriteTest("empty-archive", "single-group") { store -> + Unpooled.wrappedBuffer("OpenRS2".toByteArray()).use { buf -> + store.write(255, 0, buf) + } + } + + overwriteTest("single-group", "single-group-overwritten") { store -> + Unpooled.wrappedBuffer("Hello, world!".toByteArray()).use { buf -> + store.write(255, 0, buf) + } + } + } + + @Test + fun testRemoveArchive() { + overwriteTest("multiple-groups", "empty") { store -> + store.remove(0) + store.remove(255) + } + + writeTest("empty") { store -> + store.remove(0) + store.remove(255) + } + } + + @Test + fun testRemoveGroup() { + overwriteTest("single-group", "empty-archive") { store -> + store.remove(255, 0) + } + + overwriteTest("empty-archive", "empty-archive") { store -> + store.remove(255, 0) + } + + writeTest("empty") { store -> + store.remove(255, 0) + } + } + + private fun readTest(name: String, f: (Store) -> Unit) { + FlatFileStore.open(root.resolve(name)).use { store -> + f(store) + } + } + + private fun writeTest(name: String, f: (Store) -> Unit) { + Jimfs.newFileSystem(Configuration.unix()).use { fs -> + val actual = fs.getPath("/cache") + FlatFileStore.create(actual).use { store -> + f(store) + } + + val expected = root.resolve(name) + assertTrue(expected.recursiveEquals(actual, filter = IGNORE_GIT_EMPTY)) + } + } + + private fun overwriteTest(src: String, name: String, f: (Store) -> Unit) { + Jimfs.newFileSystem(Configuration.unix()).use { fs -> + val actual = fs.getPath("/cache") + root.resolve(src).recursiveCopy(actual) + + FlatFileStore.open(actual).use { store -> + f(store) + } + + val expected = root.resolve(name) + assertTrue(expected.recursiveEquals(actual, filter = IGNORE_GIT_EMPTY)) + } + } +} diff --git a/cache/src/test/resources/dev/openrs2/cache/flat-file-store/empty-archive/255/.gitempty b/cache/src/test/resources/dev/openrs2/cache/flat-file-store/empty-archive/255/.gitempty new file mode 100644 index 00000000..e69de29b diff --git a/cache/src/test/resources/dev/openrs2/cache/flat-file-store/empty/.gitempty b/cache/src/test/resources/dev/openrs2/cache/flat-file-store/empty/.gitempty new file mode 100644 index 00000000..e69de29b diff --git a/cache/src/test/resources/dev/openrs2/cache/flat-file-store/multiple-groups/0/0.dat b/cache/src/test/resources/dev/openrs2/cache/flat-file-store/multiple-groups/0/0.dat new file mode 100644 index 00000000..e69de29b diff --git a/cache/src/test/resources/dev/openrs2/cache/flat-file-store/multiple-groups/0/1.dat b/cache/src/test/resources/dev/openrs2/cache/flat-file-store/multiple-groups/0/1.dat new file mode 100644 index 00000000..e69de29b diff --git a/cache/src/test/resources/dev/openrs2/cache/flat-file-store/multiple-groups/255/0.dat b/cache/src/test/resources/dev/openrs2/cache/flat-file-store/multiple-groups/255/0.dat new file mode 100644 index 00000000..7eab06ac --- /dev/null +++ b/cache/src/test/resources/dev/openrs2/cache/flat-file-store/multiple-groups/255/0.dat @@ -0,0 +1 @@ +OpenRS2 \ No newline at end of file diff --git a/cache/src/test/resources/dev/openrs2/cache/flat-file-store/multiple-groups/255/65536.dat b/cache/src/test/resources/dev/openrs2/cache/flat-file-store/multiple-groups/255/65536.dat new file mode 100644 index 00000000..7eab06ac --- /dev/null +++ b/cache/src/test/resources/dev/openrs2/cache/flat-file-store/multiple-groups/255/65536.dat @@ -0,0 +1 @@ +OpenRS2 \ No newline at end of file diff --git a/cache/src/test/resources/dev/openrs2/cache/flat-file-store/single-group-overwritten/255/0.dat b/cache/src/test/resources/dev/openrs2/cache/flat-file-store/single-group-overwritten/255/0.dat new file mode 100644 index 00000000..5dd01c17 --- /dev/null +++ b/cache/src/test/resources/dev/openrs2/cache/flat-file-store/single-group-overwritten/255/0.dat @@ -0,0 +1 @@ +Hello, world! \ No newline at end of file diff --git a/cache/src/test/resources/dev/openrs2/cache/flat-file-store/single-group/255/0.dat b/cache/src/test/resources/dev/openrs2/cache/flat-file-store/single-group/255/0.dat new file mode 100644 index 00000000..7eab06ac --- /dev/null +++ b/cache/src/test/resources/dev/openrs2/cache/flat-file-store/single-group/255/0.dat @@ -0,0 +1 @@ +OpenRS2 \ No newline at end of file diff --git a/util/src/main/java/dev/openrs2/util/io/PathExtensions.kt b/util/src/main/java/dev/openrs2/util/io/PathExtensions.kt index 8dfd68ab..2b4b6d4e 100644 --- a/util/src/main/java/dev/openrs2/util/io/PathExtensions.kt +++ b/util/src/main/java/dev/openrs2/util/io/PathExtensions.kt @@ -41,14 +41,15 @@ public fun Path.recursiveCopy( public fun Path.recursiveEquals( other: Path, linkOptions: Array = emptyArray(), - openOptions: Array = emptyArray() + openOptions: Array = emptyArray(), + filter: (Path) -> Boolean = { true } ): Boolean { val list1 = Files.newDirectoryStream(this).use { stream -> - stream.map { this.relativize(it).toString() }.sorted().toList() + stream.filter(filter).map { this.relativize(it).toString() }.sorted().toList() } val list2 = Files.newDirectoryStream(other).use { stream -> - stream.map { other.relativize(it).toString() }.sorted().toList() + stream.filter(filter).map { other.relativize(it).toString() }.sorted().toList() } if (list1 != list2) {