Ivy
  1. Ivy
  2. IVY-435

LatestRevisionStrategy.sort() doesn't sort as specified

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0.0-alpha-1
    • Component/s: Core
    • Labels:
      None

      Description

      According to the LatestStrategy.sort() javadoc, the sort method should sort the artifacts from latest to oldest.
      However, the LatestRevisionStrategy.sort() method does the opposite!

      The following junit test shows this (add to LatestRevisionStrategyTest.java):

          public void testSort() {
              ArtifactInfo[] revs = toMockAI(new String[] {
                      "0.2a", 
                      "0.2_b", 
                      "0.2rc1", 
                      "0.2-final", 
                      "1.0-dev1", 
                      "1.0-dev2", 
                      "1.0-alpha1", 
                      "1.0-alpha2", 
                      "1.0-beta1", 
                      "1.0-beta2", 
                      "1.0-gamma",
                      "1.0-rc1",
                      "1.0-rc2",
                      "1.0", 
                      "1.0.1", 
                      "2.0" 
                      });
              
              List shuffled = new ArrayList(Arrays.asList(revs)); 
              ArtifactInfo[] shuffledRevs = (ArtifactInfo[]) shuffled.toArray(new ArtifactInfo[revs.length]);
              
              LatestRevisionStrategy latestRevisionStrategy = new LatestRevisionStrategy();
              List sorted = latestRevisionStrategy.sort(shuffledRevs);
          	assertEquals(Arrays.asList(revs), sorted);
          }
      

      The question is: should we change the javadoc or the LatestRevisionStrategy.sort method?

        Activity

        Hide
        Maarten Coene added a comment -

        Changed javadoc on sort() method and updated the AbstractLatestStrategy.findLatest() method which assumed the latest revision came first.

        This bug in the findLatest() method wasn't noticed because the ComparatorLatestStrategy overrides this method. Since this isn't necessary (it is already implemented in the AbstractLatestStrategy class and I didn't see any performance optimisations in the subclass), I've removed this method in the ComparatorLatestStrategy class.

        Show
        Maarten Coene added a comment - Changed javadoc on sort() method and updated the AbstractLatestStrategy.findLatest() method which assumed the latest revision came first. This bug in the findLatest() method wasn't noticed because the ComparatorLatestStrategy overrides this method. Since this isn't necessary (it is already implemented in the AbstractLatestStrategy class and I didn't see any performance optimisations in the subclass), I've removed this method in the ComparatorLatestStrategy class.
        Hide
        Xavier Hanin added a comment -

        This is weird, since all LatestStrategies implementation in Ivy provide the same order. So there are things to fix, maybe more than just the javadoc

        Show
        Xavier Hanin added a comment - This is weird, since all LatestStrategies implementation in Ivy provide the same order. So there are things to fix, maybe more than just the javadoc
        Hide
        Maarten Coene added a comment -

        I took a quick look at this last night, and at first sight it seemed that some code depending on this sort() method was assuming it was sorted as stated by the javadoc and some other code was assuming it was sorted as the actual implementation.

        I'll try to investigate this further next week.

        Show
        Maarten Coene added a comment - I took a quick look at this last night, and at first sight it seemed that some code depending on this sort() method was assuming it was sorted as stated by the javadoc and some other code was assuming it was sorted as the actual implementation. I'll try to investigate this further next week.
        Hide
        Xavier Hanin added a comment -

        I think we should modify the javadoc, because otherwise it would break a lot of code depending on the actual implementation, and not on the javadoc

        Show
        Xavier Hanin added a comment - I think we should modify the javadoc, because otherwise it would break a lot of code depending on the actual implementation, and not on the javadoc

          People

          • Assignee:
            Maarten Coene
            Reporter:
            Maarten Coene
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development