Axiom
  1. Axiom
  2. AXIOM-333

getFirstChildWithName should not read the next element.

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.2.11
    • Component/s: DOOM, LLOM
    • Labels:
      None

      Description

      When calling getFirstChildWithName operation over an element, it reads not only the first element, but the next also. I think that could be improved since the operation name semantics specifies that the caller is only interested in the first element and not the rest of them, so a iterator (the reason to read the next element) is useless here. Suppose the following scenario.

      <root-element>
      <chlid-element att="value">
      <sub-child-element>
      – very big content –
      </sub-child-element>
      </child-element>
      </root-element>

      I want to read the sub-child-element only in some cases depending on the 'value' attribute. If I call to getFirstElement I get the behaviour I want, since only the header is readed and I can skip and discard the message if it doesn't match. I get the behaviour that I would expect from a StaX parser and a getFirst* method, so I can discard the message quickly. Instead, getFirstElementWithName seems to call internally to getElementsWithName and returns the first occurence, which forces the parser to read the next element and (in this case) the complete big message.
      I think that the efficiency of that method could be improved since, for a StaX model, one expects that the parser only reads the minimal information needed to serve the request. Instead of calling to getElementsWithName, it would be better an implementation based on getFirstElement and then going through getNextOMSibling.

        Issue Links

          Activity

          Hide
          Andreas Veithen added a comment -

          I think that the issue is in OMChildrenQNameIterator#next(), which advances to the next child before returning the current child. Advancing to the next child should only be done in hasNext().

          Since this is only an optimization question and there is a risk of breaking things, let's schedule it for 1.2.9, i.e. for the next minor release after the one planned for this month.

          Show
          Andreas Veithen added a comment - I think that the issue is in OMChildrenQNameIterator#next(), which advances to the next child before returning the current child. Advancing to the next child should only be done in hasNext(). Since this is only an optimization question and there is a risk of breaking things, let's schedule it for 1.2.9, i.e. for the next minor release after the one planned for this month.
          Hide
          Andreas Veithen added a comment -

          Postponing to 1.2.10 since the issue is blocked by WSCOMMONS-449, which is scheduled for 1.2.10.

          Show
          Andreas Veithen added a comment - Postponing to 1.2.10 since the issue is blocked by WSCOMMONS-449 , which is scheduled for 1.2.10.
          Hide
          Andreas Veithen added a comment -

          Fixed by r1054444.

          Show
          Andreas Veithen added a comment - Fixed by r1054444.
          Hide
          Hudson added a comment -

          Integrated in ws-axiom-trunk #317 (See https://hudson.apache.org/hudson/job/ws-axiom-trunk/317/)

          Show
          Hudson added a comment - Integrated in ws-axiom-trunk #317 (See https://hudson.apache.org/hudson/job/ws-axiom-trunk/317/ )
          Hide
          Olve Sæther Hansen added a comment -

          I see that this issue is long-fixed. It is not the issue I want to discuss, but the fix. Isn't removing protected variables from the iterator in the API-part of axiom considered an API-change? I use Abdera, which subclasses some iterators. I am forced to upgrade from 1.2.10 to 1.2.12 version of Axiom, and Abdera breaks.
          I have reported a bug in Abdera ABDERA-290, but now I am unsure whether it should be reported here instead.

          Show
          Olve Sæther Hansen added a comment - I see that this issue is long-fixed. It is not the issue I want to discuss, but the fix. Isn't removing protected variables from the iterator in the API-part of axiom considered an API-change? I use Abdera, which subclasses some iterators. I am forced to upgrade from 1.2.10 to 1.2.12 version of Axiom, and Abdera breaks. I have reported a bug in Abdera ABDERA-290 , but now I am unsure whether it should be reported here instead.
          Hide
          Andreas Veithen added a comment -

          OMChildrenIterator is part of axiom-api because it is code shared by different Axiom implementations [1]. That means that it should not be considered part of the public API, but rather part of the support classes for Axiom implementations. We maintain compatibility of the public API between minor releases, but there may be changes required in the Axiom implementations, i.e the SPI may slightly change.

          Unfortunately, since axiom-api contains both the public API and classes shared by different implementations, the distinction between these two got a bit blurred over time. This is something that will be fixed in Axiom 1.3.

          Anyway, since Abdera is an Axiom implementation that extends the default Axiom LLOM implementation (axiom-impl) it is strongly coupled to Axiom's internals and upgrading the Abdera build to a new Axiom version will always require some changes.

          In my opinion, the real problem is that this issue also occurs at runtime. Actually, Abdera should not have a dependency on axiom-impl at runtime, but instead include a (relocated) copy of these classes (using the maven-shade-plugin). This is also the reason why currently Abdera and Axiom don't play nicely together in an OSGi environment [2].

          To summarize, the structural solution to the problem that Abdera in general breaks after upgrading Axiom is to:

          • ensure that there is a clear separation between the public Axiom API and implementation classes, and move implementation classes out of axiom-api (so that axiom-api can be upgraded safely);
          • eliminate the runtime dependency on axiom-impl in Abdera (so that upgrading axiom-impl will never be a problem).

          Obviously, that is a long term solution. In the meantime, I can try to come up with a patch that updates Abdera to Axiom 1.2.13-SNAPSHOT.

          [1] http://markmail.org/message/acrwzktvoulde22u
          [2] https://builds.apache.org/view/S-Z/view/WS/job/ws-axiom-trunk/site/devguide/ch02.html#osgi-ref-impl-not-exported

          Show
          Andreas Veithen added a comment - OMChildrenIterator is part of axiom-api because it is code shared by different Axiom implementations [1] . That means that it should not be considered part of the public API, but rather part of the support classes for Axiom implementations. We maintain compatibility of the public API between minor releases, but there may be changes required in the Axiom implementations, i.e the SPI may slightly change. Unfortunately, since axiom-api contains both the public API and classes shared by different implementations, the distinction between these two got a bit blurred over time. This is something that will be fixed in Axiom 1.3. Anyway, since Abdera is an Axiom implementation that extends the default Axiom LLOM implementation (axiom-impl) it is strongly coupled to Axiom's internals and upgrading the Abdera build to a new Axiom version will always require some changes. In my opinion, the real problem is that this issue also occurs at runtime. Actually, Abdera should not have a dependency on axiom-impl at runtime, but instead include a (relocated) copy of these classes (using the maven-shade-plugin). This is also the reason why currently Abdera and Axiom don't play nicely together in an OSGi environment [2] . To summarize, the structural solution to the problem that Abdera in general breaks after upgrading Axiom is to: ensure that there is a clear separation between the public Axiom API and implementation classes, and move implementation classes out of axiom-api (so that axiom-api can be upgraded safely); eliminate the runtime dependency on axiom-impl in Abdera (so that upgrading axiom-impl will never be a problem). Obviously, that is a long term solution. In the meantime, I can try to come up with a patch that updates Abdera to Axiom 1.2.13-SNAPSHOT. [1] http://markmail.org/message/acrwzktvoulde22u [2] https://builds.apache.org/view/S-Z/view/WS/job/ws-axiom-trunk/site/devguide/ch02.html#osgi-ref-impl-not-exported
          Hide
          Andreas Veithen added a comment -

          Small correction to my comment above: actually the OSGi bundles produced by Abdera are correct (although I didn't test them) since the abdera-parser bundle embeds axiom-api and axiom-impl. The issue with OSGi occurs only with the repackaged Abdera bundles produced by the ServiceMix project (see SMX4-877).

          Show
          Andreas Veithen added a comment - Small correction to my comment above: actually the OSGi bundles produced by Abdera are correct (although I didn't test them) since the abdera-parser bundle embeds axiom-api and axiom-impl. The issue with OSGi occurs only with the repackaged Abdera bundles produced by the ServiceMix project (see SMX4-877 ).

            People

            • Assignee:
              Andreas Veithen
              Reporter:
              Jose Antonio
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development