From 8426dff0447f587463bbbeb912c32b825f349c58 Mon Sep 17 00:00:00 2001 From: Gleb Koval Date: Sun, 7 Jan 2024 16:09:23 +0000 Subject: [PATCH] Improve tests & validate FSEntry names (#4) Contributes to #2 . - Add some additional tests to existing functionality. - Cleanup code to use more of `kotlin.io.*` instead of `java.nio.*`. - Validate FSEntry names on object construction (and tests). - Validate folders do not contain entries with duplicate names on `FSCreator.create()` (and tests). Reviewed-on: https://git.koval.net/cyclane/teamcity-build-step-extension-test-task/pulls/4 --- src/main/kotlin/filesystem/FSCreator.kt | 30 ++++--- src/main/kotlin/filesystem/FSEntry.kt | 36 +++++++- src/test/kotlin/filesystem/FSCreatorTest.kt | 70 +++++++++++---- src/test/kotlin/filesystem/FSEntryTest.kt | 96 ++++++++++++++++++++- 4 files changed, 200 insertions(+), 32 deletions(-) diff --git a/src/main/kotlin/filesystem/FSCreator.kt b/src/main/kotlin/filesystem/FSCreator.kt index 4b8d4ca..8f8e484 100644 --- a/src/main/kotlin/filesystem/FSCreator.kt +++ b/src/main/kotlin/filesystem/FSCreator.kt @@ -1,28 +1,36 @@ package filesystem import java.nio.file.FileAlreadyExistsException -import java.nio.file.Files -import java.nio.file.Path +import kotlin.io.path.Path +import kotlin.io.path.createDirectory +import kotlin.io.path.createFile +import kotlin.io.path.writeText class FSCreator { /** * Create entry, leaving existing folders' contents, but overwriting existing files. * @throws CyclicFolderException Cyclic folders cannot be created. + * @throws DuplicateEntryNameException A folder or sub-folder contains entries with duplicate names. */ - @Throws(CyclicFolderException::class) + @Throws(CyclicFolderException::class, DuplicateEntryNameException::class) fun create( entryToCreate: FSEntry, destination: String, ) { // No point in running anything if we know the input is invalid. - if (entryToCreate is FSFolder && entryToCreate.isCyclic()) { - throw CyclicFolderException() + if (entryToCreate is FSFolder) { + if (entryToCreate.isCyclic()) { + throw CyclicFolderException() + } + if (entryToCreate.deepHasDuplicateNames()) { + throw DuplicateEntryNameException() + } } val queue = ArrayDeque( listOf( - entryToCreate to Path.of(destination), + entryToCreate to Path(destination), ), ) @@ -31,17 +39,19 @@ class FSCreator { val path = dest.resolve(entry.name) try { when (entry) { - is FSFile -> Files.createFile(path) - is FSFolder -> Files.createDirectory(path) + is FSFile -> path.createFile() + is FSFolder -> path.createDirectory() } } catch (_: FileAlreadyExistsException) { } // Allow files/folders to already exist. when (entry) { - is FSFile -> Files.write(path, entry.content.toByteArray()) + is FSFile -> path.writeText(entry.content) is FSFolder -> queue.addAll(entry.entries.map { it to path }) } } } } -class CyclicFolderException : Exception("Cyclic FSFolders are not supported") \ No newline at end of file +class CyclicFolderException : Exception("Cyclic FSFolders are not supported") + +class DuplicateEntryNameException : Exception("Folder contains entries with duplicate names") \ No newline at end of file diff --git a/src/main/kotlin/filesystem/FSEntry.kt b/src/main/kotlin/filesystem/FSEntry.kt index 6cad9ad..534972b 100644 --- a/src/main/kotlin/filesystem/FSEntry.kt +++ b/src/main/kotlin/filesystem/FSEntry.kt @@ -1,8 +1,18 @@ package filesystem +import kotlin.io.path.Path + // Note sealed allows for simpler logic in FSCreator by guaranteeing FSFile and FSFolder are the only possible FSEntries // (as we expect), and it also implicitly makes the class abstract as required. -sealed class FSEntry(val name: String) +sealed class FSEntry(val name: String) { + init { + val p = Path(name) + // Only allow single filenames (no paths or relative references (e.g. "..")) + if (p.toList().size != 1 || p.fileName != p.toFile().canonicalFile.toPath().fileName) { + throw InvalidEntryNameException(name) + } + } +} class FSFile(name: String, val content: String) : FSEntry(name) @@ -24,4 +34,26 @@ class FSFolder(name: String, val entries: List) : FSEntry(name) { } return false } -} \ No newline at end of file + + /** + * Check whether a folder contains multiple entries with the same name. + */ + fun hasDuplicateNames(): Boolean { + val seen = HashSet() + return entries.any { !seen.add(it.name) } + } + + internal fun deepHasDuplicateNames(): Boolean { + val queue = ArrayDeque(listOf(this)) + while (queue.isNotEmpty()) { + val entry = queue.removeFirst() + if (entry.hasDuplicateNames()) { + return true + } + queue.addAll(entry.entries.mapNotNull { it as? FSFolder }) + } + return false + } +} + +class InvalidEntryNameException(name: String) : Exception("Invalid FSEntry name: '$name'") \ No newline at end of file diff --git a/src/test/kotlin/filesystem/FSCreatorTest.kt b/src/test/kotlin/filesystem/FSCreatorTest.kt index ab7c2df..29ada59 100644 --- a/src/test/kotlin/filesystem/FSCreatorTest.kt +++ b/src/test/kotlin/filesystem/FSCreatorTest.kt @@ -1,9 +1,10 @@ package filesystem import org.junit.jupiter.api.* -import java.nio.file.Files -import java.nio.file.Path +import java.io.File import java.util.concurrent.TimeUnit +import kotlin.io.path.Path +import kotlin.io.path.createDirectory import kotlin.test.Test import kotlin.test.assertEquals @@ -13,17 +14,26 @@ class FSCreatorTest { @BeforeEach fun `before each`() { assertDoesNotThrow("should create _tmp directory") { - Files.createDirectory(Path.of("_tmp")) + Path("_tmp").createDirectory() } } @AfterEach fun `after each`() { assertDoesNotThrow("should delete _tmp directory") { - deleteRecursive(Path.of("_tmp")) + File("_tmp").deleteRecursively() } } + @Test + fun `create file`() { + val file = FSFile("test.txt", "This is a file") + assertDoesNotThrow("should create file") { + creator.create(file, "_tmp") + } + assertEquals(file.content, File("_tmp/", file.name).readText()) + } + @Test fun `create entries`() { val readme = FSFile("README", "Hello World!") @@ -61,10 +71,10 @@ class FSCreatorTest { } // If objects don't exist, these functions will throw anyway, so don't explicitly check for existence. // Similarly, don't explicitly check if an object is a directory. - assertEquals(readme.content, Files.readString(Path.of("_tmp/folder", readme.name))) - assertEquals(gomod.content, Files.readString(Path.of("_tmp/folder", gomod.name))) - assertEquals(maingo.content, Files.readString(Path.of("_tmp/folder", maingo.name))) - assertEquals(helloworldgo.content, Files.readString(Path.of("_tmp/folder/utils", helloworldgo.name))) + assertEquals(readme.content, File("_tmp/folder", readme.name).readText()) + assertEquals(gomod.content, File("_tmp/folder", gomod.name).readText()) + assertEquals(maingo.content, File("_tmp/folder", maingo.name).readText()) + assertEquals(helloworldgo.content, File("_tmp/folder/utils", helloworldgo.name).readText()) } @Test @@ -94,6 +104,7 @@ class FSCreatorTest { "folder", listOf( FSFolder( + // Repeated name should be fine here (not throw) "folder", listOf( FSFolder( @@ -111,11 +122,11 @@ class FSCreatorTest { "_tmp", ) } - assertEquals("hi", Files.readString(Path.of("_tmp/folder/sub-folder/hi"))) - assertEquals("P4ssW0rd", Files.readString(Path.of("_tmp/folder/folder/secrets/secret"))) - assertEquals("One is a good number", Files.readString(Path.of("_tmp/folder/1.txt"))) - assertEquals("Two!", Files.readString(Path.of("_tmp/folder/2.txt"))) - assertEquals("Three!", Files.readString(Path.of("_tmp/folder/3.txt"))) + assertEquals("hi", File("_tmp/folder/sub-folder/hi").readText()) + assertEquals("P4ssW0rd", File("_tmp/folder/folder/secrets/secret").readText()) + assertEquals("One is a good number", File("_tmp/folder/1.txt").readText()) + assertEquals("Two!", File("_tmp/folder/2.txt").readText()) + assertEquals("Three!", File("_tmp/folder/3.txt").readText()) } @Test @@ -142,13 +153,34 @@ class FSCreatorTest { creator.create(folder4, "_tmp") } } -} -fun deleteRecursive(path: Path) { - if (Files.isDirectory(path)) { - for (child in Files.list(path)) { - deleteRecursive(child) + @Test + fun `create throws on folder with duplicate names`() { + val folder = + FSFolder( + "folder", + listOf( + FSFile("README.md", "# Test File"), + FSFile("hello-world.txt", "Hello World!"), + FSFolder( + "src", + listOf( + FSFile("README.md", "# Source files"), + FSFolder( + "solution", + listOf( + FSFile("solution.py", "print('1 + 1 = 1')"), + FSFile("tmp", "A temporary file"), + FSFolder("tmp", listOf()), + ), + ), + ), + ), + FSFolder("tmp", listOf()), + ), + ) + assertThrows { + creator.create(folder, "_tmp") } } - Files.delete(path) } \ No newline at end of file diff --git a/src/test/kotlin/filesystem/FSEntryTest.kt b/src/test/kotlin/filesystem/FSEntryTest.kt index b474ac2..86076bd 100644 --- a/src/test/kotlin/filesystem/FSEntryTest.kt +++ b/src/test/kotlin/filesystem/FSEntryTest.kt @@ -1,10 +1,34 @@ package filesystem -import org.junit.jupiter.api.Test +import org.junit.jupiter.api.* import kotlin.test.assertFalse import kotlin.test.assertTrue class FSEntryTest { + @Test + fun `valid name entries`() { + assertDoesNotThrow("should construct FSFile and FSFolder without throwing") { + FSFile("A file with a name.tar.xz", "Contents") + FSFolder(".a folder with a name", listOf()) + } + } + + @Test + fun `invalid name entries`() { + assertThrows { + FSFile("File/here", "Contents") + } + assertThrows { + FSFolder("Folder/here", listOf()) + } + assertThrows { + FSFolder(".", listOf()) + } + assertThrows { + FSFolder("/", listOf()) + } + } + @Test fun `non-cyclic folder`() { val folder = @@ -39,4 +63,74 @@ class FSEntryTest { assertTrue(folder3.isCyclic()) assertTrue(folder4.isCyclic()) } + + @Test + fun `no duplicate names folder`() { + val folder = + FSFolder( + "folder", + listOf( + FSFile("README.md", "# Test File"), + FSFile("hello-world.txt", "Hello World!"), + FSFolder( + "src", + listOf( + FSFile("README.md", "# Source files"), + FSFile("solution-1.py", "print('1 + 1 = 1')"), + FSFile("solution-2.py", "print('1 + 1 = 1')"), + ), + ), + FSFolder("tmp", listOf()), + ), + ) + assertFalse(folder.hasDuplicateNames()) + assertFalse(folder.deepHasDuplicateNames()) + } + + @Test + fun `shallow duplicate names folder`() { + val folder = + FSFolder( + "folder", + listOf( + FSFile("README.md", "# Test File"), + FSFile("hello-world.txt", "Hello World!"), + FSFolder( + "src", + listOf( + FSFile("README.md", "# Source files"), + FSFile("solution-1.py", "print('1 + 1 = 1')"), + FSFile("solution-2.py", "print('1 + 1 = 1')"), + ), + ), + FSFolder("tmp", listOf()), + FSFile("tmp", "A temporary file"), + ), + ) + assertTrue(folder.hasDuplicateNames()) + assertTrue(folder.deepHasDuplicateNames()) + } + + @Test + fun `deep duplicate names folder`() { + val folder = + FSFolder( + "folder", + listOf( + FSFile("README.md", "# Test File"), + FSFile("hello-world.txt", "Hello World!"), + FSFolder( + "src", + listOf( + FSFile("README.md", "# Source files"), + FSFile("solution-1.py", "print('1 + 1 = 1')"), + FSFile("solution-1.py", "print('1 + 1 = 1')"), + ), + ), + FSFolder("tmp", listOf()), + ), + ) + assertFalse(folder.hasDuplicateNames()) + assertTrue(folder.deepHasDuplicateNames()) + } } \ No newline at end of file