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

Support for convention-based max length in EF Core #25

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

adampaquette
Copy link
Contributor

@adampaquette adampaquette commented Dec 4, 2022

  • The maximum length is now automatically configured with the use of configurationBuilder.Conventions.AddBaseTypeConventions();.
  • I exposed the properties of MaxLengthStringAttribute and MinMaxLengthStringAttribute to be able to grab thoses values from inside the convention builder.
  • The doc has been modified

@ghost ghost force-pushed the max-length-convention-efcore branch from 332e2cd to 59b8103 Compare December 4, 2022 14:39
@adampaquette adampaquette force-pushed the max-length-convention-efcore branch from 59b8103 to 5110be9 Compare December 4, 2022 18:34
@Andreas-Dorfer
Copy link
Owner

Hi @adampaquette,
I’m a bit hesitant about the change. I’ll try to give you my reasoning behind that:

Up until now, it’s optional to use the predefined validations in AD.BaseTypes. That means, a library user is free to create his own MaxLengthStringAttribute and everything will continue to work. One possible reason to create your own validations might be to throw custom exceptions with localized error messages.

The implementation of BaseTypeMaxLengthConventions deviates from that principle. The library user has to use the predefined MaxLengthStringAttribute for the convention to work.

I can think of 3 solutions to that problem:

  1. Accept that AD.BaseTypes.EFCore works differently and requires the user to use the predefined attributes.
  2. Define an interface IMaxLengthStringValidation and change the implementation of BaseTypeMaxLengthConventions to check for the interface.
    I’m not sure where the best place to put IMaxLengthStringValidation would be, but we could put it in an "EFCore" folder in AD.BaseTypes.Core.
  3. Don’t provide the BaseTypeMaxLengthConventions in the first place and leave it up to the library user to set up the MaxLength accordingly.

I'm preferring solution 2, but what do you think?

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

Successfully merging this pull request may close these issues.

2 participants