Generator-jhipster: add field like to Filters

[yelhouti]

Overview of the feature request

Sometimes, it convient to filter an entity base on a pattern (usernames/firstNames for example are better suited to be filtered using startWith) it would be great to add String field to jhipster.service.filter.Filter to be able to request that without adding hacks.

Why not StringFilter, because sometime you want to filter by date: Like 2020-01-%...

• [X] Checking this box is mandatory (this is just to show you read everything)

[gzsombor]

I don't think it's good idea to:

• add 'like' field to every field type, even if databases support doing it - converting a timestamp or a number to uppercase? And doing a string match? Doesn't sound too meaningful. And I guess, this could be slow - so it won't be too hard to slow your server down, with couple of curl commands, with something like /api/myentity?id.like=%9%1%&start=10000
• making the internal, db field format of the date/timestamp/etc format public to the clients. Maybe you are storing in an international standard format, or your local, country specific format, etc. You are in a much better position, if you only allow to query by start and end date/timestamp, and not by string matches.
• Neither the usage of the '%' should be public. First, this is an implementation detail - I guess - for an average user, using *(star) as a wildcard would make a bit more sense.

If in your project, you are fine with this issues, you can implement this changes as of now. (With creating the necessary db indexes, or somehow blacklisting/whitelisting the fields, for which you support this) However, as this project is used, and will be used by hundred of projects, I think it's more important to have reasonable/sane defaults and to follow industry best practices.

[yelhouti]

• add 'like' field to every field type, even if databases support doing it - converting a timestamp or a number to uppercase? And doing a string match? Doesn't sound too meaningful. And I guess, this could be slow - so it won't be too hard to slow your server down, with couple of curl commands, with something like /api/myentity?id.like=%9%1%&start=10000

I can move it to StringFilter only, and in case someone needs it somewhere else, he can reimplement the buildSpecification... which I find cumbersome.

• making the internal, db field format of the date/timestamp/etc format public to the clients. Maybe you are storing in an international standard format, or your local, country specific format, etc. You are in a much better position, if you only allow to query by start and end date/timestamp, and not by string matches.

You are focusing on the date example, and I would also use between for that case, I was just giving an example.

• Neither the usage of the '%' should be public. First, this is an implementation detail - I guess - for an average user, using *(star) as a wildcard would make a bit more sense.

I am not proposing to show the % to the end user, the % would be added at the end by the javascript for startWith, at the beginning for endWith...

If in your project, you are fine with this issues, you can implement this changes as of now. (With creating the necessary db indexes, or somehow blacklisting/whitelisting the fields, for which you support this) However, as this project is used, and will be used by hundred of projects, I think it's more important to have reasonable/sane defaults and to follow industry best practices.

Like is very versatile, and useful for many people specially the startWith case, We have few blueprints that we use internally for specific use cases, and i don't even bother requesting a change to the main generator as it doesn't make any sense. But discarding like, just because I proposed to add it to other field types than string seems a bit excessive to me.

[yelhouti]

I would also like to note that the reason why I added like at the top, is because of the Java limitation of not being able to extend multiple classes.
A better approche would have been to add a LikeFilter (that adds the like field and methods), and use the visitor pattern (ot just checking instance of) to call it when implemented, it would also allow to have one buildSpecification function that depending on the preset filters to take them or not into account (instead of using polymorphism and supercharging).

[gzsombor]

• add 'like' field to every field type, even if databases support doing it - converting a timestamp or a number to uppercase? And doing a string match? Doesn't sound too meaningful. And I guess, this could be slow - so it won't be too hard to slow your server down, with couple of curl commands, with something like /api/myentity?id.like=%9%1%&start=10000

I can move it to StringFilter only, and in case someone needs it somewhere else, he can reimplement the buildSpecification... which I find cumbersome.

Have you seen, that there is already a 'contains' in the StringFilter? You can use it now - it even has the problem, that it won't escape the % characters in the input :(

• making the internal, db field format of the date/timestamp/etc format public to the clients. Maybe you are storing in an international standard format, or your local, country specific format, etc. You are in a much better position, if you only allow to query by start and end date/timestamp, and not by string matches.

You are focusing on the date example, and I would also use between for that case, I was just giving an example.

• Neither the usage of the '%' should be public. First, this is an implementation detail - I guess - for an average user, using *(star) as a wildcard would make a bit more sense.

I am not proposing to show the % to the end user, the % would be added at the end by the javascript for startWith, at the beginning for endWith...

When I wrote 'user' I meant the user of the API -it's the frontend developer, or the developer of an other microservice, who needs to call this API. And even if we just consider web clients - you need to handle this in the 3 official frontends.

If in your project, you are fine with this issues, you can implement this changes as of now. (With creating the necessary db indexes, or somehow blacklisting/whitelisting the fields, for which you support this) However, as this project is used, and will be used by hundred of projects, I think it's more important to have reasonable/sane defaults and to follow industry best practices.

Like is very versatile, and useful for many people specially the startWith case, We have few blueprints that we use internally for specific use cases, and i don't even bother requesting a change to the main generator as it doesn't make any sense. But discarding like, just because I proposed to add it to other field types than string seems a bit excessive to me.

Adding 'startsWith' field to the StringFilter would make more sense to me, as prefix searching could be a common pattern for auto-complete like functionality.
I suggested discarding this 'like' functionality, because you wanted to add to every field types, where I can't see any reason to do it, and because I think that bringing the SQL syntax to the URL doesn't look right, and could be a bit dangerous.
This criteria API is not about writing SQL through HTTP, GraphQL would be more adequate for that, it's about the minimal amount of code to solve 90% of the problems related to filtering entities for the frontend.

[yelhouti]

Have you seen, that there is already a 'contains' in the StringFilter? You can use it now.

Actually I can't since it automatically add % around the value I send %value% when sometime as you mentionned, we only need startWith.

you need to handle this in the 3 official frontends.

No you don't, you just ignore it, the same way you ignore greaterThan notIn... you only use what you need.

Adding 'startsWith' field to the StringFilter would make more sense to me.

The reason I proposed like, is that it avoids other people coming later and asking for endWith, or some weird match, that would be very tedious to implement (override methods in queryService..., replace all StringFilter, with myStringFilter...), having a more generic answer (that is not too generic, seemed like a nice compromise), but I will follow what the majority decides, if you guys prefer startWith with a clearly majority I will implement that instead.

and could be a bit dangerous.

Using prepared queries (which we are using) prevent against this kind of issues, and DDosing, using a % or _ seems very far fetched to me.

[gzsombor]

As this filtering feature is in JHipster for around 4 years, and in these years, people generated thousands of applications (yeah, 99% was just a POC, but...) and if I remember correctly, you are the first, who opened a ticket about adding prefix/postfix/'like' matching, I think:

• this is not a too common requirement
• the ones, who needed, after 10 minutes, figured out, that they can add it by themself to their generated code in half an hour - even with integration tests.
• the ones, who were even lazier, convinced their product owner, that 'contains' matching is perfectly fine, so they didn't have to touch the code :wink:

Sooo as we try to avoid adding trivially implemented features and features which are rarely needed, because it's just add unnecessary code and increase the maintenance overhead for everyone, I'm not convinced that JHipster needs that generic like filter, maybe the 'startsWith' could be useful, but if other JHipster-devs could share their experience, how often do they needed prefix string matching, and what they did it, as they don't complain here in tickets/PR :smile:

[yelhouti]

@gzsombor I generated few applications that are now in production with JHipster I added code manually to fix the use case. BUT, it is ugly and I need replace the extends QueryService with MyQueryService and StringFilter with MyStringFilter everytime I generate a new entity (which is just a waste of time).

I have also been guilty of: "convinced their product owner, that 'contains' matching is perfectly fine"

I am ok implementing only startWith, but I don't think it's the best course of action.

If the startWith is a good enough solution for the person who will merge, will do just that.

Anyone able and willing to merge any of these cares to give it's opinion please?

[gzsombor]

@yelhouti I sympathize with you, as I maintained too a JHipster fork for 1-2 years, because we wanted to generate not 1, but 3 (or 4) different DTOs for every entitiy. With mappers. And I don't think, that was unreasonable :wink:
We had

• 1 DTO for the entity listing - every fields plus all the parent relations with only id + name field
• 1 DTO for the details - every fields, plus all the child entities with their 'DetailDTO's, and all the parent entities with only id + name field
• 1 DTO for the update/create call - every fields plus all the child entities with their UpdateDTO and id fields for the parent entities
• 1 DTO for ElasticSearch.

The generated code was pretty good,and made our life much easier, but I haven't upstreamed it, because the general consensus was that even one DTO is good enough for most of the users, and it's rare that someone is aiming such completeness (and honestly, the generated code was not perfect, it needed manual editing, as to handle some subtleties, corner cases that would made the generator much more complex, would needed to add a lot of questions - manually editing the code was much easier, and faster.
So long story short: I prefer if we just target the 95% correctness, and not over generalize the solution to cases which is not yet requested by anyone

[yelhouti]

@gzsombor I would totally agree if the added code was too complex or made the generator harder to maintain, but it's none of the above.
Anyway, if like is not accepted, I would still argue of startWith.