Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conventions: Add public model building API #2589

Closed
AndriySvyryd opened this issue Jul 10, 2015 · 5 comments
Closed

Conventions: Add public model building API #2589

AndriySvyryd opened this issue Jul 10, 2015 · 5 comments

Comments

@AndriySvyryd
Copy link
Member

The InternalModelBuilder has several problems:

  • It is a leaky abstraction, as it exposes ConfigurationSource and requires the users to keep track of the last returned builder that contains the up-to-date configuration.
  • Its shape resembles ModelBuilder, but the behavior is different, sometimes in non-obvious ways
  • The API is inconsistent in parameters as it can take string ids, Model objects or InternalModelBuilder objects.
@AndriySvyryd
Copy link
Member Author

One approach that could solve these issues is the following:

  1. Make InternalModelBuilder the only source-of-truth while the model is being built.
  2. Make all users go through one of the interfaces that the InternalModelBuilder would implement:
  • `IMutableModel`, would have mostly the same shape as `Model` currently has, but would also have overloads that use names for element identification (e.g. `IMutableKey AddKey(IEnumerable)`). Can be dropped down to from `ModelBuilder`.
  • `IConventionModel` would have a similar shape to IMutableModel, while conveying that configuration is not assured (e.g. `IConventionKey TryAddKey(IEnumerable, ConfigurationSource)`, `TrySetRequired(bool, ConfigurationSource)`).
  • These interfaces would derive from `IModel` and `IAccessor`
  • `ModelBuilder`: conflicting configuration - last one wins, invalid configuration - throws
  • `IMutableModel`: conflicting configuration - throws, invalid configuration - throws
  • `IConventionModel`: conflicting configuration - last one wins, invalid configuration or conflict with explicit configuration - returns `null`
3. Add 3 more values to `ConfigurationSource`: `WhatIfExplicit`, `WhatIfDataAnnotation`, `WhatIfConvention`. When used they would not perform any changes, but would return the same success result as the equivalent 'real' sources. 4. When the model is built and the `ModelValidator` ran successfully it can be converted to `ReadOnlyModel`, that only implements IModel and is optimized for reading speed and low memory consumption.

@AndriySvyryd AndriySvyryd changed the title Conventions: Add public model building API Conventions: Add public model building API and improve implementation Jul 10, 2015
@rowanmiller rowanmiller added this to the 7.0.0 milestone Jul 10, 2015
@AndriySvyryd
Copy link
Member Author

Marking for retriage.
I think this should be pri0 since it would introduce a big API surface and early release can generate helpful feedback.
Also it will make other changes and fixes in the builders easier as there would be less code duplication.

@AndriySvyryd AndriySvyryd removed this from the 7.0.0 milestone Aug 18, 2015
@rowanmiller
Copy link
Contributor

@AndriySvyryd can you bring a proposal to the design meeting... or a corridor meeting in our corridor-less open space

@rowanmiller rowanmiller added pri0 and removed pri1 labels Aug 28, 2015
@rowanmiller rowanmiller added this to the 7.0.0 milestone Aug 28, 2015
@rowanmiller rowanmiller changed the title Conventions: Add public model building API and improve implementation Conventions: Add public model building API and improve implementation (proposal to design meeting) Aug 28, 2015
@AndriySvyryd AndriySvyryd modified the milestones: 7.0.0-rc1, 7.0.0 Oct 6, 2015
@rowanmiller rowanmiller changed the title Conventions: Add public model building API and improve implementation (proposal to design meeting) Perf: Conventions: Add public model building API and improve implementation (proposal to design meeting) Oct 13, 2015
@AndriySvyryd AndriySvyryd changed the title Perf: Conventions: Add public model building API and improve implementation (proposal to design meeting) Conventions: Add public model building API and improve implementation Oct 19, 2015
@AndriySvyryd AndriySvyryd removed this from the 7.0.0-rc1 milestone Oct 19, 2015
@AndriySvyryd
Copy link
Member Author

We've discussed this and decided to keep the model and the model building logic separate. To improve perf ConfigurationSource will be stored as part of the model, see #3476

This issue is now only for adding public model building API for conventions.

@AndriySvyryd AndriySvyryd changed the title Conventions: Add public model building API and improve implementation Conventions: Add public model building API Oct 19, 2015
@rowanmiller
Copy link
Contributor

Custom conventions is tracked by #214, so closing this out.

@AndriySvyryd AndriySvyryd removed their assignment Mar 8, 2016
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants