Commonjava/indy

Implied repos should replace property placeholders

Closed this issue · 7 comments

I have come across a remote repository created by implied-repos add-on with this configuration

{
  "type" : "remote",
  "key" : "remote:i-oracle-repository",
  "description" : "Implicitly created repo for: Oracle Repository (oracle.repository) from repository declaration in POM: org.apache.openjpa:openjpa-persistence-jdbc:2.4.0",
  "metadata" : {
    "implied_by_stores" : "{\n  \"items\" : [ \"remote:central\" ]\n}",
    "changelog" : "Disabling indefinitely via UI"
  },
  "disabled" : true,
  "port" : 0,
  "doctype" : "remote",
  "name" : "i-oracle-repository",
  "path_style" : "plain",
  "allow_snapshots" : false,
  "allow_releases" : true,
  "url" : "${oracle.maven.repo}",
  "timeout_seconds" : 5,
  "nfc_timeout_seconds" : 0,
  "is_passthrough" : false,
  "cache_timeout_seconds" : 0,
  "proxy_port" : 0
}

Obviously the placeholder ${oracle.maven.repo} was not replaced by its value which is specified in the same profile.

yma96 commented

@pkocandr
Hi, could you help to provide the input.pom example for me? I try to do a unit-test for impiled repos expression replace (for main reporitories), but it passed, seem wrong place I've located.

The problem is probably not related to implied repos and is maybe a problem in galley that it does not replace placeholders for repository definitions in a profile, but it should of course work for implied repos too.

I believe the MavenPomReader should already be doing property replacement for these things. I wonder if the property itself was originally specified externally, like in the settings.xml or on the command line? FWIW I found this comment in the POM:

Example oracle profile. You can use this profile if you:
                1) have the Oracle artifacts installed in a local repo and
                supply the URL:
                -Doracle.maven.repo=http://my.local.repo
                2) have a copy of the Oracle driver and run the following
                command:
                    mvn install:install-file -Dfile=${path to ojdbc.jar} \
                                             -DgroupId=com.oracle \
                                             -DartifactId=jdbc-driver \
                                             -Dversion=10g \
                                             -Dpackaging=jar

                You must also set the following properties:
                    -Dopenjpa.oracle.url
                    -Dopenjpa.oracle.username
                    -Dopenjpa.oracle.password

                Optionally, you can override the default Oracle groupId and
                version by also supplying the following properties:
                    -Doracle.groupid=com.oracle
                    -Doracle.version=10g

If we can't resolve the property, I think the other thing we can really do is to test that it's a valid URL. We could start by just constructing a java.net.URL instance with it and see if it throws a malformed url exception. Beyond that, we could use a simplified form of the approach autoprox uses to validate repository URLs:

https://github.com/Commonjava/indy/blob/master/addons/autoprox/common/src/main/java/org/commonjava/indy/autoprox/data/AutoProxDataManagerDecorator.java#L167

Yeah, the validation would help in cases when the property value is not available, but in this case there is a value specified in the properties of the profile

<oracle.maven.repo>http://not.a.real.repository</oracle.maven.repo>

So here the validation would pass while the repo url is still not any real url. But the problem is mainly the placeholoder replacement that does not happen in this case. It seems to be related to an issue we encountered before Commonjava/galley#19

Strange...that looks like it should work. I think we need to add this as an issue to galley-maven, and use that POM as a basis for a test case.

In any case, you're right that the value above would pass URL construction. That's what I was mentioning before about using something more like autoprox validation to actually perform a HEAD request to ensure the server is reachable.