[Gradle][MPP] Refine zipUtils.kt & implement ZipUtilsTest.kt

^KT-46198

libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/CompositeMetadataJarTest.kt

@@ -7,7 +7,6 @@
 
 package org.jetbrains.kotlin.gradle
 
-import org.gradle.kotlin.dsl.support.unzipTo
 import org.gradle.kotlin.dsl.support.zipTo
 import org.jetbrains.kotlin.gradle.plugin.mpp.CompositeMetadataJar
 import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinProjectStructureMetadata
@@ -120,6 +119,7 @@ class CompositeMetadataJarTest {
             "Expected correct archiveExtension for extracted sourceSetA"
         )
         assertZipContentEquals(
+            temporaryFolder,
             primaryArtifactContent.resolve("sourceSetA"), sourceSetAMetadataFile,
             "Expected correct content of extracted 'sourceSetA'"
         )
@@ -132,6 +132,7 @@ class CompositeMetadataJarTest {
             "Expected correct archiveExtension for extracted sourceSetB"
         )
         assertZipContentEquals(
+            temporaryFolder,
             primaryArtifactContent.resolve("sourceSetB"), sourceSetBMetadataFile,
             "Expected correct content of extracted 'sourceSetA'"
         )
@@ -180,6 +181,7 @@ class CompositeMetadataJarTest {
                 val interopA0MetadataFile = sourceSetAInteropMetadataFiles.firstOrNull { it.name == "interopA0.klib" }
                     ?: fail("Failed to find 'interopA0.klib'")
                 assertZipContentEquals(
+                    temporaryFolder,
                     primaryArtifactContent.resolve("sourceSetA-cinterop/interopA0"), interopA0MetadataFile,
                     "Expected correct content for extracted 'interopA0'"
                 )
@@ -190,6 +192,7 @@ class CompositeMetadataJarTest {
                 val interopA1MetadataFile = sourceSetAInteropMetadataFiles.firstOrNull { it.name == "interopA1.klib" }
                     ?: fail("Failed to find 'interopA1.klib")
                 assertZipContentEquals(
+                    temporaryFolder,
                     primaryArtifactContent.resolve("sourceSetA-cinterop/interopA1"), interopA1MetadataFile,
                     "Expected correct content for extracted 'interopA1'"
                 )
@@ -205,44 +208,12 @@ class CompositeMetadataJarTest {
             val interopB0MetadataFile = sourceSetBInteropMetadataFiles.firstOrNull { it.name == "interopB0.klib" }
                 ?: fail("Failed to find 'interopB0.klib'")
             assertZipContentEquals(
+                temporaryFolder,
                 primaryArtifactContent.resolve("sourceSetB-cinterop/interopB0"), interopB0MetadataFile,
                 "Expected correct content for extracted 'interopB0'"
             )
         }
     }
-
-    private fun assertZipContentEquals(expectedContent: File, zipFile: File, message: String) {
-        val zipOutputDirectory = temporaryFolder.newFolder()
-        unzipTo(zipOutputDirectory, zipFile)
-        assertContentEquals(expectedContent, zipOutputDirectory, message)
-    }
-
-    private fun assertContentEquals(expected: File, actual: File, message: String) {
-        assertTrue(expected.isDirectory, "Expected $expected to be directory")
-        assertTrue(actual.isDirectory, "Expected $actual to be directory")
-
-        val expectedFiles = expected.listFiles().orEmpty()
-        val actualFiles = actual.listFiles().orEmpty()
-
-        val expectedFileNames = expectedFiles.map { it.name }.sorted().toSet()
-        val actualFileNames = actualFiles.map { it.name }.sorted().toSet()
-        assertEquals(expectedFileNames, actualFileNames, "$message: ${expected.name} does not contain the same files")
-
-        expectedFiles.forEach { expectedFile ->
-            val actualFile = actualFiles.single { it.name == expectedFile.name }
-
-            if (expectedFile.isFile && actualFile.isFile) {
-                assertTrue(
-                    expectedFile.readBytes().contentEquals(actualFile.readBytes()),
-                    "$message: ${expectedFile.name} does not match in ${expected.name}"
-                )
-            } else if (expectedFile.isDirectory && actualFile.isDirectory) {
-                assertContentEquals(expectedFile, actualFile, message)
-            } else {
-                fail("Expected $expectedFile to be 'file' or 'directory'")
-            }
-        }
-    }
 }
 
 private fun createProjectStructureMetadata(

libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/ZipUtilsTest.kt

@@ -0,0 +1,208 @@
+/*
+ * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+@file:Suppress("FunctionName")
+
+package org.jetbrains.kotlin.gradle
+
+import org.gradle.kotlin.dsl.support.unzipTo
+import org.gradle.kotlin.dsl.support.zipTo
+import org.jetbrains.kotlin.gradle.utils.copyZipFilePartially
+import org.jetbrains.kotlin.gradle.utils.listDescendants
+import org.junit.Rule
+import org.junit.Test
+import org.junit.jupiter.api.assertThrows
+import org.junit.rules.TemporaryFolder
+import java.io.File
+import java.util.zip.ZipEntry
+import java.util.zip.ZipFile
+import java.util.zip.ZipOutputStream
+import kotlin.test.assertEquals
+import kotlin.test.assertTrue
+import kotlin.test.fail
+
+class ZipUtilsTest {
+
+    @get:Rule
+    val temporaryFolder = TemporaryFolder()
+
+    private val zipContentFolder by lazy {
+        temporaryFolder.newFolder().apply {
+            resolve("stub0.txt").apply {
+                parentFile.mkdirs()
+                writeText("stub0 content")
+            }
+
+            resolve("a/stub1.txt").apply {
+                parentFile.mkdirs()
+                writeText("stub1 content")
+            }
+
+            resolve("a/b/stub2.txt").apply {
+                parentFile.mkdirs()
+                writeText("stub2 content")
+            }
+
+            resolve("a/b/stub3.txt").apply {
+                parentFile.mkdirs()
+                writeText("stub3 content")
+            }
+
+            resolve("c/stub4.txt").apply {
+                parentFile.mkdirs()
+                writeText("stub4 content")
+            }
+        }
+    }
+
+    private val zipFile by lazy {
+        temporaryFolder.newFile().apply {
+            zipTo(this, zipContentFolder)
+        }
+    }
+
+    @Test
+    fun `test - listDescendants - malformed path string`() {
+        ZipFile(zipFile).use { zip ->
+            assertThrows<IllegalArgumentException> { zip.listDescendants("a") }
+            assertThrows<IllegalArgumentException> { zip.listDescendants("a/b") }
+            assertThrows<IllegalArgumentException> { zip.listDescendants("c") }
+        }
+    }
+
+    @Test
+    fun `test listDescendants`() {
+        ZipFile(zipFile).use { zip ->
+            assertEquals(
+                setOf("stub0.txt", "a/stub1.txt", "a/b/stub2.txt", "a/b/stub3.txt", "c/stub4.txt").sorted().toSet(),
+                zip.listDescendants("").filter { it.isDirectory.not() }.map { it.name }.sorted().toSet(),
+                "Expected all descendants being listed"
+            )
+
+            assertEquals(
+                setOf("a/stub1.txt", "a/b/stub2.txt", "a/b/stub3.txt").sorted().toSet(),
+                zip.listDescendants("a/").filter { it.isDirectory.not() }.map { it.name }.sorted().toSet(),
+                "Expected all children of 'a/' being listed"
+            )
+
+            assertEquals(
+                setOf("a/b/stub2.txt", "a/b/stub3.txt").sorted().toSet(),
+                zip.listDescendants("a/b/").filter { it.isDirectory.not() }.map { it.name }.sorted().toSet(),
+                "Expected all descendants of 'a/b/' being listed"
+            )
+
+            assertEquals(
+                setOf("c/stub4.txt"),
+                zip.listDescendants("c/").filter { it.isDirectory.not() }.map { it.name }.sorted().toSet(),
+                "Expected all descendants of 'c/' being listed"
+            )
+        }
+    }
+
+    @Test
+    fun `test copyZipFilePartially - root path`() {
+        val copiedFile = temporaryFolder.newFile()
+        copyZipFilePartially(zipFile, copiedFile, path = "")
+        assertZipContentEquals(
+            temporaryFolder, zipContentFolder, copiedFile,
+            "Expected correct content for copied file with root path"
+        )
+    }
+
+    @Test
+    fun `test copyZipFilePartially - a`() {
+        val copiedFile = temporaryFolder.newFile()
+        copyZipFilePartially(zipFile, copiedFile, path = "a/")
+        assertZipContentEquals(
+            temporaryFolder, zipContentFolder.resolve("a"), copiedFile,
+            "Expected correct content for copied file with path 'a/'"
+        )
+    }
+
+    @Test
+    fun `test copyZipFilePartially - a b`() {
+        val copiedFile = temporaryFolder.newFile()
+        copyZipFilePartially(zipFile, copiedFile, path = "a/b/")
+        assertZipContentEquals(
+            temporaryFolder, zipContentFolder.resolve("a/b"), copiedFile,
+            "Expected correct content for copied file with path 'a/b/'"
+        )
+    }
+
+    @Test
+    fun `test copyZipFilePartially - c`() {
+        val copiedFile = temporaryFolder.newFile()
+        copyZipFilePartially(zipFile, copiedFile, path = "c/")
+        assertZipContentEquals(
+            temporaryFolder, zipContentFolder.resolve("c"), copiedFile,
+            "Expected correct content for copied file with path 'c/'"
+        )
+    }
+
+    @Test
+    fun `test copyZipFilePartially - retains folder entries`() {
+        val sourceZipFile = temporaryFolder.newFile()
+        ZipOutputStream(sourceZipFile.outputStream()).use { zipOutputStream ->
+            zipOutputStream.putNextEntry(ZipEntry("a/"))
+            zipOutputStream.closeEntry()
+
+            zipOutputStream.putNextEntry(ZipEntry("a/b/"))
+            zipOutputStream.closeEntry()
+
+            zipOutputStream.putNextEntry(ZipEntry("a/b/c/"))
+            zipOutputStream.closeEntry()
+
+            zipOutputStream.putNextEntry(ZipEntry("a/b/c/stub.txt"))
+            zipOutputStream.write("Text".toByteArray())
+            zipOutputStream.closeEntry()
+        }
+
+        val destinationZipFile = temporaryFolder.newFile()
+        copyZipFilePartially(sourceZipFile, destinationZipFile, "a/")
+
+        ZipFile(destinationZipFile).use { zip ->
+            assertEquals(
+                setOf("b/", "b/c/", "b/c/stub.txt"),
+                zip.entries().toList().map { it.name }.sorted().toSet()
+            )
+        }
+    }
+}
+
+fun assertZipContentEquals(
+    temporaryFolder: TemporaryFolder,
+    expectedContent: File, zipFile: File, message: String
+) {
+    val zipOutputDirectory = temporaryFolder.newFolder()
+    unzipTo(zipOutputDirectory, zipFile)
+    assertDirectoryContentEquals(expectedContent, zipOutputDirectory, message)
+}
+
+fun assertDirectoryContentEquals(expected: File, actual: File, message: String) {
+    assertTrue(expected.isDirectory, "Expected $expected to be directory")
+    assertTrue(actual.isDirectory, "Expected $actual to be directory")
+
+    val expectedFiles = expected.listFiles().orEmpty()
+    val actualFiles = actual.listFiles().orEmpty()
+
+    val expectedFileNames = expectedFiles.map { it.name }.sorted().toSet()
+    val actualFileNames = actualFiles.map { it.name }.sorted().toSet()
+    assertEquals(expectedFileNames, actualFileNames, "$message: ${expected.name} does not contain the same files")
+
+    expectedFiles.forEach { expectedFile ->
+        val actualFile = actualFiles.single { it.name == expectedFile.name }
+
+        if (expectedFile.isFile && actualFile.isFile) {
+            assertTrue(
+                expectedFile.readBytes().contentEquals(actualFile.readBytes()),
+                "$message: ${expectedFile.name} does not match in ${expected.name}"
+            )
+        } else if (expectedFile.isDirectory && actualFile.isDirectory) {
+            assertDirectoryContentEquals(expectedFile, actualFile, message)
+        } else {
+            fail("Expected $expectedFile to be 'file' or 'directory'")
+        }
+    }
+}

libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/CompositeMetadataJar.kt

@@ -6,7 +6,7 @@
 package org.jetbrains.kotlin.gradle.plugin.mpp
 
 import org.jetbrains.kotlin.gradle.utils.copyZipFilePartially
-import org.jetbrains.kotlin.gradle.utils.listChildren
+import org.jetbrains.kotlin.gradle.utils.listDescendants
 import java.io.File
 import java.util.zip.ZipFile
 
@@ -73,7 +73,7 @@ private class CompositeMetadataJarImpl(
 
         ZipFile(getArtifactFile(sourceSetName)).use { artifactZipFile ->
             val cinteropRootPath = "$sourceSetName-cinterop/"
-            val cinteropEntries = artifactZipFile.listChildren(cinteropRootPath)
+            val cinteropEntries = artifactZipFile.listDescendants(cinteropRootPath)
             val cinteropNames = cinteropEntries.map { entry ->
                 entry.name.removePrefix(cinteropRootPath).split("/", limit = 2).first()
             }.toSet()

libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/utils/zipUtils.kt

@@ -14,22 +14,21 @@ internal fun copyZipFilePartially(sourceZipFile: File, destinationZipFile: File,
     requireValidZipPath(path)
 
     ZipFile(sourceZipFile).use { zip ->
-        val entries = zip.entries().asSequence()
+        val entries = zip.listDescendants(path).toList()
-            .filter { it.name.startsWith(path) }
-            .filter { !it.isDirectory }.toList()
-
         if (entries.isEmpty()) return
 
         ZipOutputStream(destinationZipFile.outputStream()).use { destinationZipOutputStream ->
-            entries.forEach { entry ->
+            entries.forEach { sourceEntry ->
-                // Drop the source set name from the entry path
+                val destinationEntry = ZipEntry(sourceEntry.name.substringAfter(path))
-                val destinationEntry = ZipEntry(entry.name.substringAfter(path))
+                destinationZipOutputStream.putNextEntry(destinationEntry)
 
-                zip.getInputStream(entry).use { inputStream ->
+                if (!sourceEntry.isDirectory) {
-                    destinationZipOutputStream.putNextEntry(destinationEntry)
+                    zip.getInputStream(sourceEntry).use { inputStream ->
-                    inputStream.copyTo(destinationZipOutputStream)
+                        inputStream.copyTo(destinationZipOutputStream)
-                    destinationZipOutputStream.closeEntry()
+                    }
                 }
+
+                destinationZipOutputStream.closeEntry()
             }
         }
     }
@@ -42,22 +41,6 @@ internal fun ZipFile.listDescendants(path: String): Sequence<ZipEntry> {
     }
 }
 
-internal fun ZipFile.listChildren(path: String): Sequence<ZipEntry> {
+private fun requireValidZipPath(path: String) = require(path.isEmpty() || path.endsWith("/")) {
-    requireValidZipPath(path)
+    "Expected path to end with '/', found '$path'"
-    return listDescendants(path).filter { entry ->
-        entry.name.removePrefix(path).count { it == '/' } == 1
-    }
 }
-
-internal fun ZipFile.listDescendants(zipEntry: ZipEntry): Sequence<ZipEntry> {
-    require(zipEntry.isDirectory)
-    return listDescendants(zipEntry.name)
-}
-
-internal fun ZipFile.listChildren(zipEntry: ZipEntry): Sequence<ZipEntry> {
-    return listDescendants(zipEntry).filter { entry ->
-        entry.name.removePrefix(zipEntry.name).count { it == '/' } == 1
-    }
-}
-
-private fun requireValidZipPath(path: String) = require(path.endsWith("/")) { "Expected path to end with '/', found '$path'" }