1 2 Previous Next 29 Replies Latest reply on Sep 24, 2011 8:50 AM by badr

    Resolving Maven artifacts

    silenius

      Hi to all,

       

      I'm ready to start working on the following issues:

      1. https://issues.jboss.org/browse/SHRINKWRAP-261
      2. https://issues.jboss.org/browse/SHRINKWRAP-265

       

      Do you have any thoughts about what you would like the shorthand API to look like? And do we all agree on the changes for SHRINKWRAP-265?

        • 1. Resolving Maven artifacts
          kpiwko

          Hi Samuel,

           

          here are my thoughts for both jiras:

           

          SW-265:

           

          It was not clearly announced what's StrictFilter() for. Maven way is to download all transitive dependencies. Maybe we should resolve in a StrictFilter mode by default and have resolveWithDependencies() method with AllFilter(). I'm basically happy with current way, but fortunately I don't represent majority of the users

           

           

          SW-261:

           

          1/ MavenArtifact.resolve(Coordinate.of("joda-time", "joda-time", "1.6")).as(GenericArchive.class)

           

          This won't help user to reduce the code lenght. Moreover, Coordinate builder would have a bunch of method all of them accepting Strings, imagine that there is type (extension), classifier, scope as well and user might want to use existing POM/BOM file to load version dependency from there

           

          2/ MavenArtifact.resolve("joda-time:joda-time:1.6").as(GenericArchive.class)

           

          Method like this was in implementation for a short time. It's generally a shortcut to DependencyResolvers.use(MavenDependencyResolver.class). Makes sence IMHO.

           

          3/ Maven.artifact("joda-time:joda-time:1.6")

           

          Same as 2/, it makes sense to have a shortcut which returns GenericArchive.

           

           

          For all shortcut methods I'm missing ability to resolve more than one artifact. The reason why I'm considering this a problem is that intialization of Aether is a costy operation and so is the construction of dependency graph and by an static import of let's say Maven.artifact user will probably use:

           

          addAsLibraries(artifact("foo:bar:1"), artifact("foo:bar:2)) which will be much slower then current artifacts("foo:bar:1", "foo:bar:2") or chaining equivalent artifact("foo:bar:1").artifact("foo:bar:2).resolveAs();

           

          The other problem of shortcuts is that the expect evaluation in StrictFilter mode - it means no exclusions etc...is that intention? Having all these methods to allow user to grab a single file from standard Maven repo in a less verbose way?

           

          Karel

          • 2. Re: Resolving Maven artifacts
            silenius

            Hi Karel,

             

            SW-265:

             

            It was not clearly announced what's StrictFilter() for. Maven way is to download all transitive dependencies. Maybe we should resolve in a StrictFilter mode by default and have resolveWithDependencies() method with AllFilter(). I'm basically happy with current way, but fortunately I don't represent majority of the users

            My issue here is that most of the time I only need a direct dependency since its transitive dependencies are already present in JBoss AS.

            I usually end up with less code adding the dependencies one by one than having to filter which ones I don't need.

            Renaming methods also clarifies what they really do.

            However, if we are going to have a shortcut API I don't mind keeping it the way it is and use the shortcut API when I just need a specific dependency.

            SW-261:

             

            1/ MavenArtifact.resolve(Coordinate.of("joda-time", "joda-time", "1.6")).as(GenericArchive.class)

             

            This won't help user to reduce the code lenght. Moreover, Coordinate builder would have a bunch of method all of them accepting Strings, imagine that there is type (extension), classifier, scope as well and user might want to use existing POM/BOM file to load version dependency from there

             

            2/ MavenArtifact.resolve("joda-time:joda-time:1.6").as(GenericArchive.class)

             

            Method like this was in implementation for a short time. It's generally a shortcut to DependencyResolvers.use(MavenDependencyResolver.class). Makes sence IMHO.

             

            3/ Maven.artifact("joda-time:joda-time:1.6")

             

            Same as 2/, it makes sense to have a shortcut which returns GenericArchive.

             

             

            For all shortcut methods I'm missing ability to resolve more than one artifact. The reason why I'm considering this a problem is that intialization of Aether is a costy operation and so is the construction of dependency graph and by an static import of let's say Maven.artifact user will probably use:

             

            addAsLibraries(artifact("foo:bar:1"), artifact("foo:bar:2)) which will be much slower then current artifacts("foo:bar:1", "foo:bar:2") or chaining equivalent artifact("foo:bar:1").artifact("foo:bar:2).resolveAs();

            We agree in the tree points above, for our shortcut I was thinking about something like MavenArtifact.resolve("joda-time:joda-time:1.6") which returns a GenericArchive.

            I'm not sure whether or not we should include the ability to resolve more than one artifact in the shortcut API. But if we decide to go ahead and add this feature I would change the API to the following:

             

            Collection<GenericArchive> Maven.artifacts(String... coordinates);
            GenericArchive Maven.artifact(String coordinate);
            
            The other problem of shortcuts is that the expect evaluation in StrictFilter mode - it means no exclusions etc...is that intention? Having all these methods to allow user to grab a single file from standard Maven repo in a less verbose way?

            Yep, that is the intention.

            • 3. Re: Resolving Maven artifacts
              kpiwko

              My issue here is that most of the time I only need a direct dependency since its transitive dependencies are already present in JBoss AS.

              I usually end up with less code adding the dependencies one by one than having to filter which ones I don't need.

              Renaming methods also clarifies what they really do.

              However, if we are going to have a shortcut API I don't mind keeping it the way it is and use the shortcut API when I just need a specific dependency.

               

               

              Renaming methods is a good idea. I would rather go for resolveAs() and resolveTransitivelyAs(), but direct dependency (resolveAs()) should be the default, because test developers usually want this behavior.

               

              As for the shortcut API I'm a bit afraid users will discover sooner or later POM related methods, which allows you to specify a POM file where for instance dependency versions are located. That's why I would like to have POM related functionality there as well.

               

              See https://issues.jboss.org/browse/SHRINKWRAP-268 and https://github.com/shrinkwrap/shrinkwrap/blob/master/extension-resolver/impl-maven/src/test/java/org/jboss/shrinkwrap/resolver/impl/maven/PomDependenciesUnitTestCase.java to see how POM related functionality might be used. Something like

               

               

              Collection<GenericArchive> Maven.artifacts(String... coordinates);
              GenericArchive Maven.artifact(String coordinate);
              Maven Maven.withPom(String pomFile)
              
              
              =>
              
              
              Maven.withPom("pom.xml").artifact("foo:bar"); // notice no version required there, it's in POM
              
                                                            // POM may contain specific remote repositories definitions as well
              
              • 4. Re: Resolving Maven artifacts
                silenius

                I agree that adding the POM functionality to the shortcut API in indeed a good idea.

                 

                In recap:

                 

                SW-261: shortcut API

                Collection<GenericArchive> Maven.artifacts(String... coordinates);
                GenericArchive Maven.artifact(String coordinate);
                Maven Maven.withPom(String pomFile);
                Maven.withPom("pom.xml").artifact("foo:bar"); // no version required there, POM may contain specific remote repositories definitions as well
                

                 

                SW-265 and SW-268: MavenDependencyResolver methods renaming

                <T extends Archive<T>> Collection<T> MavenDependencyResolver.resolveTransitivelyAs(T type); // return the Maven artifact and its dependencies
                <T extends Archive<T>> T MavenDependencyResolver.resolveAs(T type); // return the artifact without any dependencies
                MavenDependencyResolver loadMetadataFromPom(String file); // was MavenDependencyResolver loadReposFromPom(String path);
                MavenDependencyResolver includeDependenciesFromPom(String path) throws ResolutionException; // was MavenDependencyResolver loadDependenciesFromPom(String path) throws ResolutionException;
                // remove method: MavenDependencyResolver loadDependenciesFromPom(final String path, final MavenResolutionFilter filter) throws ResolutionException;
                

                 

                Does everyone agree on those changes?

                • 5. Re: Resolving Maven artifacts
                  kpiwko

                  I do.

                  • 6. Re: Resolving Maven artifacts
                    silenius

                    If there are no objections I intend to start working on these issues next week

                    • 7. Resolving Maven artifacts
                      dan.j.allen

                      Karel Piwko wrote:

                       

                      Renaming methods is a good idea. I would rather go for resolveAs() and resolveTransitivelyAs(), but direct dependency (resolveAs()) should be the default, because test developers usually want this behavior.

                       

                      I'm 100% in agreement. I've started incorporating the dependency resolver into the Arquillian showcase, and almost immediately found myself wanting to cherry pick depenedencies in most cases rather than having it pull in all the transitive dependencies.

                       

                      The method name resolveTransitivelyAs() doesn't sit right with me, though. Does it fit to use a boolean argument to switch on transitive resolution?

                       

                      resolver.artifact(true, "org.hibernate:hibernate-validator:4.1.0.Final")

                       

                      That would grab just hibernate-validator-4.1.0.Final jar, leaving out the slf4j-api dependency.

                       

                      The other approach would be to set transitive behavior at the resolver level (perhaps we need both ways).

                       

                      MavenDependencyResolver resolver = DependencyResolvers.use(MavenDependencyResolver).resolveTransitiveDependencies().loadReposFromPom("pom.xml")...

                       

                      In general, though, you guys are definitely headed in the right direction. The easier we can make pulling libraries into the archive, the better

                      • 8. Re: Resolving Maven artifacts
                        kpiwko

                        No reason to have true/false flag. There is already a StrictFilter(), which does the exact thing and it is a part of the API.

                         

                        The problem is it is a filter so it does not make sense to have it renamed to IncludeTransitiveFilter.

                         

                        Maybe having it renamed to FilteringStrategy, then IncludeTransitiveStrategy and IsolatedStrategy impl (by default)?

                        Introducing strategy() "wrap" and removing Filter from the resolveXYZ() method API, it will look like

                         

                        MavenDependencyResolver.artifact("foo:bar:1").resolveAs(Archive.class) // IsolatedStrategy
                        MavenDependencyResolver.artifact("foo:bar:1").strategy(new IncludeTransitiveStrategy()).resolveAs(Archive.class) 
                        Maven.strategy(new IncludeTransitiveStrategy()).artifact("foo:bar:1") // or singleton instance Strategies.TRANSITIVE
                        

                         

                        Seems more reasonable and flexible than boolean to me.

                        • 9. Re: Resolving Maven artifacts
                          silenius

                          Hi Dan and Karel,

                           

                          I don't think that renaming StrictFilter is a good idea.

                          Currently we have 4 filters: AcceptAllFilter, CombinedFilter, ScopeFilter and StrictFilter.

                           

                          My idea to implement the method:

                          <T extends Archive<T>> T MavenDependencyResolver.resolveAs(T type);

                          is to create a new SingleFilter that will resolve only the direct dependency.

                           

                          The method:

                          <T extends Archive<T>> Collection<T> MavenDependencyResolver.resolveTransitivelyAs(T type);

                          will use the AcceptAllFilter as used by the current implementation:

                          public <ARCHIVEVIEW extends Assignable> Collection<ARCHIVEVIEW> resolveAs(final Class<ARCHIVEVIEW> archiveView) throws ResolutionException
                          {
                             return resolveAs(archiveView, AcceptAllFilter.INSTANCE);
                          }
                          

                           

                          Those two filters are transparent to developers.

                           

                          For anything else in between you will mostly need StrictFilter:

                          <T extends Archive<T>> Collection<T> MavenDependencyResolver.resolveTransitivelyAs(T type, MavenResolutionFilter filter); // Need to exclude some transitive dependencies
                          • 10. Re: Resolving Maven artifacts
                            kpiwko

                            I don't think that renaming StrictFilter is a good idea.

                            Currently we have 4 filters: AcceptAllFilter, CombinedFilter, ScopeFilter and StrictFilter.

                             

                            My idea there was to change filter to strategy. User will specify how the artifacts should be resolved instead of filtering the results. It will give us even more flexibility and its usage is more clear to me.

                             

                            Imagine following call:

                             

                            MavenDependencyResolver.strategy(Strategies.IGNORE_REMOTE_REPOSITORIES).artifact("foo:bar:1").resolveAs(...)
                            

                             

                            The only problem I see here is that strategy must be quite powerfull object and I've been not thinking about how to split it to api and impl yet. Currently filters are wrapped to DependencyFilter of Aether and applied on the graph of resolved dependencies.

                             

                            Strategy should encapsule whole resolution so developers can hack it. It means extracting functinality out from MavenRepositorySystem class and move it to strategy. Are you interested in such change? Should I elaborate on this?

                             

                            My idea to implement the method:

                            <T extends Archive<T>> T MavenDependencyResolver.resolveAs(T type);
                            

                            is to create a new SingleFilter that will resolve only the direct dependency.

                             

                             

                            Any reason to have SingleFilter? Applying a StrictFilter while one depedency is specified will do the same work.

                            • 11. Re: Resolving Maven artifacts
                              silenius

                              Hi Karel,

                               

                              Creating a new StrictFilter instance can be quite verbose.

                              I was thinking about the SingleFilter as a shortcut for the StrictFilter, to include only the direct dependency.

                              Nothing very fancy here.

                               

                              If we were to change filters into strategies, what do you have in mind for filtering the resolveTransitivelyAs() method to include only some transitive dependencies?

                              • 12. Re: Resolving Maven artifacts
                                kpiwko

                                Hi Samuel,

                                 

                                Creating a new StrictFilter instance can be quite verbose.

                                I was thinking about the SingleFilter as a shortcut for the StrictFilter, to include only the direct dependency.

                                Nothing very fancy here.

                                Ahh, I see. But it will be still hidden from user, so he won't need to create it anyway, right?

                                 

                                If we were to change filters into strategies, what do you have in mind for filtering the resolveTransitivelyAs() method to include only some transitive dependencies?

                                That's a very good remark, I completely missed that. So now the question reduces to if having a strategy(...) for changing between resolveAs() and resolveTransitivelyAs() behavior is a good idea. Implementation can simply use CombinedFilter and add StrictFilter transparently to any filter user has passed to resolveAs() method if Strategies.TRANSITIVE is registered. Sounds resonable?

                                • 13. Re: Resolving Maven artifacts
                                  silenius

                                  Hi Karel,

                                  Ahh, I see. But it will be still hidden from user, so he won't need to create it anyway, right?

                                  Yes, it will be hidden from the user, but I believe it will help to keep the code clean since we can use it internally in other places.

                                  That's a very good remark, I completely missed that. So now the question reduces to if having a strategy(...) for changing between resolveAs() and resolveTransitivelyAs() behavior is a good idea. Implementation can simply use CombinedFilter and add StrictFilter transparently to any filter user has passed to resolveAs() method if Strategies.TRANSITIVE is registered. Sounds resonable?

                                  resolveAs() and resolveTransitivelyAs() methods return different objects:

                                  <T extends Archive<T>> T MavenDependencyResolver.resolveAs(T type)
                                  <T extends Archive<T>> Collection<T> MavenDependencyResolver.resolveTransitivelyAs(T type[, MavenResolutionFilter filter])
                                  

                                   

                                  If you change it into something similar to

                                  MavenDependencyResolver.strategy(...).resolveAs(T type [, MavenResolutionFilter filter])
                                  

                                  You cannot distinguish if it will return an Archive or a collection of Archives.

                                   

                                  StrictFilter is only needed for transitive dependencies. I do not think we need to add complexity to what should be the most used method - resolveAs().

                                  Summarizing, the approaches we discuss earlier in this thread still are the ones that make the most sense to me.

                                  • 14. Re: Resolving Maven artifacts
                                    kpiwko

                                    Hi Samuel,

                                     

                                    You cannot distinguish if it will return an Archive or a collection of Archives.

                                     

                                    correct, unless strategy(...) returns a different interface implementation, similarly to what Assignable interface do. However, this would mean that we'll need at least one interface with collection based methods (e.g. TransitiveDependencyResolver)

                                     

                                     

                                    public interface ResolutionStrategy
                                    {
                                       <S extends ResolutionStrategy> S strategy(Class<S> clazz);
                                    }
                                    
                                    

                                     

                                     

                                    public interface TransitiveDependencyResolver<F extends DependencyResolutionFilter<F, E>, 
                                    E extends ResolutionElement<E>> extends ResolutionStrategy   {
                                       <ARCHIVEVIEW extends Assignable> Collection<ARCHIVEVIEW> resolveAs(Class<ARCHIVEVIEW> archiveView)
                                             throws ResolutionException;
                                    
                                       ...  
                                    
                                       File[] resolveAsFiles();
                                    
                                    }
                                    
                                    

                                     

                                    and one with single element methods (e.g. DependencyResolver)

                                     

                                    public interface DependencyResolver<F extends DependencyResolutionFilter<F, E>, 
                                    E extends ResolutionElement<E>> extends ResolutionStrategy  {
                                    
                                       <ARCHIVEVIEW extends Assignable> ARCHIVEVIEW resolveAs(Class<ARCHIVEVIEW> archiveView)
                                             throws ResolutionException;
                                    
                                       ...
                                    
                                       File resolveAsFile();
                                    
                                    }
                                    
                                    

                                     

                                    kinds of DependencyResolver interfaces within resolvers-api package which extend ResolutionStrategy.

                                    They will be interchangable and user can provide his own strategy, if he wants. MavenDependencyResolver will implement DependencyResolver, so it will retrieve a single archive by default.

                                     

                                    I have to say I'm still wondering if it is worth it. On the one hand it makes API for users simpler and more fluent to me, on the other hand it makes work for resolver extension developers (e.g. Ivy) more complicated. Other thing is that <code>strategy(TransitiveDependencyResolver.class).resolveAs(...)</code> is (much) longer than <code>resolveTransitivelyAs(...)</code>.

                                     

                                    Edit: I forgot to mention that this would change call semantics a bit w.r.t. discussion above:

                                     

                                    MavenDependencyResolver.artifact("foo:bar:1").strategy(TransitiveDependencyResolver.class).resolveAs(...)
                                    
                                    1 2 Previous Next