Java8 Collections.sort (sometimes) does not sort JPA returned lists
Asked Answered
N

3

14

Java8 keeps doing strange things in my JPA EclipseLink 2.5.2 environment. I had to delete the question https://stackoverflow.com/questions/26806183/java-8-sorting-behaviour yesterday since the sorting in that case was influenced by a strange JPA behaviour - I found a workaround for that one by forcing the first sort step before doing the final sort.

Still in Java 8 with JPA Eclipselink 2.5.2 the following code some times does not sort in my environment (Linux, MacOSX, both using build 1.8.0_25-b17). It works as expected in the JDK 1.7 environment.

public List<Document> getDocumentsByModificationDate() {
    List<Document> docs=this.getDocuments();
    LOGGER.log(Level.INFO,"sorting "+docs.size()+" by modification date");
    Comparator<Document> comparator=new ByModificationComparator();
    Collections.sort(docs,comparator);
    return docs;
}

When called from a JUnit test the above function works correctly. When debbuging in a production environment I get a log entry:

INFORMATION: sorting 34 by modification date

but in TimSort the return statement with nRemaining < 2 is hit - so no sorting happens. The IndirectList (see What collections does jpa return?) supplied by JPA is considered to be empty.

static <T> void sort(T[] a, int lo, int hi, Comparator<? super T> c,
                     T[] work, int workBase, int workLen) {
    assert c != null && a != null && lo >= 0 && lo <= hi && hi <= a.length;

    int nRemaining  = hi - lo;
    if (nRemaining < 2)
        return;  // Arrays of size 0 and 1 are always sorted

This workaround sorts correctly:

   if (docs instanceof IndirectList) {
        IndirectList iList = (IndirectList)docs;
        Object sortTargetObject = iList.getDelegateObject();
        if (sortTargetObject instanceof List<?>) {
            List<Document> sortTarget=(List<Document>) sortTargetObject;
            Collections.sort(sortTarget,comparator);
        }
    } else {
        Collections.sort(docs,comparator);
    }

Question:

Is this a JPA Eclipselink bug or what could i generally do about it in my own code?

Please note - I can't change the software to Java8 source compliance yet. The current environment is a Java8 runtime.

I am suprised about this behaviour - it's especially annoying that the testcase runs correctly while in production environment there is a problem.

There is an example project at https://github.com/WolfgangFahl/JPAJava8Sorting which has a comparable structure as the original problem.

It contains a http://sscce.org/ example with a JUnit test which makes the issue reproducible by calling em.clear() thus detaching all objects and forcing the use of an IndirectList. See this JUnit case below for reference.

With eager fetching:

// https://mcmap.net/q/122022/-onetomany-relationship-is-not-working
@OneToMany(cascade = CascadeType.ALL, mappedBy = "parentFolder", fetch=FetchType.EAGER)

The Unit case works. If FetchType.LAZY is used or the fetch type is omitted in JDK 8 the behaviour might be different than in JDK 7 (I'll have to check this now). Why is that so? At this time I assume one needs to specify Eager fetching or iterate once over the list to be sorted basically fetching manually before sorting. What else could be done?

JUnit Test

persistence.xml and pom.xml can be taken from https://github.com/WolfgangFahl/JPAJava8Sorting The test can be run with a MYSQL database or in-memory with DERBY (default)

package com.bitplan.java8sorting;

import static org.junit.Assert.assertEquals;

import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.logging.Level;
import java.util.logging.Logger;

import javax.persistence.Access;
import javax.persistence.AccessType;
import javax.persistence.CascadeType;
import javax.persistence.Entity;
import javax.persistence.EntityManager;
import javax.persistence.EntityManagerFactory;
import javax.persistence.FetchType;
import javax.persistence.Id;
import javax.persistence.ManyToOne;
import javax.persistence.OneToMany;
import javax.persistence.Persistence;
import javax.persistence.Query;
import javax.persistence.Table;

import org.eclipse.persistence.indirection.IndirectList;
import org.junit.Test;

/**
 * Testcase for 
 * https://mcmap.net/q/121498/-java8-collections-sort-sometimes-does-not-sort-jpa-returned-lists
 * @author wf
 *
 */
public class TestJPASorting {

  // the number of documents we want to sort
  public static final int NUM_DOCUMENTS = 3;

  // Logger for debug outputs
  protected static Logger LOGGER = Logger.getLogger("com.bitplan.java8sorting");

  /**
   * a classic comparator
   * @author wf
   *
   */
  public static class ByNameComparator implements Comparator<Document> {

    // @Override
    public int compare(Document d1, Document d2) {
      LOGGER.log(Level.INFO,"comparing " + d1.getName() + "<=>" + d2.getName());
      return d1.getName().compareTo(d2.getName());
    }
  }

  // Document Entity - the sort target
  @Entity(name = "Document")
  @Table(name = "document")
  @Access(AccessType.FIELD)
  public static class Document {
    @Id
    String name;

    @ManyToOne
    Folder parentFolder;

    /**
     * @return the name
     */
    public String getName() {
      return name;
    }
    /**
     * @param name the name to set
     */
    public void setName(String name) {
      this.name = name;
    }
    /**
     * @return the parentFolder
     */
    public Folder getParentFolder() {
      return parentFolder;
    }
    /**
     * @param parentFolder the parentFolder to set
     */
    public void setParentFolder(Folder parentFolder) {
      this.parentFolder = parentFolder;
    }
  }

  // Folder entity - owning entity for documents to be sorted
  @Entity(name = "Folder")
  @Table(name = "folder")
  @Access(AccessType.FIELD)
  public static class Folder {
    @Id
    String name;

    // https://mcmap.net/q/122022/-onetomany-relationship-is-not-working
    @OneToMany(cascade = CascadeType.ALL, mappedBy = "parentFolder", fetch=FetchType.EAGER)
    List<Document> documents;

    /**
     * @return the name
     */
    public String getName() {
      return name;
    }

    /**
     * @param name the name to set
     */
    public void setName(String name) {
      this.name = name;
    }

    /**
     * @return the documents
     */
    public List<Document> getDocuments() {
      return documents;
    }

    /**
     * @param documents the documents to set
     */
    public void setDocuments(List<Document> documents) {
      this.documents = documents;
    }

    /**
     * get the documents of this folder by name
     * 
     * @return a sorted list of documents
     */
    public List<Document> getDocumentsByName() {
      List<Document> docs = this.getDocuments();
      LOGGER.log(Level.INFO, "sorting " + docs.size() + " documents by name");
      if (docs instanceof IndirectList) {
        LOGGER.log(Level.INFO, "The document list is an IndirectList");
      }
      Comparator<Document> comparator = new ByNameComparator();
      // here is the culprit - do or don't we sort correctly here?
      Collections.sort(docs, comparator);
      return docs;
    }

    /**
     * get a folder example (for testing)
     * @return - a test folder with NUM_DOCUMENTS documents
     */
    public static Folder getFolderExample() {
      Folder folder = new Folder();
      folder.setName("testFolder");
      folder.setDocuments(new ArrayList<Document>());
      for (int i=NUM_DOCUMENTS;i>0;i--) {
        Document document=new Document();
        document.setName("test"+i);
        document.setParentFolder(folder);
        folder.getDocuments().add(document);
      }
      return folder;
    }
  }

  /** possible Database configurations
  using generic persistence.xml:
    <?xml version="1.0" encoding="UTF-8"?>
    <!-- generic persistence.xml which only specifies a persistence unit name -->
    <persistence xmlns="http://java.sun.com/xml/ns/persistence"
      version="2.0">
      <persistence-unit name="com.bitplan.java8sorting" transaction-type="RESOURCE_LOCAL">
        <description>sorting test</description>
        <provider>org.eclipse.persistence.jpa.PersistenceProvider</provider>
        <exclude-unlisted-classes>false</exclude-unlisted-classes> 
        <properties>
        <!--  set programmatically -->
         </properties>
      </persistence-unit>
    </persistence>
  */
  // in MEMORY database
  public static final JPASettings JPA_DERBY=new JPASettings("Derby","org.apache.derby.jdbc.EmbeddedDriver","jdbc:derby:memory:test-jpa;create=true","APP","APP");
  // MYSQL Database
  //  needs preparation:
  //    create database testsqlstorage;
  //    grant all privileges on testsqlstorage to cm@localhost identified by 'secret';
  public static final JPASettings JPA_MYSQL=new JPASettings("MYSQL","com.mysql.jdbc.Driver","jdbc:mysql://localhost:3306/testsqlstorage","cm","secret");

  /**
   * Wrapper class for JPASettings
   * @author wf
   *
   */
  public static class JPASettings {
    String driver;
    String url;
    String user;
    String password;
    String targetDatabase;

    EntityManager entityManager;
    /**
     * @param driver
     * @param url
     * @param user
     * @param password
     * @param targetDatabase
     */
    public JPASettings(String targetDatabase,String driver, String url, String user, String password) {
      this.driver = driver;
      this.url = url;
      this.user = user;
      this.password = password;
      this.targetDatabase = targetDatabase;
    }

    /**
     * get an entitymanager based on my settings
     * @return the EntityManager
     */
    public EntityManager getEntityManager() {
      if (entityManager == null) {
        Map<String, String> jpaProperties = new HashMap<String, String>();
        jpaProperties.put("eclipselink.ddl-generation.output-mode", "both");
        jpaProperties.put("eclipselink.ddl-generation", "drop-and-create-tables");
        jpaProperties.put("eclipselink.target-database", targetDatabase);
        jpaProperties.put("eclipselink.logging.level", "FINE");

        jpaProperties.put("javax.persistence.jdbc.user", user);
        jpaProperties.put("javax.persistence.jdbc.password", password);
        jpaProperties.put("javax.persistence.jdbc.url",url);
        jpaProperties.put("javax.persistence.jdbc.driver",driver);

        EntityManagerFactory emf = Persistence.createEntityManagerFactory(
            "com.bitplan.java8sorting", jpaProperties);
        entityManager = emf.createEntityManager();
      }
      return entityManager;
    }
  }

  /**
   * persist the given Folder with the given entityManager
   * @param em - the entityManager
   * @param folderJpa - the folder to persist
   */
  public void persist(EntityManager em, Folder folder) {
    em.getTransaction().begin();
    em.persist(folder);
    em.getTransaction().commit();    
  }

  /**
   * check the sorting - assert that the list has the correct size NUM_DOCUMENTS and that documents
   * are sorted by name assuming test# to be the name of the documents
   * @param sortedDocuments - the documents which should be sorted by name
   */
  public void checkSorting(List<Document> sortedDocuments) {
    assertEquals(NUM_DOCUMENTS,sortedDocuments.size());
    for (int i=1;i<=NUM_DOCUMENTS;i++) {
      Document document=sortedDocuments.get(i-1);
      assertEquals("test"+i,document.getName());
    }
  }

  /**
   * this test case shows that the list of documents retrieved will not be sorted if 
   * JDK8 and lazy fetching is used
   */
  @Test
  public void testSorting() {
    // get a folder with a few documents
    Folder folder=Folder.getFolderExample();
    // get an entitymanager JPA_DERBY=inMemory JPA_MYSQL=Mysql disk database
    EntityManager em=JPA_DERBY.getEntityManager();
    // persist the folder
    persist(em,folder);
    // sort list directly created from memory
    checkSorting(folder.getDocumentsByName());

    // detach entities;
    em.clear();
    // get all folders from database
    String sql="select f from Folder f";
    Query query = em.createQuery(sql);
    @SuppressWarnings("unchecked")
    List<Folder> folders = query.getResultList();
    // there should be exactly one
    assertEquals(1,folders.size());
    // get the first folder
    Folder folderJPA=folders.get(0);
    // sort the documents retrieved
    checkSorting(folderJPA.getDocumentsByName());
  }
}
Nazarene answered 8/11, 2014 at 11:32 Comment(9)
Are you sure the collection you're trying to sort is not modified by some external source?Lafollette
Between docs.size() and Collections.sort(docs,comparator) there is only the constructor. My debugging shows that this might again be a JPA issue. The list is an IndirectList and the sorting seems to trust the elementCount which is zero, modcount is 2 size is 2.Nazarene
Java8 has been released more than 6 months ago. Would you really assume a bug in Java8 Collections rather than looking more closely at your own code? Use something better than basic sysouts (like JPDA) if you are desperate, but I think you should focus on your code.Zavala
Currently I make no assumptions on where the problem is. I just don't get the reason for the strange behaviour.Nazarene
I've also heard of streams-related problems where the source is from JPA. I haven't been able to pin it down, though. Is there a way you can develop a small test case that includes JPA?Orthopterous
The testcase is now added to the question. It's not really small but some what sscce. The issue is fully reproducible now.Nazarene
A bug report is issued at bugs.eclipse.org/bugs/show_bug.cgi?id=450821Nazarene
Thanks for the updates. We'll take a look.Orthopterous
See also bugs.eclipse.org/bugs/show_bug.cgi?id=433075 - it's not exactly the same thing but it seems closely related.Orthopterous
I
16

Well, this is a perfect didactic play telling you why programmers shouldn’t extend classes not designed for being subclassed. Books like “Effective Java” tell you why: the attempt to intercept every method to alter its behavior will fail when the superclass evolves.

Here, IndirectList extends Vector and overrides almost all methods to modify its behavior, a clear anti-pattern. Now, with Java 8 the base class has evolved.

Since Java 8, interfaces can have default methods and so methods like sort were added which have the advantage that, unlike Collections.sort, implementations can override the method and provide an implementation more suitable to the particular interface implementation. Vector does this, for two reasons: now the contract that all methods are synchronized expands to sorting as well and the optimized implementation can pass its internal array to the Arrays.sort method skipping the copying operation known from previous implementations (ArrayList does the same).

To get this benefit immediately even for existing code, Collections.sort has been retrofitted. It delegates to List.sort which will by default delegate to another method implementing the old behavior of copying via toArray and using TimSort. But if a List implementation overrides List.sort it will affect the behavior of Collections.sort as well.

                  interface method              using internal
                  List.sort                     array w/o copying
Collections.sort ─────────────────> Vector.sort ─────────────────> Arrays.sort
Innumerable answered 10/11, 2014 at 10:25 Comment(4)
so it's a bug. github.com/WolfgangFahl/JPAJava8Sorting now uses 2.6.0-M3 and it's reproducible that if you change the runtime the behaviour changes to "not sorting" when using lazy fetching.Nazarene
@Wolfgang Fahl: Of course, it’s a bug. I tried to explain that it is a design error which goes deeper than just failing to sort. It’s clear that the new methods removeIf(Predicate), replaceAll(UnaryOperator), forEach(Consumer) will be broken for IndirectList as well and so is the entire stream support as mentioned by Stuart Marks. And all algorithms in Collections which use these new methods (now) will break as well.Innumerable
@Wolfgang Fahl: and it’s clear that adding the required overriding methods (without changing the inheritance) would be just a hotfix. Unless the design error of subclassing Vector will be fixed, you may experience such problems with every subsequent Java release. But I don’t know whether fixing the real cause is possible as it will break compatibility with code expecting it to be a subclass of Vector (sane programmer shouldn’t do this as there is the List interface for more than fifteen years now).Innumerable
Thanks for your analysis of this. It's quite helpful. I've been hearing about potential issues with JPA and Java 8 but I hadn't managed to get any concrete information about what's going on until now.Orthopterous
N
4

Wait for the bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=446236 to be fixed. Use the below dependency when it get's available or a snapshot.

<dependency>
  <groupId>org.eclipse.persistence</groupId>
  <artifactId>eclipselink</artifactId>
  <version>2.6.0</version>
</dependency>

Until then use the workaround from the question:

if (docs instanceof IndirectList) {
    IndirectList iList = (IndirectList)docs;
    Object sortTargetObject = iList.getDelegateObject();
    if (sortTargetObject instanceof List<?>) {
        List<Document> sortTarget=(List<Document>) sortTargetObject;
        Collections.sort(sortTarget,comparator);
    }
} else {
    Collections.sort(docs,comparator);
}

or specify eager fetching where possible:

// https://mcmap.net/q/122022/-onetomany-relationship-is-not-working
@OneToMany(cascade = CascadeType.ALL, mappedBy = "parentFolder", fetch=FetchType.EAGER)
Nazarene answered 13/11, 2014 at 10:39 Comment(0)
N
3

The issue you are having is not with sort.

TimSort is called via Arrays.sort which does the following:

TimSort.sort(a, 0, a.length, c, null, 0, 0);

So you can see the size of the array TimSort is getting is either 0 or 1.

Arrays.sort is called from Collections.sort, which does the following.

Object[] a = list.toArray();
Arrays.sort(a, (Comparator)c);

So the reason your collection is not getting sorted is that it is returning an empty array. So the collection that is being used is not conforming to the collections API by returning an empty array.

You say you have a persistence layer. So it sounds like the problem is that the library you are using retrieves entities in a lazy way and does not populate its backing array unless it has to. Have a closer look at the collection you are trying to sort and see how it works. Your original unit test didn't show anything as it was not trying to sort the same collection that is used in production.

Nakitanalani answered 8/11, 2014 at 12:40 Comment(2)
You answer is somewhat on the right track. I changed my question to be more JPA/IndirectList specificNazarene
I'v updated the question with a JUnit test and a pointer to an example project on github. The IndirectList behaves like you point out. I think eager fectching might fix things and I'll try it out. Still this does not explain the difference between JDK7 and JDK8 behaviour.Nazarene

© 2022 - 2024 — McMap. All rights reserved.