Make repeated requests for related artifact more efficient

[quintesse]

So we have this situation where for the JS backend we actually need to retrieve 3 artifacts, 2 required ones (JS, JS_MODEL) and one optional one (RESOURCES).

In the case of the 2 required ones we know they should both always be there, if not it's an error.

The thing is that right now what the JS backend does is incorrect. It just does a single ArtifactContext.getArtifact() call using both required types, the CMR will return the first one it finds and then the backend will assume the second one exists in the same folder, which is wrong (you can't assume the other exists).

And then it does a second call to ArtifactContext.getArtifact() for the 3rd optional artifact.

Thing is it should do 3 separate calls to ArtifactContext.getArtifact() to be sure that everything is correctly available.

But that is really inefficient, because that call will again go through all the repositories, including remote ones if it can't find them, while we know that they must be together.

So we should try to find an efficient way to do aArtifactContext.getArtifact() call on just a single repository, some way to search in the same place where the previous result was from.

Stef just made a change some days ago that adds the Repository that an artifact was found in to the ArtifactResult. That's probably just what we need here.

[quintesse]

@alesj I've made a stab at a possible solution for what I told you by mail (see most of it repeated in the description). Could you take a look at it to see if I'm making a complete mess of your code or if you think it's acceptable? Thx.

[alesj]

Looks somehow OK. I would hide this impl details a bit more;

e.g. no need to set the repository on artifact context, it should be there already: A user would get back ArtifactResult, which would then have a method toArtifactContext, which would transparently copy over the respository where the artifact was found

Or something like that.

I can have a look next week.

[alesj]

@quintesse ^^

[quintesse]

Well yes, that was going to be the next step. I wanted something that worked before making it better / more useful. I had thought to create a toSiblingContext() or something like that on ArtifactResult. At least knowing you don't think this is total rubbish I'll improve it a bit more and you can take a look at it when you have time.

Have a nice weekend!

[quintesse]

Added a getSiblingArtifact() to make explicitly setting the repository unnecessary. I still needed to add it to a public constructor though, I didn't see a way to prevent that.

[alesj]

I still needed to add it to a public constructor though, I didn't see a way to prevent that.

A? Can't it be a package protected setRepo method? As long as it's impl detail ... As it really should be impl detail. ;-)

[quintesse]

Well yes, but ArtifactContext doesn't have an implementation, it's wholly API (unlike `ArtifactResult`` which does have a separate api and implementation).

[alesj]

@quintesse https://github.com/alesj/ceylon-module-resolver/tree/sibling_fix

Imo, this looks better, as, no need for the ArtifactResult to be the ArtifactContext API, it should be ArtifactContext that knows how to build new sibling from itself.

[quintesse]

Perfectly fine by me. Are you done with it? can I merge when I'm ready to update the rest of the code?

[alesj]

Yes. Go ahead. On Aug 13, 2014 12:45 PM, "Tako Schotanus" notifications@github.com wrote:

Perfectly fine by me. Are you done with it? can I merge when I'm ready to update the rest of the code?

— Reply to this email directly or view it on GitHub https://github.com/ceylon/ceylon-module-resolver/issues/96#issuecomment-52033430 .

[quintesse]

Merged. Closing. Thanks @alesj !

[quintesse]

Btw, things like the "copy" tool are much much faster now (besides the API making things simpler). At least for local repositories. And for remote repositories like the Herd we hopefully have the possibility to override the multi-artifact handling some day with some special protocol.

[alesj]

@quintesse "copy" tool?

to override the multi-artifact handling some day

Yeah, that would be the way to go. Let me know when you decide to do this. Or if I can find the time ...

[quintesse]

"copy" tool?

Yes, we have a "ceylon copy" that can be used to copy artifacts from one repository to another (including from/to remote repos).

Yeah, that would be the way to go. Let me know when you decide to do this.

Wasn't planning it for now :)