Document ConstraintViolation.field_path field

[yevhenii-nadtochii]

This PR updates docs to ConstraintViolation.field_path field.

Initially, we wanted to alter this field somehow because it stands out a bit in ConstraintViolation, but we didn't come up with a much better solution.

We have considered the following alternatives:

1. Use a specific NestedConstraintViolation message for (validate) errors.
2. Use of one_of: field_path or field_name.
3. Having two fields simultaneously parent_path and field_name.

Conceptually, all three options do the same: introduce a kind of "split/branching". We decided to avoid this split.

I will also leave my latest thoughts on this.

---

The other side of validation API (options.proto) still operates with ${field.name} placeholders. Many default error messages begin with The ${parent.type}.${field.name} field must .... Probably, they should also be updated to use field.path. Or we are explicitly OK with this inconsistency. When we apply an option, we say about the field name, but when the option throws, it is talking about the field path.

---

To me, it looks like that the desire of (validate) to pass field.path and lack of our (at least, mine) desire to modify ConstraintViolation to please the necessity of a specific option already met somehow in the recent changes to options.proto.

Now, options declare a set of placeholders that a user can use in error messages. We didn't extend ConstraintViolation to allow (distinct) option pass field.duplicates to the thrown error. I suggest treating field.path as a part of option-specific metadata, which makes sense only within this option.

Otherwise, it remains either a bit inconsistent (not much really, but it is as for now) or all options become nesting-aware, which is suggested by the first paragraph.

---

As a possible extension of an idea from the paragraph above, placeholder values may play the role of option-specific metadata passed to the violation error. We are going to pass all placeholder values, no matter whether they are used in the template. So, conceptually, it seems to me more than just template values.

The basic values of placeholders are the same with the values of ConstraintViolation fields. For example, see how (set_once) tests placeholders and other fields of ConstraintViolation:

        message.withPlaceholders shouldBe template(field.index + 1)
        message.placeholderValueMap shouldContainExactly mapOf(
            FIELD_NAME to fieldName,
            FIELD_TYPE to fieldType,
            FIELD_VALUE to "$value1",
            FIELD_PROPOSED_VALUE to "$value2",
            PARENT_TYPE to parentType
        ).mapKeys { it.key.toString() }

        fieldPath shouldBe FieldPath(fieldName)
        typeName shouldBe parentType

        // Enums are a bit special. See the method docs for details.
        if (value2 is EnumValueDescriptor) {
            // Any enum in `(set_once)` tests is `YearOfStudy`, so it is safe.
            val enumConstant = YearOfStudy.forNumber(value2.number)
            fieldValue shouldBe toAny(enumConstant)
        } else {
            fieldValue shouldBe toAny(value2)
        }

field.name, parent.type and field.value are actually checked twice because they are passed twice. The only difference is that ConstraintViolation.field_value is Any, but in placeholders, everything is String.

Whether to keep placeholders within the TemplateString or move up to ConstraintViolation – not clear, I have not given a deep thought for it. The main suggestion is to treat them more like option-specific metadata (key-value pairs), which can be used as placeholders just by the way, not because this is their only aim of existence.

Indeed, through these placeholders, options pass important information for ConstraintViolation in general, not only for the template string. I would suggest that in ViolationText (which prepares an exception message), it'd be helpful to always print all passed placeholders, no matter whether the template uses them or not. The library can add a new, useful placeholder, and a user may forget to update the template.... When they are more like metadata, they are always printed along with the formatted message.

---

(validate) provides its own placeholders and its own error message. We can pass field.path as placeholder/metadata value. And, for example, nested.error with the template from the original error. All placeholder values from nested will be copied as well.

Though, we have a split, in which some options are hard messages, and some use a pair of boolean option + auxilary hard message to pass the template. To ease things and have consistent API, I suggest migrating them all to hard messages.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 33.55%. Comparing base (e47025c) to head (0b5124a).
Report is 23 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##             master     #174   +/-   ##
=========================================
  Coverage     33.55%   33.55%           
  Complexity      349      349           
=========================================
  Files           142      142           
  Lines          2834     2834           
  Branches        234      234           
=========================================
  Hits            951      951           
  Misses         1814     1814           
  Partials         69       69           

[yevhenii-nadtochii]

@armiol @alexander-yevsyukov PTAL

[armiol]

@yevhenii-nadtochii please see my comments.

[armiol]

Please rephrase this sentence. There are a couple of problems here:

• "... this violation" — this is not a violation, this is a validation error;
• let's describe this case: "violation is nested in another violation" — so that it is clear to readers not familiar with our internals; right now there is a gap between the fact "validation failed" and the phrase you've used.

[armiol]

I would say "root" instead of "parental".

[armiol]

This is here again. It's not easy to grasp what "nesting" means.

In this particular scenario will there be two items in repeated ConstraintViolation violation = 5;, or just one? If there are two (like written here), it makes it even more difficult to understand "nesting" as a term.

[armiol]

One more question here is why we cannot go with just this field. There may be a FieldPath with a single "path element" in it.

[alexander-yevsyukov]

LGTM, provided comments from @armiol are addressed.

[yevhenii-nadtochii]

@armiol @alexander-yevsyukov PTAL

As discussed, the field has been restored and is better documented now.

Also, I have left my latest thoughts on this in PR description.

[alexander-yevsyukov]

LGTM