Announcement Announcement Module
Collapse
No announcement yet.
Poor design of sorting in API from Spring Data MongoDB Page Title Module
Move Remove Collapse
X
Conversation Detail Module
Collapse
  • Filter
  • Time
  • Show
Clear All
new posts

  • Poor design of sorting in API from Spring Data MongoDB

    org.springframework.data.mongodb.core.query.Sort
    and also
    org.springframework.data.domain.Sort


    I consider it bad API design to have two classes with identical names within sub-packages of "org.springframework.data".

    In the following code, the fully qualified name of Sort has had to be referenced in-line in the Java code because both Sort classes are used within the same code sample.

    This code sample is not an unreasonable design; the two methods were written independently following advice by the Spring Data development team members on Stack Overflow.
    Code:
    import java.util.List;
    import org.springframework.data.domain.PageRequest;
    import org.springframework.data.domain.Sort;
    import org.springframework.data.mongodb.core.query.Order;
    import org.springframework.beans.factory.annotation.Autowired;
    import org.springframework.stereotype.Service;
    
    @Service
    public class UserService {
        @Autowired private BookingRepository bookingRepository;
    
        public List<Booking> getByUserId(String userId) {
            return bookingRepository.findByUserId(userId, 
                   new org.springframework.data.mongodb.core.query.Sort("submittalDate", Order.DESCENDING));
        }    
        
        public List<Booking> findAll() {
             PageRequest request = new PageRequest(0, 1, new Sort(Sort.Direction.DESC, "submittalDate"));
             return bookingRepository.findAll(request).getContent();
        }
    }
    Secondly, there is not even consistency in constructor parameter order:

    new org.springframework.data.mongodb.core.query.Sort(" submittalDate", Order.DESCENDING)
    versus
    new org.springframework.data.domain.Sort(Sort.Directio n.DESC, "submittalDate")



    Third, there is not even consistency in Order/Direction designs:

    Order.DESCENDING
    versus
    Sort.Direction.DESC


    Advice by Spring Data development team members on Stack Overflow does not show imports (this adheres to general Stack Overflow convention of not showing imports).


    However, as the examples above show, the correct imports are utterly crucial.

    i) The above issues reduce the learnability of sorting in MongoDB Spring Data.

    ii) Even if the correct code imports are eventually discovered via reading APIs, source code and checking each import, the code is a mess due to the fully qualified name of Sort has had to be referenced in-line in the Java code.


    Suggestions:
    • Changing a publicly published API is a serious issue, but in my opinion this sorting API is not fit for final release publication in the first place!
    • org.springframework.data.mongodb.core.query.Sort could be renamed as MongoSort, giving it a unique class name within sub-packages of "org.springframework.data".
    • Attempting to tidy up the other inconsistency issues
    Last edited by Alba007; Sep 12th, 2012, 07:17 PM.

  • #2
    First of all, the code you are showing above is not code one would ever write anyway. The repository API does not know anything about MongoDB in the first place. Hence it only understands org.springframework.data.domain.Sort anyway. Thus you don't run into a situation where you need both imports or have to deal with both Sort types at the same time. Still, I understand your issue.

    The reason these APIs differ a bit is that the MongoDB query API is designed to be a fluent one compared to the one in the repositories package which is a plain value object. Also, both APIs we're created in the first place without knowing about each other. So I agree this is a place to be improved, which we will definitely do for the next major release. We'll probably move into the direction of rather using the core domain type and deprecating the usage of the MongoDB specific Sort so that we can fluently face out the letter in subsequent releases.

    Comment


    • #3
      I've created and fixed a JIRA [0, 1] regarding this. We now have dedicated with(…) methods for Spring Data Commons' Sort and Pageable. The legacy sort() method is deprecated and will be removed in the next major release. Code is available in the current snapshot build.

      [0] https://jira.springsource.org/browse/DATAMONGO-538
      [1] https://github.com/SpringSource/spri...0d72ba183a312d

      Comment


      • #4
        That's a great response, thanks.

        I recognise all the masses of hard work that you and your team have put into MongoDB Spring Data.

        Documentation on Sorting in Repositories

        From my "new user" perspective, I feel that documentation on sorting within MongoDB Spring Data repositories is lacking.

        For example, this answer in Stack Overflow doesn't show imports (ambiguous) and what was posted as:
        Code:
        new Sort(Order.ASC, "age")
        ...is, less than 11 months later, the following (a different class in first parameter!)...

        Code:
        new Sort(Sort.Direction.ASC, "age")
        I fully appreciate the need to change and evolve the public face of MongoDB Spring Data, but as I could not find the solution to MongoDB Spring Data repositories sorting within official reference documentation such as sorting "4.3.2.3. Special parameter handling" section from 1.0.4.RELEASE @ http://static.springsource.org/sprin...ial-parameters

        Quote from official docs:
        If you only need sorting, simply add a Sort parameter to your method.
        "Simply" is ironic here.

        Not showing imports is an issue here as we cannot rely on compile-time strong typing as the implementation is dynamically created and there are multiple matches for "Sort" inside Spring Data packages.

        Thanks, the JIRA https://jira.springsource.org/browse/DATAMONGO-538 will help resolve this.

        Perhaps at some point, "4.3.2.3. Special parameter handling" section could be fleshed out with a few more sentences, perhaps showing the imports and a one-line example of how to call such a findByLastname method, with the relevant new Sort(Sort.Direction.ASC, "age") construction provided?

        Sorting is one of the many areas of critical importance, after all. Thanks.

        Comment


        • #5
          That's very valuable input, thanks a lot. Here's a part of the story: both the StackOverflow sample you linked to as well as the parts of the reference documentation discuss the Spring Data repository abstraction in general. These types are located in the Spring Data Commons project, which is the reason why we didn't really think discussing a Sort without context is a problem at all. So there are essentially two issues here:

          1. We're (the documentation writers and StackOverflow post authors in particular) are probably too deep into the matter to see the problem, as we're simply dealing with the codebase on a day to day basis. That's why it's awesome to have people like you bringing things like these to the table and help improving the overall experience.
          2. The general ambiguity only comes from the fact that the MongoDB Query API introduced a second Sort type which was probably a bad thing to begin with. Other stores simply don't offer a different sort, hence there's no ambiguity.

          I've created a ticket [0] against Spring Data Commons (which contains the relevant section of the reference documentation) and have this fixed with the upcoming GA releases. This should improve the reference documentation experience and avoid ambiguities. The issue wil be eventually resolved with the next GA release (not the one shipping in a week or so) when we remove the custom MongoDB Sort type entirely.

          Thanks for your input and don't hesitate to bring other stuff like this to our attention. Feedback is highly appreciated!

          [0] https://jira.springsource.org/browse/DATACMNS-230

          Comment


          • #6
            I have to remark on this thread. I have been one who has "complained" a bit about some of the documentation in the Spring Data projects, but really just to help make them better. I also understand that documentation takes time to write and revision to make better and better each time. The guys writing the Spring Data documentation are the devs, which is great in one respect in that they know the subject matter. But at the same time they are devs and not professional technical writers. Heck I can't write for anything, so that they took the time and did their best, I am really appreciative of what they have accomplished.

            Can it be better, of course, but at the same time it is still great work on their part. I also highly recommend the new O'Reilly book on Spring Data that these guys on the Spring Data team wrote. I think it puts all the Spring Data projects into perspective, so you get the common basics first, then get into each project individually in detail. I also think because of it being a released book by a publisher, it got the benefit of editors to help make it a bit better than the core documentation. (And some tech reviewers, like me. I saw today that I got a mention in the beginning of the book, thanks so much guys)

            Thanks

            Mark

            Comment

            Working...
            X