Ivy
  1. Ivy
  2. IVY-434

refactor Ivy source code to improve readibility

    Details

    • Type: Task Task
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.4.1
    • Fix Version/s: 2.0
    • Component/s: None
    • Labels:
      None

      Description

      Ivy needs some refactoring to ease the understanding of its code base for new developers. The migration to ASF is good moment to make this refactoring.

      Note that I open this issue really too late because most of the work is already done, but I want to keep track of what has been done in something easier to include in the release notes than the mailing lists.

      So I will copy some info from the mailing list to this issue:
      On 1/29/07, Xavier Hanin <xavier.hanin@gmail.com> wrote:
      Main focus:
      + split the Ivy class by features in:
      ++ IvySettings, which will be the result of the configure step (I do
      not use configuration to avoid confusion with module configurations)
      ++ ResolveEngine, which will be responsible for dependency resolution
      ++ RetrieveEngine, responsible for the retrieve step
      ++ and so on for each features/tasks
      The Ivy class will preserve an API similar to the existing one, but
      will only be a Facade to other classes. Moreover, methods taking too
      many parameters (like resolve) will be refactored to take a fewer
      number of parameters, using a class (like ResolveOptions for instance)
      to group those which are not first class parameters

      I will also work on the dependency resolution algorithm, and
      especially on IvyNode. I will split it into at least two classes, one
      representing the node in the dependency graph, and one with data
      related to the traversal of this graph during the resolution process.

      Another thing I'd like to address is to reduce the number of classes
      in the same package, and the number of packages of the same level
      (namely org.apache.ivy.* packages), to move to something more
      structured and hopefully less confusing.

      This refactoring will introduce many API incompatibilities, but it
      should hopefully help people to understand the code.

      This is only the big picture, I'll keep you informed of my progress,
      and try to process by steps to allow frequent feedback and
      discussions.

        Activity

        Hide
        Xavier Hanin added a comment -

        This first step consists mostly in package renames / class moves. I've
        tried to make the new structure cleaner and easier to understand, but
        it still requires work on the classes themselves.

        The main idea of the new structure is there:
        org.apache.ivy.core
        ---------------------------
        core of ivy, used internally to provide the main features of Ivy
        This package contains a subpackage for each main feature of Ivy
        (settings, resolve, retrieve, deliver, publish, install, ...). It also
        contains subpackages for some core data:
        + core.module.id is used to group classes storing Module related identifiers
        + core.module.descriptor is used to group classes used to represent a
        Module Descriptor in memory (i.e. usually the result of the parsing of
        an ivy.xml file)
        + core.event is used for classes related to Ivy event mechanism

        org.apache.ivy.plugins
        -----------------------------
        everything that is pluggable in Ivy can be found here. The rationale
        behind the isolation in a separate package is to help people who want
        to develop a plugin find the base classes and interfaces without too
        much trouble, and without being confused by Ivy internals found in
        core.
        Under this package, there is a subpackage for each kind of plugin.

        org.apache.ivy.ant
        -----------------------------
        everything related to ant is located here, i.e. mainly ant tasks.
        Nothing has been moved in this package during the refactoring

        org.apache.ivy.util
        -----------------------------
        Here you can find helper classes which are not directly related to
        Ivy, which could be isolated in separate projects or reused for other
        purpose.

        org.apache.ivy.tools
        -----------------------------
        Here lies tools related to Ivy, but not really part of Ivy core
        itself. Maybe this should better be kept in a separate project, for
        the moment there is only an attempt to create an automatic Ivy file
        generator from jars, leveraging the jarjar utility.

        Show
        Xavier Hanin added a comment - This first step consists mostly in package renames / class moves. I've tried to make the new structure cleaner and easier to understand, but it still requires work on the classes themselves. The main idea of the new structure is there: org.apache.ivy.core --------------------------- core of ivy, used internally to provide the main features of Ivy This package contains a subpackage for each main feature of Ivy (settings, resolve, retrieve, deliver, publish, install, ...). It also contains subpackages for some core data: + core.module.id is used to group classes storing Module related identifiers + core.module.descriptor is used to group classes used to represent a Module Descriptor in memory (i.e. usually the result of the parsing of an ivy.xml file) + core.event is used for classes related to Ivy event mechanism org.apache.ivy.plugins ----------------------------- everything that is pluggable in Ivy can be found here. The rationale behind the isolation in a separate package is to help people who want to develop a plugin find the base classes and interfaces without too much trouble, and without being confused by Ivy internals found in core. Under this package, there is a subpackage for each kind of plugin. org.apache.ivy.ant ----------------------------- everything related to ant is located here, i.e. mainly ant tasks. Nothing has been moved in this package during the refactoring org.apache.ivy.util ----------------------------- Here you can find helper classes which are not directly related to Ivy, which could be isolated in separate projects or reused for other purpose. org.apache.ivy.tools ----------------------------- Here lies tools related to Ivy, but not really part of Ivy core itself. Maybe this should better be kept in a separate project, for the moment there is only an attempt to create an automatic Ivy file generator from jars, leveraging the jarjar utility.
        Hide
        Xavier Hanin added a comment -

        In a second step I've begun to split the Ivy class. I've extracted an
        IvySettings class, which stores all settings, i.e. the result of a
        call to configure. I've tried to replace some dependencies on the Ivy
        class by dependencies on IvySettings, when it was possible, to attempt
        to remove some circular dependencies.

        For instance:
        Ivy -> ModuleDescriptorParser -> Ivy
        is now
        Ivy -> ModuleDescriptorParser -> IvySettings

        I've also extracted a class ResolveEngine, which is responsible of the
        resolve algorithm. Hence the Ivy class has slimmed, but I still have
        to extract other parts (retrieve, install, ...) so that it will only
        be a Facade to other engines/services.

        One thing which has changed due to this split is the way an Ivy
        instance is obtained. For the moment when you use a 'new', the
        instance created does not create its dependencies (instance of
        IvySettings and ResolveEngine for example). The idea is to make it
        possible to use other implementations (well, subclasses, since they
        are not interfaces) instead of the implementation provided by Ivy, to
        be able to fully customize the behaviour of Ivy by feature. So the
        creation of dependencies is only done in method I've called 'bind'. If
        you want an instance of Ivy already bound to its dependencies, you can
        use Ivy.newInstance(). If you prefer to setup the dependencies
        yourself, you can call 'new Ivy()' and then setters for the
        dependencies. Note that binding is automatically done when you call a
        method on Ivy which require a dependency, so that you don't have to
        care about that in most cases.

        Show
        Xavier Hanin added a comment - In a second step I've begun to split the Ivy class. I've extracted an IvySettings class, which stores all settings, i.e. the result of a call to configure. I've tried to replace some dependencies on the Ivy class by dependencies on IvySettings, when it was possible, to attempt to remove some circular dependencies. For instance: Ivy -> ModuleDescriptorParser -> Ivy is now Ivy -> ModuleDescriptorParser -> IvySettings I've also extracted a class ResolveEngine, which is responsible of the resolve algorithm. Hence the Ivy class has slimmed, but I still have to extract other parts (retrieve, install, ...) so that it will only be a Facade to other engines/services. One thing which has changed due to this split is the way an Ivy instance is obtained. For the moment when you use a 'new', the instance created does not create its dependencies (instance of IvySettings and ResolveEngine for example). The idea is to make it possible to use other implementations (well, subclasses, since they are not interfaces) instead of the implementation provided by Ivy, to be able to fully customize the behaviour of Ivy by feature. So the creation of dependencies is only done in method I've called 'bind'. If you want an instance of Ivy already bound to its dependencies, you can use Ivy.newInstance(). If you prefer to setup the dependencies yourself, you can call 'new Ivy()' and then setters for the dependencies. Note that binding is automatically done when you call a method on Ivy which require a dependency, so that you don't have to care about that in most cases.
        Hide
        Xavier Hanin added a comment -

        In third step I've "finished" the split of the Ivy class, it's now almost only a
        Facade to other objects, most of them being "engines" dedicated to
        each feature of Ivy.

        I've also just checked in a modification on the API of the deliver
        method. I took deliver as an example, but my plan is to follow the
        same recipe for other methods, if feedback is positive.

        What I've done is introduce a DeliverOptions class which stores most
        of the parameters the deliver method was taking. Well, when I say
        most, I mean all the parameters which have a default value. So now you
        can easily call deliver with default values only, or setup a
        DeliverOptions manually to configure the algorithm. The advantage I
        see is that if we want to add a new option, we just have to add the
        attribute with getter/setter, and the API of deliver does not change.
        Another thing is that it improves readability, especially when you
        have several boolean parameters, it's very difficult to know what
        you're actually doing in your call.

        For instance, compare these two calls:
        Ivy 1.4:
        ivy.deliver(mrid, "1.5", cache, "ivy-[revision].xml", "release", new
        Date(), pdrResolver, false, true)

        Ivy 1.5:
        ivy.deliver(mrid, "1.5", "ivy-[revision].xml",
        new DeliverOptions()
        .setStatus("release")
        .setPubdate(new Date())
        .setCache(cacheManager)
        .setPdrResolver(pdrResolver)
        .setValidate(false)
        .setResolveDynamicRevisions(true));

        The second is longer, but I think it's much cleaner too. And note that
        it's long because we set all options, but if we want to use default
        values for everything except the status, for example, then we can do:
        ivy.deliver(mrid, "1.5", "ivy-[revision].xml",
        DeliverOptions.newInstance(settings).setStatus("release"));

        The problem is that it changes the API, thus requiring a lot of work
        to migrate tools using Ivy 1.4 API. That's why I've added a Ivy14
        class, which provide a 1.4 compatible API, so that if you don't want
        to use the new API, you can simply replace your instance of Ivy by an
        instance of Ivy14, and you're done!

        Show
        Xavier Hanin added a comment - In third step I've "finished" the split of the Ivy class, it's now almost only a Facade to other objects, most of them being "engines" dedicated to each feature of Ivy. I've also just checked in a modification on the API of the deliver method. I took deliver as an example, but my plan is to follow the same recipe for other methods, if feedback is positive. What I've done is introduce a DeliverOptions class which stores most of the parameters the deliver method was taking. Well, when I say most, I mean all the parameters which have a default value. So now you can easily call deliver with default values only, or setup a DeliverOptions manually to configure the algorithm. The advantage I see is that if we want to add a new option, we just have to add the attribute with getter/setter, and the API of deliver does not change. Another thing is that it improves readability, especially when you have several boolean parameters, it's very difficult to know what you're actually doing in your call. For instance, compare these two calls: Ivy 1.4: ivy.deliver(mrid, "1.5", cache, "ivy- [revision] .xml", "release", new Date(), pdrResolver, false, true) Ivy 1.5: ivy.deliver(mrid, "1.5", "ivy- [revision] .xml", new DeliverOptions() .setStatus("release") .setPubdate(new Date()) .setCache(cacheManager) .setPdrResolver(pdrResolver) .setValidate(false) .setResolveDynamicRevisions(true)); The second is longer, but I think it's much cleaner too. And note that it's long because we set all options, but if we want to use default values for everything except the status, for example, then we can do: ivy.deliver(mrid, "1.5", "ivy- [revision] .xml", DeliverOptions.newInstance(settings).setStatus("release")); The problem is that it changes the API, thus requiring a lot of work to migrate tools using Ivy 1.4 API. That's why I've added a Ivy14 class, which provide a 1.4 compatible API, so that if you don't want to use the new API, you can simply replace your instance of Ivy by an instance of Ivy14, and you're done!
        Hide
        Xavier Hanin added a comment -

        In the fourth step, what I've done is split the IvyNode class in several classes. First, I've created a new class, VisitNode, which represents a visit of a node of the dependency graph. Before IvyNode was used to store both data of the node and data relative to the current state of the visit (from which parent do I visit the node, in which root module conf, and so on). Now IvyNode do not store this kind of data any more, it's the responsibility of VisitNode (which stores an instance of the IvyNode visited, and delegates several methods to IvyNode to offer an API quite similar to IvyNode one in 1.4).

        Besides this split, I've extracted two responsibilities from IvyNode: caller management (who depends on me, now handled by IvyNodeCallers class) and data related to conflict management (now handled by IvyNodeEviction class). IvyNode now delegates to instances of these classes for these features.

        Show
        Xavier Hanin added a comment - In the fourth step, what I've done is split the IvyNode class in several classes. First, I've created a new class, VisitNode, which represents a visit of a node of the dependency graph. Before IvyNode was used to store both data of the node and data relative to the current state of the visit (from which parent do I visit the node, in which root module conf, and so on). Now IvyNode do not store this kind of data any more, it's the responsibility of VisitNode (which stores an instance of the IvyNode visited, and delegates several methods to IvyNode to offer an API quite similar to IvyNode one in 1.4). Besides this split, I've extracted two responsibilities from IvyNode: caller management (who depends on me, now handled by IvyNodeCallers class) and data related to conflict management (now handled by IvyNodeEviction class). IvyNode now delegates to instances of these classes for these features.
        Hide
        Xavier Hanin added a comment -

        A large amount of work has been done in this area, I think Ivy source code is now much more readable and maintanable. Hence I mark this as resolved for 2.0.

        Show
        Xavier Hanin added a comment - A large amount of work has been done in this area, I think Ivy source code is now much more readable and maintanable. Hence I mark this as resolved for 2.0.

          People

          • Assignee:
            Xavier Hanin
            Reporter:
            Xavier Hanin
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development