Initial data should be populated in builders, not in validators

[antmendoza]

What would you like to be added:

I think that the initial/default object state should be builder responsibility.

The actual generated code is something like

export function eventdefValidator(data: Specification.Eventdef): (() => Specification.Eventdef) {
  return () => {
    data.kind =  data.kind || 'consumed';
    const validate = validators.get('Eventdef');
    // TODO: ignore validation if no validator or throw ?
    if (!validate) return data;
    if (!validate(data)) {
      console.warn(validate.errors);
      const firstError: DefinedError = (validate.errors as DefinedError[])[0];
      throw new Error(`Eventdef is invalid: ${firstError.message}`);
    }
    return data;
  };
}

export function eventdefBuilder(): Builder<Specification.Eventdef> {
  return builder<Specification.Eventdef>(eventdefValidator);
}

where the validator responsibility is both to validate and return the default state, in this case data.kind = data.kind || 'consumed';

data.kind: consumed should be set as part of the builder.

Why is this needed:

[antmendoza]

I had a look to "conditional subschemas" https://json-schema.org/understanding-json-schema/reference/conditionals.html, apparently if the evaluated property has no value the condition is evaluated as true.

[JBBianchi]

I had a look to "conditional subschemas" https://json-schema.org/understanding-json-schema/reference/conditionals.html, apparently if the evaluated property has no value the condition is evaluated as true.

Reply for #95 ?

[antmendoza]

yes @JBBianchi 😀

[JBBianchi]

I think the name for the function here is not the best (eventdefValidator). Its purpose is indeed to return the built entity (after validating it). The validator itself is validators.get('Eventdef');. So eventdefValidator here is more "validate and return the inner object of the proxy", it's actually the function specialising the generic builder proxy for the associated type, rather than the validation function. Also, it shouldn't be exported because it's an "inner function", and would never be used outside of that file. (it could even be declared inside eventdefBuilder).

Not sure and still open to discussion but I think the initialisation/defaulting of properties here is the right place. If you have any other suggestion, please feel free to share.

[antmendoza]

I see a lot of value exposing the validator to be used for anyone using the sdk.

related discussion here: #84 (comment)

[JBBianchi]

Yes, that's why I said (and renamed) that in the case demonstrated here, function eventdefValidator is not the proper name because it's not the validator function. But I do agree we could wrap up validators and expose them in a more friendly way than validators.get('Eventdef').

[antmendoza]

closed with this PR #108