From cf5679333968362df89f458899d6c05bbfec57cd Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Thu, 17 Mar 2022 14:07:13 +0100 Subject: [PATCH 1/6] Npm: Only enrich package data if necessary To avoid unnecessary remote queries, only do so if existing data is incomplete. Signed-off-by: Sebastian Schuberth --- analyzer/src/main/kotlin/managers/Npm.kt | 53 +++++++++++++----------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/analyzer/src/main/kotlin/managers/Npm.kt b/analyzer/src/main/kotlin/managers/Npm.kt index 1f4bcca375a4b..c48714afd3a37 100644 --- a/analyzer/src/main/kotlin/managers/Npm.kt +++ b/analyzer/src/main/kotlin/managers/Npm.kt @@ -236,36 +236,41 @@ open class Npm( val vcsFromDirectory = VersionControlSystem.forDirectory(realPackageDir)?.getInfo().orEmpty() vcsFromPackage = vcsFromPackage.merge(vcsFromDirectory) } else { - // Download package info from registry.npmjs.org. - // TODO: check if unpkg.com can be used as a fallback in case npmjs.org is down. - val encodedName = if (rawName.startsWith("@")) { - "@${URLEncoder.encode(rawName.substringAfter('@'), "UTF-8")}" - } else { - rawName - } + val hasIncompleteData = description.isEmpty() || homepageUrl.isEmpty() || downloadUrl.isEmpty() + || hash == Hash.NONE || vcsFromPackage == VcsInfo.EMPTY + + if (hasIncompleteData) { + // Download package info from registry.npmjs.org. + // TODO: check if unpkg.com can be used as a fallback in case npmjs.org is down. + val encodedName = if (rawName.startsWith("@")) { + "@${URLEncoder.encode(rawName.substringAfter('@'), "UTF-8")}" + } else { + rawName + } - log.debug { "Resolving the package info for '${id.toCoordinates()}' via NPM registry." } + log.debug { "Resolving the package info for '${id.toCoordinates()}' via NPM registry." } - OkHttpClientHelper.downloadText("$npmRegistry/$encodedName/$version").onSuccess { - val versionInfo = jsonMapper.readTree(it) + OkHttpClientHelper.downloadText("$npmRegistry/$encodedName/$version").onSuccess { + val versionInfo = jsonMapper.readTree(it) - description = versionInfo["description"].textValueOrEmpty() - homepageUrl = versionInfo["homepage"].textValueOrEmpty() + description = versionInfo["description"].textValueOrEmpty() + homepageUrl = versionInfo["homepage"].textValueOrEmpty() - versionInfo["dist"]?.let { dist -> - downloadUrl = dist["tarball"].textValueOrEmpty() - // Work around the issue described at - // https://npm.community/t/some-packages-have-dist-tarball-as-http-and-not-https/285/19. - .replace("http://registry.npmjs.org/", "https://registry.npmjs.org/") + versionInfo["dist"]?.let { dist -> + downloadUrl = dist["tarball"].textValueOrEmpty() + // Work around the issue described at + // https://npm.community/t/some-packages-have-dist-tarball-as-http-and-not-https/285/19. + .replace("http://registry.npmjs.org/", "https://registry.npmjs.org/") - hash = Hash.create(dist["shasum"].textValueOrEmpty()) - } + hash = Hash.create(dist["shasum"].textValueOrEmpty()) + } - vcsFromPackage = parseVcsInfo(versionInfo) - }.onFailure { - log.info { - "Could not retrieve package information for '$encodedName' from NPM registry $npmRegistry: " + - it.message + vcsFromPackage = parseVcsInfo(versionInfo) + }.onFailure { + log.info { + "Could not retrieve package information for '$encodedName' from NPM registry " + + "$npmRegistry: ${it.message}" + } } } } From 2c32a8aca6a0f62a0fe7bc752d6f9cd55c489921 Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Fri, 18 Mar 2022 08:52:18 +0100 Subject: [PATCH 2/6] Npm: Move fixing up the NPM registry URL protocol to a central place To be on the safe side, do this in any case in common code. Signed-off-by: Sebastian Schuberth --- analyzer/src/main/kotlin/managers/Npm.kt | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/analyzer/src/main/kotlin/managers/Npm.kt b/analyzer/src/main/kotlin/managers/Npm.kt index c48714afd3a37..227415f62dbf4 100644 --- a/analyzer/src/main/kotlin/managers/Npm.kt +++ b/analyzer/src/main/kotlin/managers/Npm.kt @@ -258,10 +258,6 @@ open class Npm( versionInfo["dist"]?.let { dist -> downloadUrl = dist["tarball"].textValueOrEmpty() - // Work around the issue described at - // https://npm.community/t/some-packages-have-dist-tarball-as-http-and-not-https/285/19. - .replace("http://registry.npmjs.org/", "https://registry.npmjs.org/") - hash = Hash.create(dist["shasum"].textValueOrEmpty()) } @@ -275,6 +271,11 @@ open class Npm( } } + downloadUrl = downloadUrl + // Work around the issue described at + // https://npm.community/t/some-packages-have-dist-tarball-as-http-and-not-https/285/19. + .replace("http://registry.npmjs.org/", "https://registry.npmjs.org/") + val vcsFromDownloadUrl = VcsHost.toVcsInfo(downloadUrl) if (vcsFromDownloadUrl.url != downloadUrl) { vcsFromPackage = vcsFromPackage.merge(vcsFromDownloadUrl) From ea3460fb1d6ff96aaf5891ffd54df76d9efa40f1 Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Fri, 18 Mar 2022 08:54:31 +0100 Subject: [PATCH 3/6] Npm: Only overwrite empty package data Do not unconditionally overwrite existing package data, but only if it is empty. Signed-off-by: Sebastian Schuberth --- analyzer/src/main/kotlin/managers/Npm.kt | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/analyzer/src/main/kotlin/managers/Npm.kt b/analyzer/src/main/kotlin/managers/Npm.kt index 227415f62dbf4..c7abb54c09376 100644 --- a/analyzer/src/main/kotlin/managers/Npm.kt +++ b/analyzer/src/main/kotlin/managers/Npm.kt @@ -253,12 +253,14 @@ open class Npm( OkHttpClientHelper.downloadText("$npmRegistry/$encodedName/$version").onSuccess { val versionInfo = jsonMapper.readTree(it) - description = versionInfo["description"].textValueOrEmpty() - homepageUrl = versionInfo["homepage"].textValueOrEmpty() + if (description.isEmpty()) description = versionInfo["description"].textValueOrEmpty() + if (homepageUrl.isEmpty()) homepageUrl = versionInfo["homepage"].textValueOrEmpty() versionInfo["dist"]?.let { dist -> - downloadUrl = dist["tarball"].textValueOrEmpty() - hash = Hash.create(dist["shasum"].textValueOrEmpty()) + if (downloadUrl.isEmpty() || hash == Hash.NONE) { + downloadUrl = dist["tarball"].textValueOrEmpty() + hash = Hash.create(dist["shasum"].textValueOrEmpty()) + } } vcsFromPackage = parseVcsInfo(versionInfo) From 0c8cd71e6ef741a3816fd17fb165c55ad27b5bec Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Wed, 16 Mar 2022 17:30:02 +0100 Subject: [PATCH 4/6] analyzer: Factor out Npm/Yarn CommandLineTools to static objects This is a preparation for an upcoming change that needs to run the `npm` command line tool from a static context. As a side effect when running `ort requirements`, `NpmCli` and `YarnCli` are now listed in the "Other tool" section instead of the "PackageManager" section. Signed-off-by: Sebastian Schuberth --- analyzer/src/main/kotlin/managers/Npm.kt | 18 ++++++++++-------- analyzer/src/main/kotlin/managers/Yarn.kt | 13 ++++++++----- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/analyzer/src/main/kotlin/managers/Npm.kt b/analyzer/src/main/kotlin/managers/Npm.kt index c7abb54c09376..116a32048fe34 100644 --- a/analyzer/src/main/kotlin/managers/Npm.kt +++ b/analyzer/src/main/kotlin/managers/Npm.kt @@ -78,6 +78,12 @@ import org.ossreviewtoolkit.utils.spdx.SpdxConstants const val PUBLIC_NPM_REGISTRY = "https://registry.npmjs.org" +object NpmCli : CommandLineTool { + override fun command(workingDir: File?) = if (Os.isWindows) "npm.cmd" else "npm" + + override fun getVersionRequirement() = Requirement.buildNPM("5.7.* - 7.20.*") +} + /** * The [Node package manager](https://www.npmjs.com/) for JavaScript. */ @@ -86,7 +92,7 @@ open class Npm( analysisRoot: File, analyzerConfig: AnalyzerConfiguration, repoConfig: RepositoryConfiguration -) : PackageManager(name, analysisRoot, analyzerConfig, repoConfig), CommandLineTool { +) : PackageManager(name, analysisRoot, analyzerConfig, repoConfig) { class Factory : AbstractPackageManagerFactory("NPM") { override val globsForDefinitionFiles = listOf("package.json") @@ -344,16 +350,12 @@ open class Npm( protected open fun hasLockFile(projectDir: File) = hasNpmLockFile(projectDir) - override fun command(workingDir: File?) = if (Os.isWindows) "npm.cmd" else "npm" - - override fun getVersionRequirement(): Requirement = Requirement.buildNPM("5.7.* - 7.20.*") - override fun mapDefinitionFiles(definitionFiles: List) = mapDefinitionFilesForNpm(definitionFiles).toList() override fun beforeResolution(definitionFiles: List) { // We do not actually depend on any features specific to an NPM version, but we still want to stick to a // fixed minor version to be sure to get consistent results. - checkVersion() + NpmCli.checkVersion() } override fun afterResolution(definitionFiles: List) { @@ -667,9 +669,9 @@ open class Npm( // Install all NPM dependencies to enable NPM to list dependencies. val process = if (hasLockFile(workingDir) && this::class.java == Npm::class.java) { - run(workingDir, "ci", *installParameters) + NpmCli.run(workingDir, "ci", *installParameters) } else { - run(workingDir, "install", *installParameters) + NpmCli.run(workingDir, "install", *installParameters) } // TODO: Capture warnings from npm output, e.g. "Unsupported platform" which happens for fsevents on all diff --git a/analyzer/src/main/kotlin/managers/Yarn.kt b/analyzer/src/main/kotlin/managers/Yarn.kt index 79e535e29e7e4..2a1c1964cc0df 100644 --- a/analyzer/src/main/kotlin/managers/Yarn.kt +++ b/analyzer/src/main/kotlin/managers/Yarn.kt @@ -28,8 +28,15 @@ import org.ossreviewtoolkit.analyzer.managers.utils.hasYarnLockFile import org.ossreviewtoolkit.analyzer.managers.utils.mapDefinitionFilesForYarn import org.ossreviewtoolkit.model.config.AnalyzerConfiguration import org.ossreviewtoolkit.model.config.RepositoryConfiguration +import org.ossreviewtoolkit.utils.common.CommandLineTool import org.ossreviewtoolkit.utils.common.Os +object YarnCli : CommandLineTool { + override fun command(workingDir: File?) = if (Os.isWindows) "yarn.cmd" else "yarn" + + override fun getVersionRequirement() = Requirement.buildNPM("1.3.* - 1.22.*") +} + /** * The [Yarn](https://classic.yarnpkg.com/) package manager for JavaScript. */ @@ -53,14 +60,10 @@ class Yarn( override fun hasLockFile(projectDir: File) = hasYarnLockFile(projectDir) - override fun command(workingDir: File?) = if (Os.isWindows) "yarn.cmd" else "yarn" - - override fun getVersionRequirement(): Requirement = Requirement.buildNPM("1.3.* - 1.22.*") - override fun mapDefinitionFiles(definitionFiles: List) = mapDefinitionFilesForYarn(definitionFiles).toList() override fun beforeResolution(definitionFiles: List) = // We do not actually depend on any features specific to a Yarn version, but we still want to stick to a // fixed minor version to be sure to get consistent results. - checkVersion() + YarnCli.checkVersion() } From 68adcc8c3e45f7ca14dd22fdc8f4e68c6f024ef8 Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Wed, 16 Mar 2022 18:33:49 +0100 Subject: [PATCH 5/6] Npm: Fix quering of package details from private / scoped registries Bump the NPM version requirement from 5.7.* to 6.* in order to be able to use the `npm view` command to query details about a package from remote registries. This lifts two severe restrictions: - The original code was only able to query package details from the public NPM registry [1]. Private registries were not supported at all. - Custom registries defined for different NPM scopes were not supported. As a bonus, `npm view` seems to return more details than querying the registry API directly even for the public NPM registry. The downside is that performance is worse now, using `npm view` seems to be roughly 4 times slower (on Linux); the command unfortunately does not support batching. However, correctness should be more important than performance here, and a future improvement could parallelize the `parsePackage()` calls in general. [1]: https://registry.npmjs.org Signed-off-by: Sebastian Schuberth --- .../external/git-repo-expected-output.yml | 2 +- .../synthetic/npm-babel-expected-output.yml | 2 +- .../synthetic/npm-expected-output.yml | 2 +- .../synthetic/yarn-expected-output.yml | 2 +- analyzer/src/main/kotlin/managers/Npm.kt | 72 ++++--------------- .../managers/utils/NpmDependencyHandler.kt | 7 +- .../utils/NpmDependencyHandlerTest.kt | 5 +- 7 files changed, 22 insertions(+), 70 deletions(-) diff --git a/analyzer/src/funTest/assets/projects/external/git-repo-expected-output.yml b/analyzer/src/funTest/assets/projects/external/git-repo-expected-output.yml index 7a8d2fc916b0e..fd75939021c92 100644 --- a/analyzer/src/funTest/assets/projects/external/git-repo-expected-output.yml +++ b/analyzer/src/funTest/assets/projects/external/git-repo-expected-output.yml @@ -4288,7 +4288,7 @@ analyzer: declared_licenses_processed: spdx_expression: "MIT" description: "TypeScript definitions for Node.js" - homepage_url: "" + homepage_url: "https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/node" binary_artifact: url: "" hash: diff --git a/analyzer/src/funTest/assets/projects/synthetic/npm-babel-expected-output.yml b/analyzer/src/funTest/assets/projects/synthetic/npm-babel-expected-output.yml index d2fff728ba512..5eb38feca5767 100644 --- a/analyzer/src/funTest/assets/projects/synthetic/npm-babel-expected-output.yml +++ b/analyzer/src/funTest/assets/projects/synthetic/npm-babel-expected-output.yml @@ -5056,7 +5056,7 @@ packages: description: "Returns `true` if the given string looks like a glob pattern or an\ \ extglob pattern. This makes it easy to create code that only uses external modules\ \ like node-glob when necessary, resulting in much faster code execution and initialization\ - \ time, and a bet" + \ time, and a better user experience." homepage_url: "https://github.com/jonschlinkert/is-glob" binary_artifact: url: "" diff --git a/analyzer/src/funTest/assets/projects/synthetic/npm-expected-output.yml b/analyzer/src/funTest/assets/projects/synthetic/npm-expected-output.yml index dc4d73155d009..0f470d22afd5f 100644 --- a/analyzer/src/funTest/assets/projects/synthetic/npm-expected-output.yml +++ b/analyzer/src/funTest/assets/projects/synthetic/npm-expected-output.yml @@ -1165,7 +1165,7 @@ packages: declared_licenses_processed: spdx_expression: "MIT" description: "TypeScript definitions for Node.js" - homepage_url: "" + homepage_url: "https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/node" binary_artifact: url: "" hash: diff --git a/analyzer/src/funTest/assets/projects/synthetic/yarn-expected-output.yml b/analyzer/src/funTest/assets/projects/synthetic/yarn-expected-output.yml index 1ece70b5c0c1e..37faeefa047ce 100644 --- a/analyzer/src/funTest/assets/projects/synthetic/yarn-expected-output.yml +++ b/analyzer/src/funTest/assets/projects/synthetic/yarn-expected-output.yml @@ -1134,7 +1134,7 @@ packages: declared_licenses_processed: spdx_expression: "MIT" description: "TypeScript definitions for Node.js" - homepage_url: "" + homepage_url: "https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/node" binary_artifact: url: "" hash: diff --git a/analyzer/src/main/kotlin/managers/Npm.kt b/analyzer/src/main/kotlin/managers/Npm.kt index 116a32048fe34..a73c5ea2b9d2f 100644 --- a/analyzer/src/main/kotlin/managers/Npm.kt +++ b/analyzer/src/main/kotlin/managers/Npm.kt @@ -28,7 +28,6 @@ import com.vdurmont.semver4j.Requirement import java.io.File import java.io.IOException -import java.net.URLEncoder import java.util.SortedSet import org.ossreviewtoolkit.analyzer.AbstractPackageManagerFactory @@ -39,8 +38,6 @@ import org.ossreviewtoolkit.analyzer.managers.utils.NpmModuleInfo import org.ossreviewtoolkit.analyzer.managers.utils.expandNpmShortcutUrl import org.ossreviewtoolkit.analyzer.managers.utils.hasNpmLockFile import org.ossreviewtoolkit.analyzer.managers.utils.mapDefinitionFilesForNpm -import org.ossreviewtoolkit.analyzer.managers.utils.readProxySettingsFromNpmRc -import org.ossreviewtoolkit.analyzer.managers.utils.readRegistryFromNpmRc import org.ossreviewtoolkit.analyzer.parseAuthorString import org.ossreviewtoolkit.downloader.VcsHost import org.ossreviewtoolkit.downloader.VersionControlSystem @@ -71,17 +68,13 @@ import org.ossreviewtoolkit.utils.common.realFile import org.ossreviewtoolkit.utils.common.stashDirectories import org.ossreviewtoolkit.utils.common.textValueOrEmpty import org.ossreviewtoolkit.utils.common.withoutPrefix -import org.ossreviewtoolkit.utils.core.OkHttpClientHelper -import org.ossreviewtoolkit.utils.core.installAuthenticatorAndProxySelector import org.ossreviewtoolkit.utils.core.log import org.ossreviewtoolkit.utils.spdx.SpdxConstants -const val PUBLIC_NPM_REGISTRY = "https://registry.npmjs.org" - object NpmCli : CommandLineTool { override fun command(workingDir: File?) = if (Os.isWindows) "npm.cmd" else "npm" - override fun getVersionRequirement() = Requirement.buildNPM("5.7.* - 7.20.*") + override fun getVersionRequirement() = Requirement.buildNPM("6.* - 7.20.*") } /** @@ -196,10 +189,10 @@ open class Npm( /** * Construct a [Package] by parsing its _package.json_ file and - if applicable - querying additional - * content from the [npmRegistry]. Result is a [Pair] with the raw identifier and the new package. + * content via the `npm view` command. The result is a [Pair] with the raw identifier and the new package. */ @Suppress("HttpUrlsUsage") - internal fun parsePackage(packageFile: File, npmRegistry: String): Pair { + internal fun parsePackage(packageFile: File): Pair { val packageDir = packageFile.parentFile log.debug { "Found a 'package.json' file in '$packageDir'." } @@ -246,36 +239,20 @@ open class Npm( || hash == Hash.NONE || vcsFromPackage == VcsInfo.EMPTY if (hasIncompleteData) { - // Download package info from registry.npmjs.org. - // TODO: check if unpkg.com can be used as a fallback in case npmjs.org is down. - val encodedName = if (rawName.startsWith("@")) { - "@${URLEncoder.encode(rawName.substringAfter('@'), "UTF-8")}" - } else { - rawName - } - - log.debug { "Resolving the package info for '${id.toCoordinates()}' via NPM registry." } - - OkHttpClientHelper.downloadText("$npmRegistry/$encodedName/$version").onSuccess { - val versionInfo = jsonMapper.readTree(it) + val process = NpmCli.run("view", "--json", "$rawName@$version") + val view = jsonMapper.readTree(process.stdoutFile) - if (description.isEmpty()) description = versionInfo["description"].textValueOrEmpty() - if (homepageUrl.isEmpty()) homepageUrl = versionInfo["homepage"].textValueOrEmpty() + if (description.isEmpty()) description = view["description"].textValueOrEmpty() + if (homepageUrl.isEmpty()) homepageUrl = view["homepage"].textValueOrEmpty() - versionInfo["dist"]?.let { dist -> - if (downloadUrl.isEmpty() || hash == Hash.NONE) { - downloadUrl = dist["tarball"].textValueOrEmpty() - hash = Hash.create(dist["shasum"].textValueOrEmpty()) - } - } - - vcsFromPackage = parseVcsInfo(versionInfo) - }.onFailure { - log.info { - "Could not retrieve package information for '$encodedName' from NPM registry " + - "$npmRegistry: ${it.message}" + view["dist"]?.let { dist -> + if (downloadUrl.isEmpty() || hash == Hash.NONE) { + downloadUrl = dist["tarball"].textValueOrEmpty() + hash = Hash.create(dist["shasum"].textValueOrEmpty()) } } + + vcsFromPackage = parseVcsInfo(view) } } @@ -325,23 +302,8 @@ open class Npm( } } - private val ortProxySelector = installAuthenticatorAndProxySelector() - - private val npmRegistry: String - - init { - val npmRcFile = Os.userHomeDirectory.resolve(".npmrc") - npmRegistry = if (npmRcFile.isFile) { - val npmRcContent = npmRcFile.readText() - ortProxySelector.addProxies(managerName, readProxySettingsFromNpmRc(npmRcContent)) - readRegistryFromNpmRc(npmRcContent) ?: PUBLIC_NPM_REGISTRY - } else { - PUBLIC_NPM_REGISTRY - } - } - private val graphBuilder: DependencyGraphBuilder = - DependencyGraphBuilder(NpmDependencyHandler(npmRegistry)) + DependencyGraphBuilder(NpmDependencyHandler()) /** * Array of parameters passed to the install command when installing dependencies. @@ -358,10 +320,6 @@ open class Npm( NpmCli.checkVersion() } - override fun afterResolution(definitionFiles: List) { - ortProxySelector.removeProxyOrigin(managerName) - } - override fun createPackageManagerResult(projectResults: Map>) = PackageManagerResult(projectResults, graphBuilder.build(), graphBuilder.packages()) @@ -425,7 +383,7 @@ open class Npm( nodeModulesDir.walk().filter { it.name == "package.json" && isValidNodeModulesDirectory(nodeModulesDir, nodeModulesDirForPackageJson(it)) }.forEach { file -> - val (id, pkg) = parsePackage(file, npmRegistry) + val (id, pkg) = parsePackage(file) packages[id] = pkg } diff --git a/analyzer/src/main/kotlin/managers/utils/NpmDependencyHandler.kt b/analyzer/src/main/kotlin/managers/utils/NpmDependencyHandler.kt index c569c5a176e66..59c99390a703f 100644 --- a/analyzer/src/main/kotlin/managers/utils/NpmDependencyHandler.kt +++ b/analyzer/src/main/kotlin/managers/utils/NpmDependencyHandler.kt @@ -49,10 +49,7 @@ data class NpmModuleInfo( /** * A specialized [DependencyHandler] implementation for NPM. */ -class NpmDependencyHandler( - /** The URL for queries to the NPM registry. */ - private val npmRegistryUrl: String -) : DependencyHandler { +class NpmDependencyHandler : DependencyHandler { override fun identifierFor(dependency: NpmModuleInfo): Identifier = dependency.id override fun dependenciesFor(dependency: NpmModuleInfo): Collection = dependency.dependencies @@ -60,5 +57,5 @@ class NpmDependencyHandler( override fun linkageFor(dependency: NpmModuleInfo): PackageLinkage = PackageLinkage.DYNAMIC override fun createPackage(dependency: NpmModuleInfo, issues: MutableList): Package = - Npm.parsePackage(dependency.packageFile, npmRegistryUrl).second + Npm.parsePackage(dependency.packageFile).second } diff --git a/analyzer/src/test/kotlin/managers/utils/NpmDependencyHandlerTest.kt b/analyzer/src/test/kotlin/managers/utils/NpmDependencyHandlerTest.kt index 7b2c248ae42ab..97a4c081913ad 100644 --- a/analyzer/src/test/kotlin/managers/utils/NpmDependencyHandlerTest.kt +++ b/analyzer/src/test/kotlin/managers/utils/NpmDependencyHandlerTest.kt @@ -78,9 +78,6 @@ class NpmDependencyHandlerTest : StringSpec({ } }) -/** The URI to be used for queries to the NPM registry. */ -const val NPM_REGISTRY_URL = "https://npm-test.example.org" - /** * Construct a test identifier with the given [name]. */ @@ -100,4 +97,4 @@ private fun createModuleInfo( /** * Creates an [NpmDependencyHandler] instance to be used by test cases. */ -private fun createHandler() = NpmDependencyHandler(NPM_REGISTRY_URL) +private fun createHandler() = NpmDependencyHandler() From 25f17340b8b58a7e0fa22f92f67d3e917198a185 Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Fri, 18 Mar 2022 09:18:04 +0100 Subject: [PATCH 6/6] NodeSupport: Remove the now unused `read*FromNpmRc()` functions Signed-off-by: Sebastian Schuberth --- .../main/kotlin/managers/utils/NodeSupport.kt | 43 ----------- .../kotlin/managers/utils/NodeSupportTest.kt | 77 ------------------- 2 files changed, 120 deletions(-) diff --git a/analyzer/src/main/kotlin/managers/utils/NodeSupport.kt b/analyzer/src/main/kotlin/managers/utils/NodeSupport.kt index b202b181d7fae..1d920ff97a4d3 100644 --- a/analyzer/src/main/kotlin/managers/utils/NodeSupport.kt +++ b/analyzer/src/main/kotlin/managers/utils/NodeSupport.kt @@ -31,9 +31,6 @@ import java.nio.file.PathMatcher import org.ossreviewtoolkit.model.readJsonFile import org.ossreviewtoolkit.utils.common.collectMessagesAsString import org.ossreviewtoolkit.utils.common.toUri -import org.ossreviewtoolkit.utils.core.AuthenticatedProxy -import org.ossreviewtoolkit.utils.core.ProtocolProxyMap -import org.ossreviewtoolkit.utils.core.determineProxyFromURL import org.ossreviewtoolkit.utils.core.log import org.ossreviewtoolkit.utils.core.showStackTrace @@ -112,46 +109,6 @@ fun expandNpmShortcutUrl(url: String): String { } } -/** - * Return all proxies defined in the provided [NPM configuration][npmRc]. - */ -fun readProxySettingsFromNpmRc(npmRc: String): ProtocolProxyMap { - val map = mutableMapOf>() - - npmRc.lines().forEach { line -> - val keyAndValue = line.split('=', limit = 2).map { it.trim() } - if (keyAndValue.size != 2) return@forEach - - val (key, value) = keyAndValue - when (key) { - "proxy" -> determineProxyFromURL(value)?.let { - map.getOrPut("http") { mutableListOf() } += it - } - - "https-proxy" -> determineProxyFromURL(value)?.let { - map.getOrPut("https") { mutableListOf() } += it - } - } - } - - return map -} - -/** - * Return the npm registry defined in the provided [NPM configuration][npmRc] or null. - */ -fun readRegistryFromNpmRc(npmRc: String): String? { - npmRc.lines().forEach { line -> - val keyAndValue = line.split('=', limit = 2).map { it.trim() } - if (keyAndValue.size != 2) return@forEach - - val (key, value) = keyAndValue - if (key == "registry") return value - } - - return null -} - private val NPM_LOCK_FILES = listOf("npm-shrinkwrap.json", "package-lock.json") private val YARN_LOCK_FILES = listOf("yarn.lock") diff --git a/analyzer/src/test/kotlin/managers/utils/NodeSupportTest.kt b/analyzer/src/test/kotlin/managers/utils/NodeSupportTest.kt index 3c00f3a574e08..6cda8246cb88c 100644 --- a/analyzer/src/test/kotlin/managers/utils/NodeSupportTest.kt +++ b/analyzer/src/test/kotlin/managers/utils/NodeSupportTest.kt @@ -26,17 +26,13 @@ import io.kotest.inspectors.forAll import io.kotest.matchers.collections.beEmpty import io.kotest.matchers.collections.containExactly import io.kotest.matchers.collections.containExactlyInAnyOrder -import io.kotest.matchers.maps.beEmpty as beEmptyMap -import io.kotest.matchers.maps.containExactly as containExactlyEntries import io.kotest.matchers.should import io.kotest.matchers.shouldBe import java.io.File import org.ossreviewtoolkit.utils.common.safeMkdirs -import org.ossreviewtoolkit.utils.core.ProtocolProxyMap import org.ossreviewtoolkit.utils.test.createTestTempDir -import org.ossreviewtoolkit.utils.test.toGenericString class NodeSupportTest : WordSpec() { companion object { @@ -205,79 +201,6 @@ class NodeSupportTest : WordSpec() { } } } - - "readProxySettingFromNpmRc" should { - "properly read proxy configuration" { - fun ProtocolProxyMap.mapSingleValuesToString() = - mapValues { (_, proxies) -> - val (proxy, authentication) = proxies.single() - listOfNotNull( - proxy.toGenericString(), - authentication?.userName, - authentication?.password?.let { String(it) } - ) - } - - readProxySettingsFromNpmRc(""" - proxy=http://user:password@host.tld:3129/ - https-proxy=http://user:password@host.tld:3129/ - """.trimIndent() - ).mapSingleValuesToString() should containExactlyEntries( - "http" to listOf("HTTP @ host.tld:3129", "user", "password"), - "https" to listOf("HTTP @ host.tld:3129", "user", "password") - ) - - readProxySettingsFromNpmRc(""" - proxy=http://user:password@host.tld - https-proxy=http://user:password@host.tld - """.trimIndent() - ).mapSingleValuesToString() should containExactlyEntries( - "http" to listOf("HTTP @ host.tld:8080", "user", "password"), - "https" to listOf("HTTP @ host.tld:8080", "user", "password") - ) - - readProxySettingsFromNpmRc(""" - proxy=user:password@host.tld - https-proxy=user:password@host.tld - """.trimIndent() - ).mapSingleValuesToString() should containExactlyEntries( - "http" to listOf("HTTP @ host.tld:8080", "user", "password"), - "https" to listOf("HTTP @ host.tld:8080", "user", "password") - ) - - readProxySettingsFromNpmRc(""" - proxy=host.tld - https-proxy=host.tld - """.trimIndent() - ).mapSingleValuesToString() should containExactlyEntries( - "http" to listOf("HTTP @ host.tld:8080"), - "https" to listOf("HTTP @ host.tld:8080") - ) - } - - "ignore non-proxy URLs" { - readProxySettingsFromNpmRc(""" - registry=http://my.artifactory.com/artifactory/api/npm/npm-virtual - """.trimIndent() - ) should beEmptyMap() - } - } - - "readRegistryFromNpmRc" should { - "properly read registry configuration" { - readRegistryFromNpmRc(""" - registry=http://my.artifactory.com/artifactory/api/npm/npm-virtual - """.trimIndent() - ) shouldBe "http://my.artifactory.com/artifactory/api/npm/npm-virtual" - } - - "return null when no registry is defined" { - readRegistryFromNpmRc(""" - - """.trimIndent() - ) shouldBe null - } - } } private lateinit var tempDir: File