BndPomRepository should support both bsn and gav like MavenBndRepository

[chrisrueger]

Closes #5961

This PR should fix the problem that for a valid OSGi bundle MavenBndRepository.get(bsnOrGav) can be used with bsn or gav and returns something.
BndPomRepostory.get() instead only works with the bsn for a valid OSGi bundle. (Non OSGi bundles which do not have a bsn meta data can only be looked up by gav wich works as expected)

This PR's goal is to make both repos behave the same and support bsn or gav for OSGi bundles.

[chrisrueger]

1st draft, base for discussion.

• I ported MavenBndRepositoryTest.testGetViaBSNAndGAV() to PomRepositoryTest.testGetViaBSNAndGAV() to reproduce the problem
• next commits are a try to fix it. Not sure if it is a good impl, but at least based for discussion @pkriens

I hooked into the indexing of BridgeRepository because this seems to handle the indexing of 3-digit versions which allows you to lookup for "1.2.0" as well as "1.2" (e.g. for commons-cli:commons-cli:1.2 (commons-cli-1.2.jar) which physically just defines a 2-digit version.

[pkriens]

I think you're making it too hard but I might be wrong.

A non-OSGi artifact has nothing to do with the BridgeRepository. They lack any OSGi metadata. Trying to support them as a first class citizen there feels uncomfortable because they wreak havoc on many different places, the bsn is a pretty big invariant of OSGi. This is a hack only the Maven repositories need, it shouldn't touch code unrelated.

That said, to be able to wrap them, and sometimes use in unit tests, we need a way to address them. However, we should keep in mind that they are not the common way to address bundles.

This only needs to be supported in the RepositoryPlugin::get method. It is rather easy to check the bsn for a ':' and if it has, you can directly construct the GA, get the versions, and compare the versions to find the best matching. The BndPomRepository already has a list of archives from the POM.

Something like this:

	@Override
	public File get(String bsn, Version version, Map<String, String> properties, DownloadListener... listeners)
		throws Exception {
		if (!init()) {
			return null;
		}

		Archive archive;
		if (isMavenGAV(bsn)) {
			Archive spec = Archive.valueOf(bsn + ":" + version);
			archive = archives.stream()
				.filter(a -> matches(a, spec))
				.findAny()
				.orElse(null);
			if (archive == null)
				return null;
		} else {
			ResourceInfo resource = bridge.getInfo(bsn, version);
			if (resource == null) {
				archive = trySources(bsn, version);
				if (archive == null)
					return null;
			} else {

				String from = resource.getInfo()
					.from();
				archive = Archive.valueOf(from);
			}
		}
		Promise<File> p = repoImpl.getMavenRepository()
			.get(archive);

		if (listeners.length == 0)
			return p.getValue();

		Map<String, String> attrs = archive.attributes();
		new DownloadListenerPromise(reporter, name + ": get " + bsn + ";" + version, p, attrs, listeners);
		return repoImpl.getMavenRepository()
			.toLocalFile(archive);
	}

Notice that the IndexFile in the MavenBndRepository is basically collapsed into the BndPomRepository. The archives is the index.

[chrisrueger]

Thanks @pkriens for the review.
I will rollback the BridgeRepo code.

The archives is the index.

I think I had something similar in between. I saw the archives were null or so.. Not sure anymore, I will double check.

Thanks a lot. I post an update.

[chrisrueger]

@pkriens I have changed it to something similar to your suggestion.
In order to access the archives found no other way than populating them first in PomRepository

https://github.com/bndtools/bnd/pull/5964/files#diff-3a6aadeb70271d493c56e3f477c4b1ddf7edd867b4b0cfefef9b6bbb20f355fdR94-R95

so I can do pomRepo.archives.stream().... (see)

The archives in BndPomRepository itself were always null when using a pom.xml. They were only populated when setting revisions (configuration.revision()).

[chrisrueger]

Furthermore I added the same GAV handling to pomRepo.versions(bsn) because I noticed that this was also related to my initial reported problem and pomRepo.get() was not enough.

After fixing .versions(bsn) too all the errors below in my project went away.

[pkriens]

Yeah, much better. It is now restricted to the BndPomRepository and does not pollute the generic code. Thanks!

[pkriens]

pressed the wrong button ... sorry

[chrisrueger]

@pkriens

While playing with my fix in this PR I noticed the following behavior:

When the pom.xml contains two versions of the same dep like this:

<dependency>
    <groupId>org.apache.felix</groupId>
    <artifactId>org.apache.felix.gogo.shell</artifactId>
    <version>1.1.4</version>
</dependency>
<dependency>
    <groupId>org.apache.felix</groupId>
    <artifactId>org.apache.felix.gogo.shell</artifactId>
    <version>1.1.0</version>
</dependency>

I get this when I want to start my .bndrun (-runbundles org.apache.felix.gogo.shell;version='[1.1.4,1.1.5)',\)

It works fine (no error) if I reverse the order and put 1.1.4 last:

<dependency>
    <groupId>org.apache.felix</groupId>
    <artifactId>org.apache.felix.gogo.shell</artifactId>
    <version>1.1.0</version>
</dependency>

<dependency>
    <groupId>org.apache.felix</groupId>
    <artifactId>org.apache.felix.gogo.shell</artifactId>
    <version>1.1.4</version>
</dependency>

or remove 1.1.0

So it seems: last version wins.

In MavenBndRepository (.mvn) I do not have this problem. Since I generated the pom.xml from my .mvn file, it coindicently ended up in that order in the pom.xml but I just noticed it now with my first steps with BndPomRepo

Question:
Is this expected behavior of BndPomRespository?
Or should I file a bug and dig deeper? Any idea where I could start search putting my breakpoint to search the needle in the haystack?

EDIT:

It can even be seen in the Repo Browser, that only one version per bundle (both are OSGi bundles with a bsn) is shown:

1.1.0 is last:

1.1.4 is last:

In MavenBndRepo there are both versions in the RepoBrowser.

[pkriens]

Well, officially you cannot have 2 versions in maven so from a pom pov it is problematic. From a BndPomRepository pov it is obviously wrong ... We should be able to support multiple versions. I think this is the culprit:

bnd/biz.aQute.repository/src/aQute/maven/provider/POM.java

Line 512 in bc9ecf9

if (deps.containsKey(d.program))

It explicitly removes multiple versions. I'd expect it would only show the first so it could be something else

[chrisrueger]

Thanks @pkriens I will create an issue out of it and try digging in during the next days.