Back contributions from integrations (MGNLDAM-549)

[MGNLDAM-546] AssetQuery: add sorting capabilities Created: 16/Feb/15  Updated: 25/Jun/15  Resolved: 12/Jun/15

Status: Closed
Project: Magnolia DAM Module
Component/s: None
Affects Version/s: None
Fix Version/s: 2.1

Type: Sub-task Priority: Neutral
Reporter: Robert Šiška Assignee: Robert Šiška
Resolution: Fixed Votes: 0
Labels: api
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Relates
relates to MGNLDAM-545 AssetQuery: add offset Closed
dependency
depends upon MGNLDAM-411 Support custom metadata Closed
is depended upon by MGNLDAMEXT-32 List view Closed
duplicate
duplicates MGNLDAM-405 AssetQuery: add orderBy Closed
Template:
Date of First Response:

 Description   

Add ability to specify sorting in query.

Rationale: Decoupling UI and JCR yields the need for abstract query statements directed to AssetProviders. AssetQuery fulfills this role, but it lacks sorting ability.



 Comments   
Comment by Magnolia International [ 20/Mar/15 ]

Similarly to MGNLDAM-545, we need more specification here. In particular, the propertyId should be specified, and, arguably, typed. Instead of an arbitrary String, which we can't know will be supported across providers ("name" will mean something different for each provider (it's not even a property of the Asset interface) : I'd rather limit that to an enum of the basic properties defined on the Asset interface.

Similarly, I'd rather use an enum for asc/desc rather than a boolean.

A couple more remarks:

  • AssetQuery.OrderBy probably doesn't need to be public, but it should be static, unless there's a reason not to.
  • Its fields should be final.
  • Your API could be simpler to use with orderBy(String propertyId, boolean isAscending) instead of withSorter(OrderBy... sorters): https://gist.github.com/gjoseph/ae08f0e21a1d05a2846a

And as an example, using the suggested enum; to be completed, documented, validated, etc.. : https://gist.github.com/gjoseph/811015992ed488a18edd

Comment by Magnolia International [ 20/Mar/15 ]

Also keep the following in mind:

  • In real life, those sorting methods will many times be too weak anyway - they rely on "natural order" for the given properties, which is very loosely defined. What if you want case-insensitive sorting, etc etc.
  • I would rather do .sortWith(Comparator<Asset>), but that doesn't translate well to remote queries
  • Especially for remote providers, we need to build a caching layer. When most (or all) providers are caching, then we can think about utilizing modern cache/index querying capabilities.
Comment by Robert Šiška [ 27/Mar/15 ]

I know that the enum would be a bit nicer, but I think that limiting the sort to some set of properties just won't cut it.

Note that the need for these changes arose from the real life integrations, not just out of my whim. Out of three recent integrations (wcs,cumulus,konakart), all use offset/maxResults pagination & ordering by the name of the property + asc/desc. Some of them expect to be able to sort queries by properties like price or ratings.

So we either leave it as String, which is ugly & loosely defined or we need to rethink the idea of "asset property" altogether. How about sorting by metadata properties? We still need to redo metadata anyway...

Comment by Robert Šiška [ 12/Jun/15 ]

OrderBy now takes the metadata class as argument.

Commited in:
https://git.magnolia-cms.com/gitweb/?p=modules/dam.git;a=commitdiff;h=6c628f9749259862d579e943927c2f25a64d6960

Generated at Mon Feb 12 05:00:55 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.