[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, 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]

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.
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]

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.