oasp / oasp4j

The Open Application Standard Platform for Java
Apache License 2.0
60 stars 303 forks source link

Generic parent for SortBy objects #194

Closed amarinso closed 9 years ago

amarinso commented 9 years ago

As sortby objetcts (example: ProductSortBy) share common structure it could be useful to have a parent with generics.

That will reduce the need for more aggregated objects and maybe implement some common behaviour for sorting.

hohwille commented 9 years ago

Actually the entire model around that ProductSortBy is bad. This is some leftover from an earlier sample app that was refactored and needs some tidy up. Its simple and it is working but we are also planing a generic search mechanism (was proposed on the summit) including a generic dialog component to dynamically create queries on the client by adding criteria. As soon as we have this, it will supersede the current stuff like ProductSortByHitEntry, etc.

dsanche2 commented 9 years ago

I think new sorting model should be prepared for multisorting also. I don´t know how you can do multisorting in Angularjs applications but in Sencha applications you can send an array with differents multisorting fields.

ghost commented 9 years ago

I think in the Angular client we're going for ui-grid which supports sorting by multiple columns, so I see no problem here.

hohwille commented 9 years ago

For large data sets we still must keep in mind that we will use the paging support and therefore sorting has to be done via the server in that case. Therefore the client has to provide the property to sort by.

dsanche2 commented 9 years ago

In the projects I have participated the most of the use cases had a pagination list so I had to implementate a remote sorting like @hohwille says.

Other thing I was thinking is in some request server side returns a TO not based in entity fields (for performance issues for example). I´m thinking in a request to get a list from an entity with many relationships and you only want to show in client side few fields. So the situation is in your client you are representating fields that are not acopled with entity fields so the actual ordering model (or maybe the future model you have thought) based on enum used acopled to entity fields is not valid. You need "translate" your sort fields params.

I think, maybe, the best idea could be a very simple sorting model. You send sorting params with some structure and user manage this sort params in the DAO.

I hope I have expressed myself clearly..

amarinso commented 9 years ago

we are also planing a generic search mechanism (was proposed on the summit)

Do we have some information about the proposed generic mechanism?

I would assign this issue better to the 1.4.0 milestone.

And if we opt for a simpler solution we could event get it into 1.3.0 release.

We are wondering about the complexity added by having Enumerations for each sort case and it's benefits. If we go for a simpler solution where the field used for sorting is only an String we don't need to create so many classes for adding pagination to the services.

With the Enumeration type the only gain I see (please correct me) it that it is validated on the REST endpoint. But then what is used on the DAO is the value of the Enum type which is a String. Also if the name of the field is different on the client side, we should convert from one Enum to other thus two different enumeration classes have to be created.

browser --> { sortBy : "descName" } --> 
    REST SortByEntityDTO.NAME("descName") --> 
       logic SortByEntity.NAME("name")  --> 
           DAO only uses "name"

I'm proposing having only this for sorting coming from the browser:

{
  "sort" : [
    { "field" : "name", "direction": "ASC"},
    { "field" : "surname", "direction": "DESC"},
 ]
}

which can be generalized on the server side with a simple class valid for all entities.

The only downside here is that the field for sorting goes until the DAO without first validating. But for this we can provide some kind of utility method to do it (or maybe an annotation on the DTO object on the endpoing)

hohwille commented 9 years ago

I am fine with this suggestion. As we have a technological gap between client and server due to different programming languages in the end we exchange Strings anyways and compile-time safety is not given (like e.g. in a GWT project). So why enforcing to use enums? In most cases a single field for sorting would be sufficient but from a general metadata protocol I would also suggest to provide a list/array of sort fields as suggested by @amarinso. If someone wants to create an indirection from the field-names send by the client (to technical names treated as implementation secret of the server-side data-model) he can still do so by mapping the string to yet another string. For KISS we all do not need that in the sample app.

The "generic search solution" is rather a different story and should be out of scope for this issue.

amarinso commented 9 years ago

Thanks @dsanche2 for the PR

hohwille commented 9 years ago

Just before it is too late: We are going to release this as an official API now that we should try to keep stable over time. IMHO the name OrderBy is not good as it clashes with javax.persistence.OrderBy (and org.hibernate.annotations.OrderBy) and also the name does not match its semantics. I mean OrderByTo makes sense but OrderBy is not saying by which property I am ordering but specifying the sort order. Further we already have the same featured enum already on our classpath several times:

Unfortunately JPA does not offer this. So we can still keep our own copy of that enum if we want to strictly decouple but we should at least call it different. Maybe Sorting. Ordering is already 3 times on our classpath...

WDYT?

amarinso commented 9 years ago

Could it be OrderDirection as this the name of the field where it will be used?

hohwille commented 9 years ago

OrderDirection is also fine.

amarinso commented 9 years ago

Code already committed for the 1.3.0 release