Skip to content

Control automatic port forwarding with a devcontainer.json file #49

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Feb 27, 2025
Prev Previous commit
Next Next commit
Use kotlinx.serialization for JSON deserialization
  • Loading branch information
aaronlehmann committed Feb 26, 2025
commit 3433d28c2ccf9c0e4e86d6fa6764731dab3fccf0
3 changes: 2 additions & 1 deletion build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ plugins {
alias(libs.plugins.changelog) // Gradle Changelog Plugin
alias(libs.plugins.qodana) // Gradle Qodana Plugin
alias(libs.plugins.kover) // Gradle Kover Plugin
kotlin("plugin.serialization") version "2.1.10"
}

group = properties("pluginGroup").get()
Expand All @@ -24,7 +25,7 @@ repositories {
// Dependencies are managed with Gradle version catalog - read more: https://docs.gradle.org/current/userguide/platforms.html#sub:version-catalog
dependencies {
// implementation(libs.annotations)
implementation("org.json:json:20210307")
implementation("org.jetbrains.kotlinx:kotlinx-serialization-json:1.8.0")
Copy link
Collaborator

@fioan89 fioan89 Feb 26, 2025

Choose a reason for hiding this comment

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

Thank you for changing this.

Since IntelliJ provides it at runtime as well this should be compileOnly. Another important thing is that here we need to declare the version that is packaged with minimum supported IntelliJ.
We support IntelliJ 2022.3 which uses kotlinx.serialization:1.4.1 so that's the minimum version we should use, and it should only be changed when the minimum supported version of IntelliJ changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's unfortunate because this version is too old to support allowComments, and devcontainer.json files often include comments (since the reference implementation and VS Code allow them). It's not an issue in my particular case because I'll be stripping comments before injecting this file, but for maximum compatibility it seems like supporting comments would be a good thing.

Could we depend on this newer version and add a comment that the dependency can be changed to compileOnly once the minimum supported IDE version provides kotlinx.serialization >= 1.7.0? It looks like that minimum IDE version will be idea/251.14649.49 (which includes JetBrains/intellij-community@548a3c9).

Copy link
Collaborator

Choose a reason for hiding this comment

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

as far as I know it should be possible to shadow the dependencies but please test it. As a rule of thumb is best not to shadow intellij dependencies to avoid any classloader/memory leak issue.

Copy link

Choose a reason for hiding this comment

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

Some context: any jars packaged with the plugin are not exposed to the rest of the IDE or other plugins, the plugin has its own classloader, so it's a good way to make plugin stable in its sandbox

So if we need this serialization library and there's a compatibility issue, it's better to bundle it.

Copy link
Contributor Author

@aaronlehmann aaronlehmann Feb 26, 2025

Choose a reason for hiding this comment

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

I added the comment about switching to compileOnly in the future. I've been testing with IntelliJ 242.23339.11 and it seems to work fine, but just tried with EAP 251.22821.72 and it works with that version as well.

Let me know if you think it's better or safer to downgrade the dependency and remove allowComments. I'm also fine with that approach.

Copy link
Collaborator

Choose a reason for hiding this comment

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

the plugin has its own classloader, so it's a good way to make plugin stable in its sandbox

Correct , and intellij platform sdk also allows the plugins to reference classes provided by other plugins in which the classloader for those plugin dependencies will be used. Not really the case here, so I agree with your statement. It should be safe to pack the serialization library.

Let me know if you think it's better or safer to downgrade the dependency and remove allowComments

I agree with @kirillk there is no need for a downgrade for now.

}

// Set the JVM language level used to build the project.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ import kotlinx.coroutines.isActive
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import java.io.File
import org.json.JSONObject
import kotlinx.serialization.json.Json
import kotlinx.serialization.ExperimentalSerializationApi
import com.coder.jetbrains.settings.CoderBackendSettings

/**
Expand Down Expand Up @@ -56,41 +57,43 @@ class CoderPortForwardService(
poller?.cancel()
}

class InvalidJsonTypeException(message: String) : Exception(message)
companion object {
@OptIn(ExperimentalSerializationApi::class)
private val json = Json {
ignoreUnknownKeys = true
allowTrailingComma = true
allowComments = true
}
}

private fun start() {
val devcontainerFile = CoderBackendSettings.getDevcontainerFile()
if (devcontainerFile.exists()) {
try {
val json = devcontainerFile.readText()
val obj = JSONObject(json)
val jsonContent = devcontainerFile.readText()
val config = json.decodeFromString<DevContainerConfig>(jsonContent)

val portsAttributes = obj.optJSONObject("portsAttributes") ?: JSONObject()
portsAttributes.keys().forEach { spec ->
portsAttributes.optJSONObject(spec)?.let { attrs ->
val onAutoForward = attrs.opt("onAutoForward")
if (!isValidString(onAutoForward)) {
throw InvalidJsonTypeException("onAutoForward for port $spec is not a string value")
}
val onAutoForwardStr = onAutoForward as String
if (onAutoForwardStr == "ignore") {
// Process port attributes
config.portsAttributes.forEach { (spec, attrs) ->
when (attrs.onAutoForward) {
"ignore" -> {
logger.info("found ignored port specification $spec in devcontainer.json")
rules.add(0, PortRule(PortMatcher(spec), false))
} else if (onAutoForwardStr != "") {
}
"" -> {}
else -> {
logger.info("found auto-forward port specification $spec in devcontainer.json")
rules.add(0, PortRule(PortMatcher(spec), true))
}
}
}

val otherPortsAttributes = obj.optJSONObject("otherPortsAttributes") ?: JSONObject()
val otherPortsAutoForward = otherPortsAttributes.opt("onAutoForward")
if (!isValidString(otherPortsAutoForward)) {
throw InvalidJsonTypeException("otherPortsAttributes.onAutoForward is not a string value")
}
if ((otherPortsAutoForward as String) == "ignore") {
logger.info("found ignored setting for otherPortsAttributes in devcontainer.json")
defaultForward = false
// Process other ports attributes
config.otherPortsAttributes?.let {
if (it.onAutoForward == "ignore") {
logger.info("found ignored setting for otherPortsAttributes in devcontainer.json")
defaultForward = false
}
}
} catch (e: Exception) {
logger.warn("Failed to parse devcontainer.json", e)
Expand Down Expand Up @@ -151,8 +154,4 @@ class CoderPortForwardService(
}
}
}

private fun isValidString(value: Any?): Boolean {
return value != null && value is String
}
}
24 changes: 24 additions & 0 deletions src/main/kotlin/com/coder/jetbrains/services/DevContainerConfig.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package com.coder.jetbrains.services

import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable

@Serializable
data class DevContainerConfig(
@SerialName("portsAttributes")
val portsAttributes: Map<String, PortAttributes> = mapOf(),
@SerialName("otherPortsAttributes")
val otherPortsAttributes: OtherPortsAttributes? = null
)

@Serializable
data class PortAttributes(
@SerialName("onAutoForward")
val onAutoForward: String = ""
)

@Serializable
data class OtherPortsAttributes(
@SerialName("onAutoForward")
val onAutoForward: String = ""
)