Camel
  1. Camel
  2. CAMEL-547

remove cycle between builder and spi

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Duplicate
    • Affects Version/s: 1.3.0
    • Fix Version/s: 1.4.0
    • Component/s: camel-core
    • Labels:
      None

      Description

      Currently the spi package references builder.RouteBuilder. This creates a bad cycle in Camel as builder is one of the top level packages and spi is one of the lowest level packages. The method where RouteBuilder is used is in CamelContext.addRoutes(RouteBuilder builder). I think this reference is not necessary. CamelContext already has a method addRoutes(Collection<Route> routes).

      So instead of calling camelContext.addRoutes(builder) I think the better way would to call:
      builder.setContext(camelContext);
      builder.addRoutesToContext(); // This is a new method that replaces the functionality of CamelContext.addRoutes(RouteBuilder builder)

      This way we will also cut some part of the cycle between impl and builder. impl.DefaultCamelContext references both RouteBuilder and BuilderSupport from the builder package. Both will not be necessary anymore as as soon as we get rid of the addRoutes method.

      This way CamelContext does not have to know anything about it´s builder. What do you think about this change?

      1. Main cluster (11).png
        27 kB
        Christian Schneider
      2. routecontext.patch
        2 kB
        Christian Schneider

        Issue Links

          Activity

          Hide
          Christian Schneider added a comment -

          This image shows the dependencies in camel core. The model package is left out to make the graph easier to read. What we see is that spi references builder and impl also refrences builder. Both dependencies can be cut.

          Show
          Christian Schneider added a comment - This image shows the dependencies in camel core. The model package is left out to make the graph easier to read. What we see is that spi references builder and impl also refrences builder. Both dependencies can be cut.
          Hide
          Hadrian Zbarcea added a comment -

          It's actually between camel and builder. This sound like a reasonable solution, will take a looksie later today.

          I just committed a change that removes dependencies between util, converter and processor.

          Show
          Hadrian Zbarcea added a comment - It's actually between camel and builder. This sound like a reasonable solution, will take a looksie later today. I just committed a change that removes dependencies between util, converter and processor.
          Hide
          Hadrian Zbarcea added a comment -

          @Christian, addRoutes(RouteBuilder) was used in many places and the usage pattern was something like this:
          addRoutes(new RouteBuilder()

          {...}

          ), so your solution was not quite appealing. I solved it by introducing a Routes interface in oac.

          I will keep this issue open for another few days, i think there is at least one dependency that could be untangled.

          Show
          Hadrian Zbarcea added a comment - @Christian, addRoutes(RouteBuilder) was used in many places and the usage pattern was something like this: addRoutes(new RouteBuilder() {...} ), so your solution was not quite appealing. I solved it by introducing a Routes interface in oac. I will keep this issue open for another few days, i think there is at least one dependency that could be untangled.
          Hide
          Christian Schneider added a comment -

          Hi Hadrian,

          your solution is in any case much better than the Camel 1.3 code but I would suggest to try to remove the addRoutes(RouteBuilder) in the long run. Why not mark the addRoutes method as deprecated and document in the method header the code I meantioned above as the way people should change their code to. So the addRoutes could be set for removal in Camel 1.5 or perhaps in 2.0.

          Perhaps an even better solution that I mentioned above would be to simply have the pattern:
          builder.addRoutesToContext(camelContext);

          So the RouteBuilder does no have to know the CamelContext for a longer time. I think it could even be possible that we remove all other references to CamelContext from RouteBuilder.

          The current code is really ugly. If you look at DefaultCamelContext.addRoutes(Routes builder). It calls RouteBuilder.getRouteList().
          This method calls checkinitialized which calls camelContext.addRouteDefinitions(routeCollection.getRoutes()). As you see the calls jump back and forth between RouteBuilder and DefaultCamelContext. There must be a cleaner way to achieve this. I will try to find a solution for this. Though I guess we are leaving the area of the low hanging fruits here

          Show
          Christian Schneider added a comment - Hi Hadrian, your solution is in any case much better than the Camel 1.3 code but I would suggest to try to remove the addRoutes(RouteBuilder) in the long run. Why not mark the addRoutes method as deprecated and document in the method header the code I meantioned above as the way people should change their code to. So the addRoutes could be set for removal in Camel 1.5 or perhaps in 2.0. Perhaps an even better solution that I mentioned above would be to simply have the pattern: builder.addRoutesToContext(camelContext); So the RouteBuilder does no have to know the CamelContext for a longer time. I think it could even be possible that we remove all other references to CamelContext from RouteBuilder. The current code is really ugly. If you look at DefaultCamelContext.addRoutes(Routes builder). It calls RouteBuilder.getRouteList(). This method calls checkinitialized which calls camelContext.addRouteDefinitions(routeCollection.getRoutes()). As you see the calls jump back and forth between RouteBuilder and DefaultCamelContext. There must be a cleaner way to achieve this. I will try to find a solution for this. Though I guess we are leaving the area of the low hanging fruits here
          Hide
          Christian Schneider added a comment -

          Small modifications in RouteContext and RouteType to make initialization easier and cleaner

          Show
          Christian Schneider added a comment - Small modifications in RouteContext and RouteType to make initialization easier and cleaner
          Hide
          Christian Schneider added a comment -

          Hi Hadrian,

          I have just added a patch for DefaultRouteContext to make the dependependency on CamelContext more transparent. I also change RouteType and moved some of the things that should not happen there to DefaultRouteContext. Perhaps you can take a look at it.

          Show
          Christian Schneider added a comment - Hi Hadrian, I have just added a patch for DefaultRouteContext to make the dependependency on CamelContext more transparent. I also change RouteType and moved some of the things that should not happen there to DefaultRouteContext. Perhaps you can take a look at it.
          Hide
          Christian Schneider added a comment -

          I have created a new issue for this problem with a better solution than I suggested before.

          Show
          Christian Schneider added a comment - I have created a new issue for this problem with a better solution than I suggested before.
          Hide
          Christian Schneider added a comment -

          Please see the newer issue with the better patch

          Show
          Christian Schneider added a comment - Please see the newer issue with the better patch

            People

            • Assignee:
              Hadrian Zbarcea
              Reporter:
              Christian Schneider
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 3h
                3h
                Remaining:
                Remaining Estimate - 3h
                3h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development