Conversation
And hence, congestion with possible timeouts on resource. Fixes apache#1609
| return metadataType.substring(0, slash) | ||
| + separator | ||
| + getMetadataName( | ||
| new DefaultMetadata( |
There was a problem hiding this comment.
It looks inefficient to create an object just to call the method recursively. It may be cleaner to define a new method which would have the individual parameters.
| if (metadata.getType().contains("/")) { | ||
| String metadataType = metadata.getType(); | ||
| int slash = metadataType.indexOf('/'); | ||
| return metadataType.substring(0, slash) |
There was a problem hiding this comment.
Why do we put the first part of the metadata in front while we usually used g/a/v/t ?
What are the types containing a / ?
There was a problem hiding this comment.
This needs a bit of history.... originally Maven had no means to (re)use Resolver transport to fetch (or put) non "standard" (not on layout) resources, and similarly, you could not reuse cache/checksum policies to perform such a transport, youd need to redo everything. As everything was either artifact or maven metadata. But with latest Resolver, metadata URI is calculated like this:
Metadata is G, A, V and T. Remote URI is constructed as:
- remote repository
baseUrl - if G given,
+G.replaceAll(".", "/")-- as artifact groupId - if A given,
+A-- as artifact artifactId - if V given,
+V-- as artifact version +type
Type was always "maven-metadata.xml". This is how you can make Resolver download the Maven Repository Metadata from G (plugin) level, GA (versions) level, and GAV (snapshot resolution) level.
Later, we realized that type does not have to be always "maven-metadata.xml", but can be something else, so for example Archetype Catalog (that sits in repo root) can be addressed (and hence, use same transport to get artifacts) as this (this is how it is done in m-archetype-p to get and cache the catalog):
Metadata(G:"", A:"", V:"", Type:"archetype-catalog.xml") and is cached locally as archetype-catalog-$repoID.xml (w/ checksums) by Resolver automatically (this is how "metadata" is handled by resolver).
The subsequent step was to make Metadata be able to address any (out of layout) file on remote repository, or, in other words, make Maven able to get any file (and cache it, with all the whistle and bells of caching/policies/checksums) that are not addressable by GAV/artifact. This change was done in #1491 and it was needed for prefixes (again, to use same transport and all features of local repo like checksum validation/cache retention/etc and maven metadata tracking by origin). Basically prefix files lie on this path:
Metadata(G:"", A:"", V:"", Type:".meta/prefixes.txt") and is cached locally as .meta/prefixes-$repoId.txt (w/ checksums) by resolver automatically.
And currently, they are the ONLY Metadata files (in resolver and/or maven) that contain / (slash).
The point is, that archetype catalog, but also prefixes are repository metadata resources, and they need same checksum validation, cache retention as any other "metadata". And what happens is total reuse of all existing functionality of Resolver to get/cache these files (so -U also updates them, etc).
There was a problem hiding this comment.
One of the typical pain was long time index: indexes are also published within repository, here. And originally, Maven Indexer had to reimplement everything (full transport stack) to get it from same place, from where user was already getting artifacts from (with same possible proxies, mirrors, auth and all the cruft). Now, Resolver would be able even to get indexes for you, no need to reimplement anything, as any file, that is published via repository is reachable to Resolver and Maven (and plugins).
There was a problem hiding this comment.
And to answer your original question "we put the first part of the metadata in front"... due local repository layout. If I ask from Central following metadata:
Metadata(G:"", A:"", V:"", Type:"prefixes.txt")Metadata(G:"", A:"", V:"", Type:".meta/prefixes.txt")Metadata(G:"", A:"", V:"", Type:".meta/meta/prefixes.txt")
They would call conflict, as Resolver handles Metadata file names in local repository by appending origin repoId, so they would be locally then be named prefix-$repoId.txt and clash.
Oh, but you are right, these are local repo paths, and we talk about lock names. Yes, you are right, we could replace / to _ or anything to keep them "flat", agreed.
We already had means to "make sure string is valid for path segment", but it was buried in RepositoryIdHelper. Pull it out, and make it reusable, as this is essentially very same use case. Also, the "expensive" method really punishes ONLY non maven metadata, and those should be few only.
It caused in certain cases that lock names for different metadata collapsed onto same name, and hence, congestion with possible timeouts on resource happened. In case of metadata,
Metadata#getTypewas totally ignored, which turns out was wrong, as today we have more types than the simplemaven-metadata.xml.Fix: We already had means to make sure "string is valid for path segment", but it was buried in
RepositoryIdHelper. Pull it out, and make it reusable, as this is essentially very same use case. Also, this extra work really "punishes" only non maven metadata, and that is okay (right now we have only repository prefix and archetype catalog files in play).Fixes #1609