Custom DataSource validation flow when populating fields in executeAdd/Update

[claudiobosticco]

Hi, in an implementation of a custom DataSource that extends SQLDataSource, I have some fields that are populated by the executeAdd / executeUpdate methods.

Now I need to execute the validation defined on those DataSource field, and I see that this can be done using this.validate() or dsRequest.validate().

I wanted to ask whether there is a preferable best practice.
For example, for efficiency reasons, it seems to me that it is possible to use dsRequest.setValidated(true) in the execute override, and then call validate() (I am not sure whether it is preferable to call it on the dsRequest or on the DataSource) after adding the values, and then set the ErrorReport if present.

[Isomorphic]

Typically, whatever data you're adding would be already validated, since it's not coming from a user: you, as a developer, can ensure that it's right. The other problem with validating data you added is that, if it fails, it's going to be mysterious to the end user ("I didn't set that!").

If you want the data you're adding validated anyway (perhaps that's just the simplest for this use case), the most efficient thing is to turn off the automatic validation, then manually validate() yourself, so that you don't do two validation runs.

This really only makes a difference if you have expensive validators, such as isUnique(), that actually perform other DataSource operations as part of their work.

[claudiobosticco]

If you want the data you're adding validated anyway (perhaps that's just the simplest for this use case), the most efficient thing is to turn off the automatic validation, then manually validate() yourself, so that you don't do two validation runs.

and it's correct to:
- call dsRequest.setValidated(true) in the SQLDataSource.execute override
- and then call dsRequest.validate() in the SQLDataSource.executeUpdate/Add override?

[Isomorphic]

It's neither correct nor incorrect, it just depends on your usage. Framework features and your application code that run in the window between window between execute and executeUpdate typically would assume a validated DSRequest, and may blow up if the request contains invalid data, which is further dependent on what kinds of requests your frontend sends.

An example of a framework feature would be field.multipleStorage.

[claudiobosticco]

Hi, thanks for the suggestions.

Assuming I leave the default validation behavior unchanged and therefore want to explicitly re-run validation after populating the additional field values (which, for now, would only be two fields with simple validators and no dependencies on other fields), if I were to use DataSource.validate(newValues) instead of DSRequest.validate(), could I expect better efficiency?

What other differences should I expect between the two methods?

Also, if there were validator dependencies involving the newly populated values, how would DataSource.validate(newValues) behave in that case?

[Isomorphic]

Those are rather different methods. DataSource.validate() doesn't have the dsRequest at all, so things like isUnique can't work (no operationType). It basically works for simple values only.

So we have to come back to the point we made before: this is your (trusted) server-side code adding these values. Why are you validating them at all?

The main scenarios where validation could plausibly be useful involve complex cross-checking with other stored records, but those are precisely the scenarios where DataSource.validate() doesn't really make sense.

So basically, the most efficient approach is: make sure the values are already valid when you add them.

[claudiobosticco]

I was about to explain that my use case is an image upload: on the server side I compute and store the image dimensions, and I wanted to take advantage of defining integerRange validators in order to potentially return a validation error.

However, I had not realized that it is sufficient to add those values in the execute override itself, rather than in executeAdd / executeUpdate.

Thanks a lot for the heads-up!