Skip to content
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

Allow for globs in probe.resolvers.intellij.repositories config #285

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ trait ProbeExtensions {
path.getFileName.toString
}

def exists: Boolean = {
Files.exists(path)
}

def isFile: Boolean = {
Files.isRegularFile(path)
}
Expand Down
17 changes: 17 additions & 0 deletions core/driver/sources/src/main/resources/reference.conf
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,23 @@ probe {
// ~/.pluginVerifier/ides/IC-[revision]/ directory (if exists). If it does not exist under specified path,
// then ide-probe will download it from one of the 6 official repositories.
//
// If your config contains a pattern pointing to an IntelliJ instance that exists on local filesystem, then you
Copy link
Contributor

Choose a reason for hiding this comment

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

:benissimo: the doc

// can use globs (* widcard characters) as parts of the pattern. One * wildcard character replaces
// one atomic directory on the path. Multiple * wildcards can be used in one pattern. For example - this can
// be useful if you want to use .zip OR installed IntelliJ downloaded earlier by some gradle task:
// intellij.repositories = [
// "file:///"${HOME}"/.gradle/caches/modules-2/files-2.1/com.jetbrains.intellij.idea/ideaIC/[revision]/*/ideaIC-[revision]/"
// ]
// - as you can see - the * wildcard is handy as it replaces a hash that could not be known easily. There could be
// multiple different directories in the * place - but not all of them lead to a valid IntelliJ resource. ide-probe
// deals with such situations and finds the correct path to a valid IntelliJ resource. The pattern must start with
// "file:" if resolving * wildcards should be used.
// Note: example below would work as good as the previous one. Multiple * wildcards in one path are handled properly:
// intellij.repositories = [
// "file:///"${HOME}"/.gradle/*/modules-2/*/*/ideaIC/[revision]/*/ideaIC-[revision]/"
// ]
// Note: you can NOT use the * wildcard in place of the last element of the pattern (as that would lead to ambiguous results).
//
// Note: if you use the `intellij.repositories` config with a value pointing to an installed
// IntelliJ instance directory - then DO NOT use the `probe.intellij.path` config in the same time.
// The `probe.intellij.path` config takes precedence over `intellij.repositories` in such case, so the path
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
package org.virtuslab.ideprobe.dependencies

import java.nio.file.Paths

import scala.annotation.tailrec

import org.virtuslab.ideprobe.IntelliJFixture
import org.virtuslab.ideprobe.config.DependenciesConfig

import org.virtuslab.ideprobe.Extensions.PathExtension

trait IntelliJResolver {
def community: DependencyResolver[IntelliJVersion]
def ultimate: DependencyResolver[IntelliJVersion]
Expand Down Expand Up @@ -37,9 +43,54 @@ case class IntelliJPatternResolver(pattern: String) extends IntelliJResolver {
"version" -> version.releaseOrBuild,
"release" -> version.release.getOrElse("snapshot-release")
)
val replaced = replacements.foldLeft(pattern) { case (path, (pattern, replacement)) =>
val replacedBeforeResolvingGlobs = replacements.foldLeft(pattern) { case (path, (pattern, replacement)) =>
path.replace(s"[$pattern]", replacement)
}
val replaced =
if (replacedBeforeResolvingGlobs.startsWith("file:"))
resolveGlobsInPattern(replacedBeforeResolvingGlobs)
else
replacedBeforeResolvingGlobs

Dependency(replaced)
}

private def resolveGlobsInPattern(pathPattern: String): String =
replaceGlobsWithExistingDirectories(List(pathPattern), pathPattern).head

// Solution below assumes that each * character is used to mark one part of the path (one atomic directory),
// for example: "file:///home/.cache/ides/com.jetbrains.intellij.idea/ideaIC/[revision]/*/ideaIC-[revision]/".
// Wildcard character should NOT be used as the last element of the pattern - to avoid ambiguous results.
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm technically, ambiguous results can also happen when wildcard is somewhere inside the path as well, right?

Maybe sth like:

Suggested change
// Wildcard character should NOT be used as the last element of the pattern - to avoid ambiguous results.
// Wildcard character should NOT be used as the last element of the pattern - to decrease the probability of getting ambiguous results.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ambiguous results can also happen when wildcard is somewhere inside the path as well, right?

No, if the * wildcard is used in replacement of an atomic directory from the path (but not the last part) - then

private def resolveGlobsInPattern(pathPattern: String): String =
    replaceGlobsWithExistingDirectories(List(pathPattern), pathPattern).head
  • will return only one element if pattern is valid (so the result will not be ambiguous)
    or
  • will throw NoSuchElementException if pattern is not valid

Copy link
Contributor

Choose a reason for hiding this comment

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

Huh okay... so let's break down all cases:

  1. zero glob matches: exception regardless of where the glob is located?
  2. exactly one glob match: OK
  3. more than one glob match:
    a. if * is used for the last part of the glob — multiple results returned?
    b. if * is used for other part of the glob — exception thrown?

Copy link
Contributor Author

@LukaszKontowski LukaszKontowski Oct 18, 2022

Choose a reason for hiding this comment

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

Answered in PMs but short answers below:

  1. zero glob matches: exception regardless of where the glob is located?

NoSuchElementException will be thrown (as pattern is not valid - such file does not exist)

  1. exactly one glob match: OK

yes

  1. more than one glob match:
    a. if * is used for the last part of the glob — multiple results returned?

no - NoSuchElementException will be thrown - using * as the last element of the path is not possible, because then the logic would not be able to choose the proper path.
For example - if we would have path endings like below:

.../df364b7662aeb20ca2eafead730380017c395661/ideaIC-223.4884.69-EAP-SNAPSHOT 
.../df364b7662aeb20ca2eafead730380017c395661/ideaIC-223.4884.69-EAP-SNAPSHOT.zip
.../df364b7662aeb20ca2eafead730380017c395661/ideaIC-223.4884.69-EAP-SNAPSHOT.jar
.../df364b7662aeb20ca2eafead730380017c395661/ideaIC-223.4884.69-EAP-SNAPSHOT.pom

where first one is directory and second one - a .zip file - and the pattern ends with * - how can we know, which one should be used?

  1. more than one glob match:
    b. if * is used for other part of the glob — exception thrown?

no - resolveGlobsInPattern will choose the first one by calling .head

// Works only for files in local filesystem.
@tailrec
private def replaceGlobsWithExistingDirectories(paths: List[String], originalPattern: String): List[String] =
if (paths.exists(pathMightBeValidResource(_, originalPattern)))
paths.filter(pathMightBeValidResource(_, originalPattern))
else
replaceGlobsWithExistingDirectories(paths.flatMap(replaceFirstFoundWildcardWithDirectories), originalPattern)

private def pathMightBeValidResource(candidatePath: String, originalPattern: String): Boolean =
!candidatePath.contains('*') && // below - make sure that candidatePath's last path element is same as in pattern
candidatePath.substring(candidatePath.lastIndexOf('/')) == originalPattern.substring(
originalPattern.lastIndexOf('/')
) && Paths.get(candidatePath.replace("file:", "")).exists

private def replaceFirstFoundWildcardWithDirectories(path: String): List[String] = {
def removeLastFileSeparator(path: String): String = if (path.endsWith("/")) path.init else path

val pathUntilWildcard = Paths.get(path.substring(0, path.indexOf('*')).replace("file:", ""))
Copy link
Contributor

Choose a reason for hiding this comment

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

Okay... so a ready library for globs wasn't an adequate solution 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.

I did not manage to test these libraries yet. Wanted to have a working solution first. I'm not sure if we need them here. Suggested docs https://docs.oracle.com/javase/tutorial/essential/io/find.html don't seem helpful to me in this case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, leaving up to your judgement

val stringPathAfterWildcard = path.substring(path.indexOf('*') + 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

can you use split for that? It allows to specify limit of splits.

Copy link
Contributor Author

@LukaszKontowski LukaszKontowski Oct 17, 2022

Choose a reason for hiding this comment

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

Ok, but then I would have to use three vals instead of two:

val Array(stringPathUntilWildcard, stringPathAfterWildcard) = path.split("*", 2)
val pathUntilWildcard = Paths.get(stringPathUntilWildcard.replace("file:", ""))

If you think that would be better than current solution, I can do that. Let me know, what do you think.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, let's do this. It is same number of lines, but 2 substring calls with some index offset are more complex for reading and understanding than a simple split.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a problem connected to replacing String#indexOf with String#split
The String#split(String regex, int limit) method treats the String parameter "*" as a regular expression, which leads to invalid results.

I would suggest to switch back to previous logic:

val pathUntilWildcard = Paths.get(path.substring(0, path.indexOf('*')).replace("file:", ""))
val stringPathAfterWildcard = path.substring(path.indexOf('*') + 1)

to be sure that * wildcard will not be used as a regular expression.

Copy link
Contributor

@lukaszwawrzyk lukaszwawrzyk Oct 19, 2022

Choose a reason for hiding this comment

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

To be sure, just use regex as expected r"\*" or "[*]"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, but the thing is - I don't want java String's method to treat the * as a regular expression. And String#split by default treats the String parameter as a regular expression. That's why I want to stick to current logic, without using split. So that * will never be treated as a regex.


if (pathUntilWildcard.exists) {
pathUntilWildcard
.directChildren()
.map { replaced =>
(if (path.startsWith("file:")) "file:" else "") + removeLastFileSeparator(
replaced.toString
) + stringPathAfterWildcard
}
} else {
Nil
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import org.junit.runners.JUnit4

import org.virtuslab.ideprobe.Config
import org.virtuslab.ideprobe.ConfigFormat
import org.virtuslab.ideprobe.IntelliJFixture
import org.virtuslab.ideprobe.config.DependenciesConfig

@RunWith(classOf[JUnit4])
Expand Down Expand Up @@ -47,6 +48,21 @@ class IntelliJResolverTest extends ConfigFormat {
assertExists(artifactUri)
}

@Test
def resolvesIntelliJPatternWithGlobesUsed(): Unit = {
val probeConfig = IntelliJFixture.readIdeProbeConfig(
Config.fromString(s"""
|probe.resolvers.intellij.repositories = [
| "$mavenRepo/com/*/intellij/*/$mavenArtifact/${mavenVersion.build}/$mavenArtifact-${mavenVersion.build}.zip"
|]
|""".stripMargin),
"probe"
)
val repo = IntelliJResolver.fromConfig(probeConfig.resolvers).head
val artifactUri = repo.resolve(mavenVersion)
assertExists(artifactUri)
}

private def assertExists(dependency: Dependency): Unit = dependency match {
case Dependency.Artifact(uri) =>
assertTrue(s"Resolved invalid artifact: $uri", Files.exists(Paths.get(uri)))
Expand Down