[Isomorphic]

All good.

Just to further note, you could have your RESTHandler-related behavior in a DataSource subclass too - just check dsRequest.isREST().

And yes, you should definitely take a look at the new Reify sample apps when they are available (or we can get you early access - just ask).

These are apps with things like-preconfigured saved searches, dashboards w/ aggregation, custom tiled views, custom expansion records, cross-relation validations, workflows that make server trips and decide what to based on the response - basically a tour-de-force of the powerful things a capable developer can do with SmartGWT.

Except they were created by a non-developer, with no code at all.

[Blama]

Hi Isomorphic,

thanks.
You are right, this very check should be part of a DataSource-Subclass, and in fact I already do have this for logging and addToTemplateContext-calls.

My IDACall and RESTHandler subclasses started when I started to use RESTHandler. I don't want to allow access to all operation-bindings via RESTHandler, so I have a check similar to serverOnly, called apiAllowed - this I need to check in MyRESTHandler, not MyDataSource.

All the other checks (only allow white listed operationType/operationId-combinations, the requiredEqualsCriteria I'm suggesting) could go on MyDataSource, you are right. I might look into moving them there.

W.r.t. setFailure() vs. just not returning results: Thanks on the best practice here. I might change this as well.

W.r.t. DynamicDSGenerator: I read the QSG again and you have a chapter "Why focus on .ds.xml files?" which is what I meant. Another mention of DynamicDSGenerator there and why to use it (it vastly reduces the amount of stupid setCanEdit() etc. clientside code). In theory I think that with DynamicDSGenerator + Component XML/Reify one in the most extreme case has 0 lines of JavaScript (in case of SmartGWT 0 lines of client side Java).

Best regards
Blama

[Isomorphic]

You continue to refer to subclassing both IDACall and RESTHandler, when the correct technique is to make a DataSource subclass, in which case you only need a single subclass, and it's easy to make your custom behavior configurable via custom .ds.xml properties.

So, your approach is not mentioned in the docs because it's not the recommended approach. However, the recommended approach - using a DataSource subclass - is mentioned in the QuickStart and multiple other places in the docs.

DynamicDSGenerator is also mentioned in the QuickStart, but that's for a different use case.

Finally, regarding setFailure() vs just not returning results: our understanding was that this is a security check, designed to reject requests that clearly indicate a user is trying to tamper with data. In that situation, if you talk to a security professional, they will tell you that clear error reporting is a non-goal, because the error reporting gives the attacker information.

In fact, the error message is likely to be a natural language description of the security rule that was implemented, which gives an attacker insight into how you might have structured the code.

So, counter-intuitive perhaps, but when rejecting a request that clearly indicates someone was trying to hack your system, it's just as good, or even better, for the attempt to fail in a mysterious way.

[Blama]

Hi Isomorphic,

thanks, all good here :)
As I said in #10, I did it in DMI first, but then switched to

public class MyIDACall extends IDACall implements OperationBindingAccessCheckInterface
and
public class MyRESTHandler extends RESTHandler implements OperationBindingAccessCheckInterface
and a OperationBindingAccessCheckInterface with a few default methods that I use in my handleDSRequest() overrides.

But as I said in #15 this is definitely not obvious for someone starting a new project. And even if it was I'd think this would be a nice addition to the framework.
W.r.t. of adding criteria vs. returning return new DSResponse.setFailure(...) I definitely think that if you add it to the framework you should do it in the latter way.
Not executing a request is always better than executing a request that then returns no rows. Also you this would result in some (perhaps useful) log entry in the server log.

Regarding:

if you've been copying and pasting code around, please stop! SmartClient was specifically designed to avoid that

100% approval. Important for this is use DynamicDSGenerator extensively (plus DataSourceLoader caching with cache breakers for Version, User, Language and possibly configuration changes) and subclass IDACall. I'm still suffering from not using DynamicDSGenerator because it would mean a big refactoring.
These two points (DynamicDSGenerator, subclass IDACall) really should be prominently in the Quick Start Guide (if they are not already, did not read it for a long time).

Best regards
Blama

[Isomorphic]

This seems to have the same solution we just described, but you just do it for multiple fields.

If it wasn't already clear, if the client's request tried to use the wrong operator, the top-level equals Criterion you added is going to mean that fails. The attempt to use a different operator becomes irrelevant, because there is already a top-level Criterion using equals. The DB will basically never execute the portion of the WHERE clause where the wrong operator was used.

Whether you like this particular solution or prefer to solve it with additional code, there is absolutely no reason for "boilerplate" code being repeated in multiple DMIs: just make a DataSource subclass that implements your check based on a custom attribute in your .ds.xml. Then you can simply declare the fields that should be checked in this way, for any of your DataSources, exactly the same way it would work if it were a built-in feature instead.

So if you've been copying and pasting code around, please stop! SmartClient was specifically designed to avoid that, and it troubles us to hear of it ;)

[Blama]

Hi Isomorphic,

thanks, but this still does not help. Here is my use case:

I have an entity that can be read by many users, but what entries can be seen by what user differs.
Users will send fetch request from the GUI for single records of the entity. I then need to get the criteria for the id field and do some checks with it.
If the check is OK, I do return request.execute(), if the check is not OK, I do return new DSResponse.setFailure(...).

In order to do the check I need the value of the equals criteria for the field in question.
What I'd like is that in my DMI I can do if (mycheck(dsRequest.getCriteriaValue(fieldName))), but for that dsRequest.getCriteriaValue() needs to return the right criteria.

What the suggested attribute would do is make sure that dsRequest.getCriteriaValue() can do this. This means:

• Check there is exactly one criteria for every field in that list, and that criteria is equals
• Check that mentioned criteria is top-level (no OR, no NOT)

Without this built in it means a lot of boilerplate in DMIs or implementing it in IDACall / RESTHandler, definitely not something everyone would think of.
I and think you must already have something similar for the UPDATE and DELETE Primary Key checks. There you make sure they target exactly one row by checking that there is an equals-criteria for every PK field.
The new attribute would do the same for the given fields.

Seeing claudiobosticco's reaction here I assume he'd find this useful as well.

Best regards
Blama

[Isomorphic]

You could use dsRequest.getCriteriaValue(fieldName) and create a new top-level "equals" Criterion as previously described.

If the request already contained the required Criterion, the new Criterion might be redundant, but it doesn't make a difference in terms of the final query.

[Isomorphic]

We've made a change to improve the error message according to the post #9. You will see this fixed as of tomorrow's builds April 18.

Regards
Isomorphic Software

[Blama]

Hi Isomorphic,

thank you, but that is not what I meant. In addition, I already could do this descriptively with a criteria-tag in the operation binding.
What I meant is that the user (=the GUI) must provide an equals-criteria (and any value is fine, so I can't have it in my .ds.xml in advance) for a field.

The idea is that this makes servercode easier as you don't need null checks and error handling there, as you know that values will be present once you reach the DMI.

Best regards
Blama

[Isomorphic]

You would basically use a Server Script that just combines the existing inbound criteria with a new Criterion enforcing what you want, in an "and". There's no circumventing that, since the "and" is outside of the original criteria.

[Blama]

Hi Isomorphic,

coming back to your answer from #5 of this not being a security feature: I built this myself in IDACall as "requiredEqualsCriterion", and it is definitely not a one liner.
Could you show how this is done (in an example or here)?
E.g. requiring in an operationBinding that continent is present as top level equals criterion ("top-level" meaning no circumventing it with an "or" or "not")

Thank you & Best regards
Blama

[Blama]

Hi Isomorphic,

using v12.1p_2023-04-07 I can see that the server returns the proper field name in the Developer Console RPC Tab (missingCriterion:"Area (kmē)"), but it seems that the client side does not use this to display a user friendly message. Currently I get a isc.Warn-window with Server returned REQUIRED_CRITERIA_MISSING with no error message performing operation 'fetchByRequiredCriterion'.

Best regards
Blama

[Isomorphic]

Use of field names instead of field titles has been corrected for the next build.

[Blama]

Hi Isomorphic,

We'll go ahead and enable internationalization here - it was already a client-side string constant, so just needed to be added to the language packs.

I see that this was added, and is also working as expected in German. It does still show technical column names though, and not column titles.
Of course minor, but still a good idea IMHO.

Best regards
Blama

[Blama]

Hi Isomorphic,

OK, no problem, I'll do this in my IDACall subclass on my own then.

Best regards
Blama