Merge pull request #135 from node-gradle/proxy-settings-enum

docs/faq.md

@@ -166,19 +166,7 @@ task myScript(type: NpmTask) {
 
 # How to use an HTTP or HTTPS proxy?
 
-If your network requires using a proxy to access to the internet, you probably already 
-[configured Gradle to use the proxy](https://docs.gradle.org/current/userguide/build_environment.html#sec:accessing_the_web_via_a_proxy).
-In this case, the plugin will automatically apply the proxy configuration to all `npm` and `yarn` commands.
-
-Note that:
-* This does not work with `npx` since it does not support proxy usage
-* This does work either for all `node` commands. It's the `node` script's responsibility to use the proxy or not
-* For `npm` and `yarn`, it will only work for network requests done directly by the tool (for instance downloading a 
-dependency). This will not work if you run a Node.js script for instance via `npm run`.
-
-To disable this behavior, set `useGradleProxySettings` to `false` in the `node` extension. In this case, the plugin will
-do nothing regarding proxy and you may want to configure it manually, for instance using the `.npmrc` file as 
-explained [here](https://www.devtech101.com/2016/07/21/how-to-set-npm-proxy-settings-in-npmrc/) for `npm`.
+If Gradle is configured to use a proxy, the plugin will automatically configure `yarn` and `npm` to use it, unless it is already configured thanks to the dedicated environment variables. This behavior can be configured. Read more in the [proxy manual](./usage.md#using-a-proxy).
 
 # How do I ignore some files of the `node_modules` directory that are modified by the build and prevent tasks from being up-to-date ?
 

docs/usage.md

@@ -244,7 +244,7 @@ node {
     // according the proxy configuration defined for Gradle
     // Disable this option if you want to configure the proxy for npm or yarn on your own
     // (in the .npmrc file for instance)
-    useGradleProxySettings = true
+    nodeProxySettings = ProxySettings.SMART
 }
 ```
 
@@ -271,13 +271,16 @@ If you would like the plugin to install use a custom version of yarn, you can se
 
 ## Using a Proxy
 
-By default and unless if `useGradleProxySettings` is `false`, the plugin will configure 
-`npm` and `yarn` to get them use the proxy configuration defined in the 
-[Gradle project configuration](https://docs.gradle.org/current/userguide/build_environment.html#sec:accessing_the_web_via_a_proxy).
-This is done by automatically setting the `HTTP_PROXY`, `HTTPS_PROXY` and `NO_PROXY` 
-environment variables when invoking `npm` and `yarn`.
+If your network requires using a proxy to access to the internet, you probably already [configured Gradle to use the proxy](https://docs.gradle.org/current/userguide/build_environment.html#sec:accessing_the_web_via_a_proxy).  In this case, the plugin will by default automatically apply the proxy configuration to all `npm` and `yarn` commands.
 
-Note that `npm` and `yarn` support host exclusion (`NO_PROXY`) variable but
-they do not support host name and port exclusion. In the case some host names and ports
-are defined in the proxy exclusion, the port will be removed. The exclusion will apply to 
-both HTTP and HTTPS protocols. 
+Note that:
+* This is done by automatically setting the `HTTP_PROXY`, `HTTPS_PROXY` and `NO_PROXY` environment variables when invoking `npm` and `yarn`.
+* If at least one of those environment variables is already set, no proxy configuration will be done.
+* This does not work with `npx` since it does not support proxy usage.
+* This does work either for all `node` commands. It's the `node` script's responsibility to use the proxy or not.
+* For `npm` and `yarn`, it will only work for network requests done directly by the tool (for instance downloading a dependency). This will not work if you run a Node.js script for instance via `npm run`.
+* `npm` and `yarn` support host exclusion (`NO_PROXY`) variable but they do not support host name and port exclusion. In the case some host names and ports are defined in the proxy exclusion, the port will be removed. The exclusion will apply to both HTTP and HTTPS protocols.
+
+To disable proxy configuration, set `nodeProxySettings` to `ProxySettings.OFF` in the `node` extension. In this case, the plugin will do nothing regarding the proxy configuration and you may want to configure it manually, for instance using the `.npmrc` file as explained [here](https://www.devtech101.com/2016/07/21/how-to-set-npm-proxy-settings-in-npmrc/) for `npm`.
+
+To force the proxy configuration to be done even if one of the proxy environment variables is already set (i.e. override the existing proxy configuration), set `nodeProxySettings` to `ProxySettings.FORCE` in the `node` extension.

src/main/kotlin/com/github/gradle/node/NodeExtension.kt

@@ -1,5 +1,6 @@
 package com.github.gradle.node
 
+import com.github.gradle.node.npm.proxy.ProxySettings
 import org.gradle.api.Project
 import org.gradle.kotlin.dsl.create
 import org.gradle.kotlin.dsl.getByType
@@ -82,8 +83,9 @@ open class NodeExtension(project: Project) {
      *
      * Disable this option if you want to configure the proxy for npm or yarn on your own
      * (in the .npmrc file for instance)
+     *
      */
-    val useGradleProxySettings = project.objects.property<Boolean>().convention(true)
+    val nodeProxySettings = project.objects.property<ProxySettings>().convention(ProxySettings.SMART)
 
     @Suppress("unused")
     @Deprecated("Deprecated in version 3.0, please use nodeProjectDir now")
@@ -93,6 +95,11 @@ open class NodeExtension(project: Project) {
         distBaseUrl.set("https://nodejs.org/dist")
     }
 
+    @Deprecated("useGradleProxySettings has been replaced with nodeProxySettings",
+        replaceWith = ReplaceWith("nodeProxySettings.set(i)"))
+    fun setUseGradleProxySettings(value: Boolean) {
+        nodeProxySettings.set(if (value) ProxySettings.SMART else ProxySettings.OFF)
+    }
 
     companion object {
         const val NAME = "node"

src/main/kotlin/com/github/gradle/node/NodePlugin.kt

@@ -1,5 +1,6 @@
 package com.github.gradle.node
 
+import com.github.gradle.node.npm.proxy.ProxySettings
 import com.github.gradle.node.npm.task.NpmInstallTask
 import com.github.gradle.node.npm.task.NpmSetupTask
 import com.github.gradle.node.npm.task.NpmTask
@@ -35,13 +36,14 @@ class NodePlugin : Plugin<Project> {
     }
 
     private fun addGlobalTypes() {
-        addGlobalTaskType<NodeTask>()
-        addGlobalTaskType<NpmTask>()
-        addGlobalTaskType<NpxTask>()
-        addGlobalTaskType<YarnTask>()
+        addGlobalType<NodeTask>()
+        addGlobalType<NpmTask>()
+        addGlobalType<NpxTask>()
+        addGlobalType<YarnTask>()
+        addGlobalType<ProxySettings>()
     }
 
-    private inline fun <reified T> addGlobalTaskType() {
+    private inline fun <reified T> addGlobalType() {
         project.extensions.extraProperties[T::class.java.simpleName] = T::class.java
     }
 

src/main/kotlin/com/github/gradle/node/npm/exec/NpmExecRunner.kt

@@ -4,8 +4,8 @@ import com.github.gradle.node.NodeExtension
 import com.github.gradle.node.exec.ExecConfiguration
 import com.github.gradle.node.exec.ExecRunner
 import com.github.gradle.node.exec.NodeExecConfiguration
+import com.github.gradle.node.npm.proxy.NpmProxy
 import com.github.gradle.node.npm.proxy.NpmProxy.Companion.computeNpmProxyEnvironmentVariables
-import com.github.gradle.node.npm.proxy.NpmProxy.Companion.hasProxyConfiguration
 import com.github.gradle.node.util.ProjectApiHelper
 import com.github.gradle.node.util.zip
 import com.github.gradle.node.variant.VariantComputer
@@ -29,8 +29,7 @@ internal abstract class NpmExecRunner {
 
     private fun addProxyEnvironmentVariables(nodeExtension: NodeExtension,
                                              nodeExecConfiguration: NodeExecConfiguration): NodeExecConfiguration {
-        if (nodeExtension.useGradleProxySettings.get()
-                && !hasProxyConfiguration(System.getenv())) {
+        if (NpmProxy.shouldConfigureProxy(System.getenv(), nodeExtension.nodeProxySettings.get())) {
             val npmProxyEnvironmentVariables = computeNpmProxyEnvironmentVariables()
             if (npmProxyEnvironmentVariables.isNotEmpty()) {
                 val environmentVariables =

src/main/kotlin/com/github/gradle/node/npm/proxy/NpmProxy.kt

@@ -28,6 +28,16 @@ internal class NpmProxy {
             return proxyEnvironmentVariables.toMap()
         }
 
+        fun shouldConfigureProxy(env: Map<String, String>, settings: ProxySettings): Boolean {
+            if (settings == ProxySettings.FORCED) {
+                return true
+            } else if (settings == ProxySettings.SMART) {
+                return !hasProxyConfiguration(env)
+            }
+
+            return false
+        }
+
         /**
          * Returns true if the given map of environment variables already has
          * proxy settings configured.

src/main/kotlin/com/github/gradle/node/npm/proxy/ProxySettings.kt

@@ -0,0 +1,23 @@
+package com.github.gradle.node.npm.proxy
+
+/**
+ * @since 3.0
+ */
+enum class ProxySettings {
+    /**
+     * The default, this will set the proxy settings only if there's no configuration
+     * present in the environment variables already.
+     */
+    SMART,
+
+    /**
+     * This will always set the proxy settings, overriding any settings already present.
+     * This might cause incorrect settings.
+     */
+    FORCED,
+
+    /**
+     * Don't set any proxy settings.
+     */
+    OFF
+}

src/main/kotlin/com/github/gradle/node/yarn/exec/YarnExecRunner.kt

@@ -36,8 +36,7 @@ internal abstract class YarnExecRunner {
 
     private fun addNpmProxyEnvironment(nodeExtension: NodeExtension,
                                        nodeExecConfiguration: NodeExecConfiguration): Map<String, String> {
-        if (nodeExtension.useGradleProxySettings.get()
-                && !NpmProxy.hasProxyConfiguration(System.getenv())) {
+        if (NpmProxy.shouldConfigureProxy(System.getenv(), nodeExtension.nodeProxySettings.get())) {
             val npmProxyEnvironmentVariables = NpmProxy.computeNpmProxyEnvironmentVariables()
             if (npmProxyEnvironmentVariables.isNotEmpty()) {
                 return nodeExecConfiguration.environment.plus(npmProxyEnvironmentVariables)

src/test/groovy/com/github/gradle/node/npm/task/NpmInstallTaskTest.groovy

@@ -1,6 +1,7 @@
 package com.github.gradle.node.npm.task
 
 import com.github.gradle.node.npm.proxy.GradleProxyHelper
+import com.github.gradle.node.npm.proxy.ProxySettings
 import com.github.gradle.node.task.AbstractTaskTest
 
 class NpmInstallTaskTest extends AbstractTaskTest {
@@ -32,7 +33,7 @@ class NpmInstallTaskTest extends AbstractTaskTest {
         props.setProperty('os.name', 'Linux')
         GradleProxyHelper.setHttpsProxyHost("my-super-proxy.net")
         GradleProxyHelper.setHttpsProxyPort(11235)
-        nodeExtension.useGradleProxySettings.set(false)
+        nodeExtension.nodeProxySettings.set(ProxySettings.OFF)
 
         def task = project.tasks.getByName("npmInstall")
         mockProjectApiHelperExec(task)

src/test/groovy/com/github/gradle/node/npm/task/NpmTaskTest.groovy

@@ -1,6 +1,7 @@
 package com.github.gradle.node.npm.task
 
 import com.github.gradle.node.npm.proxy.GradleProxyHelper
+import com.github.gradle.node.npm.proxy.ProxySettings
 import com.github.gradle.node.task.AbstractTaskTest
 
 import static com.github.gradle.node.NodeExtension.DEFAULT_NODE_VERSION
@@ -140,7 +141,7 @@ class NpmTaskTest extends AbstractTaskTest {
         props.setProperty('os.name', 'Linux')
         GradleProxyHelper.setHttpsProxyHost("my-super-proxy.net")
         GradleProxyHelper.setHttpsProxyPort(11235)
-        nodeExtension.useGradleProxySettings.set(false)
+        nodeExtension.nodeProxySettings.set(ProxySettings.OFF)
 
         def task = project.tasks.create('simple', NpmTask)
         mockProjectApiHelperExec(task)

src/test/groovy/com/github/gradle/node/yarn/task/YarnSetupTaskTest.groovy

@@ -1,6 +1,7 @@
 package com.github.gradle.node.yarn.task
 
 import com.github.gradle.node.npm.proxy.GradleProxyHelper
+import com.github.gradle.node.npm.proxy.ProxySettings
 import com.github.gradle.node.task.AbstractTaskTest
 
 class YarnSetupTaskTest extends AbstractTaskTest {
@@ -34,7 +35,7 @@ class YarnSetupTaskTest extends AbstractTaskTest {
         given:
         GradleProxyHelper.setHttpProxyHost("my-proxy")
         GradleProxyHelper.setHttpProxyPort(80)
-        nodeExtension.useGradleProxySettings.set(false)
+        nodeExtension.nodeProxySettings.set(ProxySettings.OFF)
 
         def task = project.tasks.create('simple', YarnSetupTask)
         mockProjectApiHelperExec(task)

src/test/groovy/com/github/gradle/node/yarn/task/YarnTaskTest.groovy

@@ -1,8 +1,8 @@
 package com.github.gradle.node.yarn.task
 
 import com.github.gradle.node.npm.proxy.GradleProxyHelper
+import com.github.gradle.node.npm.proxy.ProxySettings
 import com.github.gradle.node.task.AbstractTaskTest
-import org.gradle.process.ExecSpec
 
 class YarnTaskTest extends AbstractTaskTest {
     def cleanup() {
@@ -86,7 +86,7 @@ class YarnTaskTest extends AbstractTaskTest {
         GradleProxyHelper.setHttpsProxyPort(443)
         GradleProxyHelper.setHttpsProxyUser("me")
         GradleProxyHelper.setHttpsProxyPassword("password")
-        nodeExtension.useGradleProxySettings.set(false)
+        nodeExtension.nodeProxySettings.set(ProxySettings.OFF)
 
         def task = project.tasks.create('simple', YarnTask)
         mockProjectApiHelperExec(task)

src/test/kotlin/com/github/gradle/node/npm/proxy/NpmProxyTest.kt

@@ -11,6 +11,18 @@ class NpmProxyTest {
         GradleProxyHelper.resetProxy()
     }
 
+    @Test
+    internal fun verifyProxyConfigurationSettings() {
+        assertThat(NpmProxy.shouldConfigureProxy(emptyMap(), ProxySettings.FORCED)).isTrue
+        assertThat(NpmProxy.shouldConfigureProxy(emptyMap(), ProxySettings.OFF)).isFalse
+
+        // No proxy settings present, should be set
+        assertThat(NpmProxy.shouldConfigureProxy(emptyMap(), ProxySettings.SMART)).isTrue
+
+        // Proxy settings present, SMART shouldn't configure.
+        assertThat(NpmProxy.shouldConfigureProxy(mapOf(("HTTP_PROXY" to "")), ProxySettings.SMART)).isFalse
+    }
+
     @Test
     internal fun shouldComputeTheProxyArgsWhenNoProxyIsConfigured() {
         val result = NpmProxy.computeNpmProxyEnvironmentVariables()

src/test/resources/fixtures/kotlin/build.gradle.kts

@@ -1,3 +1,4 @@
+import com.github.gradle.node.npm.proxy.ProxySettings
 import com.github.gradle.node.npm.task.NpmTask
 import com.github.gradle.node.npm.task.NpxTask
 import com.github.gradle.node.task.NodeTask
@@ -25,7 +26,7 @@ node {
     npmWorkDir.set(file("${project.projectDir}/.cache/npm"))
     yarnWorkDir.set(file("${project.projectDir}/.cache/yarn"))
     nodeProjectDir.set(file("${project.projectDir}"))
-    useGradleProxySettings.set(true)
+    nodeProxySettings.set(ProxySettings.SMART)
 }
 
 tasks.npmInstall {