Skip to content
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type" : "bugfix",
"description" : "Preserve file permissions in the zip archive, fix bugs in gitignore matching and keep mvm and gradle files"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the change user facing, ie is a changelog required here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, it does have a user facing change. updated the messaging.

}
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,25 @@ class FeatureDevSessionContextTest : FeatureDevTestBase() {
fun testWithValidFile() {
val ktFile = mock<VirtualFile>()
whenever(ktFile.extension).thenReturn("kt")
whenever(ktFile.path).thenReturn("code.kt")
assertTrue(featureDevSessionContext.isFileExtensionAllowed(ktFile))
}

@Test
fun testWithInvalidFile() {
val txtFile = mock<VirtualFile>()
whenever(txtFile.extension).thenReturn("txt")
whenever(txtFile.path).thenReturn("file.txt")
assertFalse(featureDevSessionContext.isFileExtensionAllowed(txtFile))
}

@Test
fun testAllowedFilePath() {
val allowedPaths = listOf("Dockerfile", "Dockerfile.build", "gradlew", "build.gradle", "gradle.properties", ".mvn/wrapper/maven-wrapper.properties")
allowedPaths.forEach({
val txtFile = mock<VirtualFile>()
whenever(txtFile.path).thenReturn(it)
assertTrue(featureDevSessionContext.isFileExtensionAllowed(txtFile))
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,27 +10,34 @@
import com.intellij.openapi.vfs.VirtualFileVisitor
import com.intellij.openapi.vfs.isFile
import com.intellij.platform.ide.progress.withBackgroundProgress
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.async
import kotlinx.coroutines.flow.channelFlow
import kotlinx.coroutines.launch
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withContext
import org.apache.commons.codec.digest.DigestUtils
import software.aws.toolkits.core.utils.outputStream
import software.aws.toolkits.core.utils.putNextEntry
import org.apache.commons.io.FileUtils
import software.aws.toolkits.jetbrains.core.coroutines.EDT
import software.aws.toolkits.jetbrains.core.coroutines.getCoroutineBgContext
import software.aws.toolkits.jetbrains.services.telemetry.ALLOWED_CODE_EXTENSIONS
import software.aws.toolkits.resources.AwsCoreBundle
import software.aws.toolkits.telemetry.AmazonqTelemetry
import java.io.File
import java.io.FileInputStream
import java.net.URI
import java.nio.file.FileSystem
import java.nio.file.FileSystems
import java.nio.file.Files
import java.nio.file.Path
import java.nio.file.Paths
import java.nio.file.StandardCopyOption
import java.util.Base64
import java.util.zip.ZipOutputStream
import java.util.UUID
import kotlin.coroutines.coroutineContext
import kotlin.io.path.Path
import kotlin.io.path.createParentDirectories
import kotlin.io.path.getPosixFilePermissions
import kotlin.io.path.relativeTo

interface RepoSizeError {
Expand Down Expand Up @@ -74,6 +81,12 @@
"Dockerfile.build"
)

// patterns to explicitly allow unless matched with gitignore rules
private val allowedPatterns = setOf(
".*mvn.*",
".*gradle.*",
).map { Regex(it) }

// projectRoot: is the directory where the project is located when selected to open a project.
val projectRoot = project.guessProjectDir() ?: error("Cannot guess base directory for project ${project.name}")

Expand Down Expand Up @@ -108,7 +121,10 @@
fun isFileExtensionAllowed(file: VirtualFile): Boolean {
// if it is a directory, it is allowed
if (file.isDirectory) return true

val explicitAllowed = allowedPatterns.map { pattern -> pattern.matches(file.path) }.any { it }
if (explicitAllowed) {
return true
}
val extension = file.extension ?: return false
return ALLOWED_CODE_EXTENSIONS.contains(extension)
}
Expand All @@ -122,7 +138,12 @@
// this method reads like something a JS dev would write and doesn't do what the author thinks
val deferredResults = ignorePatternsWithGitIgnore.map { pattern ->
withContext(coroutineContext) {
async { pattern.containsMatchIn(path) }
// avoid partial match (pattern.containsMatchIn) since it causes us matching files
// against folder patterns. (e.g. settings.gradle ignored by .gradle rule!)
// we convert the glob rules to regex, add a trailing /* to all rules and then match
// entries against them by adding a trailing /.
// TODO: Add unit tests for gitignore matching
async { pattern.matches("$path/") }
}
}

Expand Down Expand Up @@ -187,18 +208,34 @@
}
}

createTemporaryZipFileAsync { zipOutput ->
val zipFilePath = createTemporaryZipFileAsync { zipfs ->
filesToIncludeFlow.collect { file ->
val relativePath = Path(file.path).relativeTo(projectRoot.toNioPath())
zipOutput.putNextEntry(relativePath.toString(), Path(file.path))
if (!file.isDirectory) {
val externalFilePath = Path(file.path)
val externalFilePermissions = externalFilePath.getPosixFilePermissions()
val relativePath = Path(file.path).relativeTo(projectRoot.toNioPath())
val zipfsPath = zipfs.getPath("/${relativePath}")
withContext(Dispatchers.IO) {
zipfsPath.createParentDirectories()
Files.copy(externalFilePath, zipfsPath, StandardCopyOption.REPLACE_EXISTING)
Files.setAttribute(zipfsPath, "zip:permissions", externalFilePermissions);

Check warning on line 221 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/FeatureDevSessionContext.kt

View workflow job for this annotation

GitHub Actions / qodana

Redundant semicolon

Redundant semicolon
}
}
}
}
zipFilePath
}.toFile()

private suspend fun createTemporaryZipFileAsync(block: suspend (ZipOutputStream) -> Unit): Path = withContext(EDT) {
val file = Files.createTempFile(null, ".zip")
ZipOutputStream(file.outputStream()).use { zipOutput -> block(zipOutput) }
file
private suspend fun createTemporaryZipFileAsync(block: suspend (FileSystem) -> Unit): Path = withContext(EDT) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be on the background thread?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good call, changed it to run on background

// Don't use Files.createTempFile since the file must not be created for ZipFS to work
val tempFilePath: Path = Paths.get(FileUtils.getTempDirectory().getAbsolutePath(), "${UUID.randomUUID()}.zip")

Check notice on line 231 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/FeatureDevSessionContext.kt

View workflow job for this annotation

GitHub Actions / qodana

Accessor call that can be replaced with property access syntax

Use of getter method instead of property access syntax
val uri = URI.create("jar:file:${tempFilePath}")
val env = hashMapOf("create" to "true")
val zipfs = FileSystems.newFileSystem(uri, env)
zipfs.use {
block(zipfs)
}
tempFilePath
}

private fun parseGitIgnore(): Set<String> {
Expand All @@ -216,7 +253,7 @@
private fun convertGitIgnorePatternToRegex(pattern: String): String = pattern
.replace(".", "\\.")
.replace("*", ".*")
.let { if (it.endsWith("/")) "$it?" else it } // Handle directory-specific patterns by optionally matching trailing slash
.let { if (it.endsWith("/")) "$it.*" else "$it/.*" } // Add a trailing /* to all patterns. (we add a trailing / to all files when matching)

var selectedSourceFolder: VirtualFile
set(newRoot) {
Expand Down
Loading