odata.net icon indicating copy to clipboard operation
odata.net copied to clipboard

Re-design model validation APIs

Open corranrogue9 opened this issue 3 years ago • 6 comments

Models can currently be used without validation, meaning that they produce errors when the model is used rather than when the model is constructed or loaded. We should not allow models to be invalid.

corranrogue9 avatar May 03 '22 17:05 corranrogue9

  • make sure to accommodate warnings as well as failures
  • revalidating models frequently on clients
  • ags formerly had some perf issue (probably around caching)
  • being compatible with current clients having "invalid" models

corranrogue9 avatar Aug 10 '22 16:08 corranrogue9

  • clients get the model from the service, perhaps those uses of the model specifically can be without validation

corranrogue9 avatar Aug 10 '22 16:08 corranrogue9

  • preconditions for the library to function should probably be validated on both the client and the service

corranrogue9 avatar Aug 10 '22 16:08 corranrogue9

Clement has previously done an investigation on client edm model loading, follow up with him about this investigation.

corranrogue9 avatar Aug 10 '22 16:08 corranrogue9

Related issue + investigation: https://github.com/OData/odata.net/issues/2321

corranrogue9 avatar Aug 10 '22 16:08 corranrogue9

I am adding context from one of our investigations meetings to each of the EDM related work items. Please keep these scenarios in mind when investigating this work item:

It has been suggested (by myself) that removing the EDM interfaces and replacing them with concrete types where appropriate would be a good idea. 2 existing scenarios were enumerated for why the interfaces are necessary, 1 existing scenario was enumerated for why the interfaces are helpful (but not strictly necessary), 1 known future scenario was enumerated for why the interaces continue to be necessary, and 1 potential future scenario was enumerate for why the interfaces provide existensibility that wouldn't be available with concrete types.

Existing necessary scenarios

  1. The interfaces are used today for mocking in unit tests
  2. The interfaces could be used by customers to provide custom implementations for each type

Existing helpful scenarios

  1. It is helpful to have a read-only view into the EDM models

Known future scenario

  1. We want to allow customers to update their models after, for example, the model has been read from a CSDL file. The interfaces being a read-only view give the option for the concrete types to be used for post-read modifications, with the interfaces being used once those modifications are complete.

Potential future scenario

  1. There is a potential scenario where the full model being loaded into memory is not necessary for a customer. With the current structure of the IEdmModel interface, this means that, if all other EDM types were concrete types, then any instance of IEdmModel would necessarily need to load portions of the model into memory even if those portions aren't used. A new implementation of non-model EDM interfaces would allow for this even though the concrete types would not.

corranrogue9 avatar Aug 17 '22 21:08 corranrogue9