12.1d: New "requiredCriterion" sample feedback

[Blama]

Hi Isomorphic,

after your blog entry I tested the new requiredCriterion sample.

I think this is another good feature w.r.t to security, as I assume one could enforce "1-row fetches allowed only" with

<field name="id" primaryKey="true">
    <validOperators>
        <operator>equals</operator>
    </validOperators>
<field>
...
<operationBindings>
    <operationBinding operationType="fetch" operationId="fetchByRequiredCriterion" requiredCriterion="id" />
</operationBindings>

which would translate to a allowMultiFetch="false" (analogue to allowMultiUpdate), if such an attribute existed. Is this correct?


As an improvement suggestion:
Right now (SNAPSHOT_v12.1d_2019-02-03) you return "Operation requires criteria for the following field(s): [area, population]" which is not localized, see e.g. here in German.
As this is supposed to be a GUI feature, too, and not only a security feature (my assumption), it would be good if the message would be localized and if you returned the field title instead of the field name. name -> title is clear here, but this might not always be the case.
If it is not meant to be a GUI feature, but only a security one, this does not apply. But then a shorthand for the primaryKey-field like the suggested allowMultiFetch="false" would be nice as well.

Best regards
Blama

[Blama]

Hi Isomorphic,

actually this can't be used as security feature right now, as it only requires some condition to be set. See this modified sample.
Operator changed to "or", notNull-criterion on a required="true" field -> fetch will always include all rows.

In order to be usable as security feature, requiredCriterion must enforce setting the criteria as top-level "and"-criteria.

isc.VStack.create({
    membersMargin: 10,
    width:600, 
    members: [
        isc.FilterBuilder.create({
            ID:"countryFilter",
            dataSource:"worldDS",
            criteria: { _constructor: "AdvancedCriteria",
[B]operator: "or"[/B], criteria: [
                    {fieldName: "area", operator: "greaterThan", value: 50},
                    {fieldName: "population", operator: "greaterThan", value: 100000},
[B]{fieldName: "code", operator: "notNull"}[/B]
                ]
            }
        }),
        isc.IButton.create({
            ID:"filterButton",
            title:"Filter",
            click : function () {
                countryList.setData([]);
                countryList.filterData(countryFilter.getCriteria());
            }
        }),
        isc.ListGrid.create({
            ID: "countryList",
            height:224, alternateRecordStyles:true, 
            dataSource: worldDS,
            fetchOperation: "fetchByRequiredCriterion",
            fields:[
                {name:"countryName"},
                {name:"continent"},
                {name:"population"},
                {name:"area"},
                {name:"gdp"},
                {name:"independence", width:100}
            ]
        })
    ]
});

Best regards
Blama

[Isomorphic]

Yes, this isn't really a security feature, more intended as an extra layer of protection against unintentionally broad queries or updates. However, like security features, the intent is to make the same check in your UI code, so that you are not sending requests to the server unnecessarily, hence the message is not internationalized - do that in your UI.

[Blama]

Hi Isomorphic,

OK, but i18n'ing this message would make sense w.r.t. to your intended effect and "just throwing FilterBuilder at some DataSource" for all users. As I'm not using FilterBuilder, this does not affect me, but I do think this might make sense as it is most likely an easy change.

W.r.t. to allowMultiFetch="false" (=fetch requires an top level equals criteria on the primaryKey field(s)): Can you log this as an enhancement, like you did for DataSource.serverOnly here?
It should be easy and I'll most likely implement such a thing myself in my IDACall subclass, but I do think this does also make sense for all developers.
Other than allowMultiUpdate it should default to true.

Best regards
Blama

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

As far as your suggestion, this strikes us as one of those shortcuts that is barely more compact than other approaches (eg Server Script) and infrequently needed, hence no one would ever notice it existed, so not worth adding. Sorry.

[Blama]

Hi Isomorphic,

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

Best regards
Blama

[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

[Isomorphic]

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

[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

[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

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

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]

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

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

[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