search

sul-dlss / cocina-models

Cocina repository data model (implemented in Ruby)
https://sul-dlss.github.io/cocina-models/
3 stars 0 forks source link

Allow generators to create and use custom types and union types #568

Closed mjgiarlo closed 1 year ago

mjgiarlo commented 1 year ago

Why was this change made? 🤔

These commits change the Ruby models so that they are working more by reference and less by value, e.g., instead of having the druid regex in every model that has an externalIdentifier, point every externalIdentifier at the Druid model and let the model handle the validation.

Currently the models handle validation through a combination of validating against the OpenAPI spec and using the generated Ruby (dry-) types, and this commit adds a bit more smarts and descriptive oomph to the Ruby side.

A separate commit moves yet more smarts into the Ruby (dry-) types, allowing them to make use of union types defined in the OpenAPI spec, such as for barcodes and DOIs. This sets up a follow-on commit around Symphony and Folio catalog links which will similarly use union types.

How was this change tested? 🤨

CI & tested against QA, stage, and 100K prod records on sdr-infra

mjgiarlo commented 1 year ago

@justinlittman 💬

  1. Have you verified that the openapi validator correctly handles union types?

Here's what I've done so far:

  1. Validated the OpenAPI specification (via CI 👆🏻)
  2. Run the specs successfully (via CI 👆🏻)
  3. Ran bin/validate-cocina against this branch on sdr-infra

Is there another way to test in the way you're asking about? If so, happy to do it.

  1. Instead of adding more validation to the Ruby, shouldn't we just remove it all? It is redundant of the openapi validation (at best) and probably just slows everything down. (I admit that this question is a bit out of scope of this PR).

Agree that it's out of scope and that we should probably kick this around at some point.

and a request: at least the most significant generator changes (e.g., union type) should have specs.

For sure. I'll admit that I opened the generator specs, didn't grok how they were testing what they purported to be testing, and shrugged. I'll give it a deeper look today.