jcoglan commented on PR #5792:
URL: https://github.com/apache/couchdb/pull/5792#issuecomment-3715309080
Thank you everyone for the feedback, a summary of my thoughts briefly
discussed on Slack:
- I agree about @ricellis remarks about overloading types in
`validate_doc_update` and `reason`. This is easily avoided using different
names for these fields for declarative VDUs, e.g. reason remains a string, but
detailed mismatch info is in its own field.
- If we take @nickva’s suggestion to not emit detailed field info at all,
then we also don't need to change the evaluation model, or adopt any features
designed to improve feedback like if/then or negation folding. I think it would
be really beneficial to have feedback be better than just "doc was invalid",
but omitting detailed feedback would mean we build a simple thing first, and
then make it fancier as a further iteration if desired. My motivation for
wanting this comes from many years building web apps that have to repeat their
validation logic in every layer; having the DB emit data that could potentially
drive UI code seems nice to me.
- Agree that returning only the first mismatch failure would lead to
horrible ergonomics; I think you either return an opaque "doc invalid" message
or a full list of matches, which echoes the "simple first version" -> "fancier
second version" implementation plan in my previous point.
- Agree with @nickva’s point about semantics differing between VDU and
filters being bad for ergonomics
- "It's not as flexible as Javascript but if users need more flexibility can
just use Javascript, that's not going away." -- Part of my motivation here is
I'd rather not have people be forced to choose between perf and functionality.
I think this will always have some limits that mean you need to use JS
sometimes, but if it's too limited, people just won't use it.
- `$if`/`$then`/`$else`: I agree the names are not ideal, other sets of
names have been suggested in earlier proposals. I do think this functionality
is necessary for good feedback in some cases, regardless of what we name the
operators.
- Using `$data` in normal filters: this could be done, with the caveat that
such queries cannot be indexed.
- `$ref` could definitely be a "phase 2" feature as it's not essential for
this feature to work, it's a convenience, again designed to reduce friction
that would otherwise make users pick JS.
- Ditto `$error` and `$reason`.
- "A simpler way I could see the above example working would be to have two
design docs: one checking for admin and the other for types": this depends on
whether we emit detailed mismatch info or not. If we do, then such feedback
should not be visible to unauthorised users, and you need some way for auth
checks to take precedence over data checks. aAgain, this could be a "phase 2 of
implementation" thing, but if we have detailed feedback this is not optional,
imo.
- "is there any precedent for it though" (re: if/then/else) -- This
functionality exists in JSON Schema and I pretty much copied how it works after
noticing it provides the benefit people were seeking in prior suggestions for
"guard" clauses. I'm not a huge fan of the specific words either, but there is
a precedent
- I'm not informed enough about the quickjs execution model to have an
opinion on running it in-process, how sandboxing works, etc.
- "This leads to more complex selector code that encodes two different
behaviours, one for Mango indexing and one for validation" -- not necessarily;
the existing filter selector workflows could interpret an empty error list as
success, rather than having two distinct eval modes, which I agree would not be
desirable.
This and ensuing discussion leads me to a breakdown of the functionality
into several distinct phases, according to what functionality needs to go
together vs what can be optionally added without effected other things:
- Phase 1: a ddoc can contain a VDU expressed as a mango selector, using
existing mango. A non-matching selector against the `{ newDoc, oldDoc }` struct
would result in a 403 and no detailed feedback.
- Phase 2: detailed feedback. Selector eval produces a list of match
failures instead of a bool, there is some way for auth logic to gain precedence
over data checks, negation is handled nicely. Optionally, `$if`/`$then`/`$else`
(exact names TBD) is added to improve feedback in some cases.
- Phase 3: authorization: `userCtx` and `securityObj` are added to the input
for the selector, and `$data` is added since this is necessary for almost any
meaningful auth logic.
- Phase 4: purely optional features that can be added independently of
anything else: `$ref`, `$cat`, `$error`, `$reason`.
Note that phases 1 and 2 do not change the set of operators that exist in
Mango or their meanings. Phase 2 changes the evaluation model to support
complete mismatch feedback but does not change the "external" semantics of any
operators.
Of the potential additions to the language, I think `$data` is the most
powerful but also adds the most complexity. It requires extra context in the
evaluator, can only combine with certain other operators to avoid injection
bugs, and may be tricky to explain to users as a result. Every other addition
is really a nice-to-have which adds convenience but no significant expressive
power to the language.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
