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

Provide a data annotation for setting backing fields #6266

Closed
ajcvickers opened this issue Aug 8, 2016 · 6 comments · Fixed by #19989
Closed

Provide a data annotation for setting backing fields #6266

ajcvickers opened this issue Aug 8, 2016 · 6 comments · Fixed by #19989
Assignees
Labels
area-model-building closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. good first issue This issue should be relatively straightforward to fix. type-enhancement
Milestone

Comments

@ajcvickers
Copy link
Member

This would allow a property to be annotated with a backing field directly in the class. One advantage of this is that the nameof() syntax could be used to reference the field.

@divega divega added this to the Backlog milestone Aug 8, 2016
ajcvickers added a commit that referenced this issue Aug 8, 2016
Issues #4357, #4855

This change adds fluent and lower-level APIs to set backing fields or properties with no fields as well as conventions for using the fields and a mechanism to provide control over when the property is used and when the field is used.

Also see #6266, #6267, and #6268.
ajcvickers added a commit that referenced this issue Aug 8, 2016
Issues #4357, #4855

This change adds fluent and lower-level APIs to set backing fields or properties with no fields as well as conventions for using the fields and a mechanism to provide control over when the property is used and when the field is used.

Also see #6266, #6267, and #6268.
ajcvickers added a commit that referenced this issue Aug 8, 2016
Issues #4357, #4855

This change adds fluent and lower-level APIs to set backing fields or properties with no fields as well as conventions for using the fields and a mechanism to provide control over when the property is used and when the field is used.

Also see #6266, #6267, and #6268.
ajcvickers added a commit that referenced this issue Aug 10, 2016
Issues #4357, #4855

This change adds fluent and lower-level APIs to set backing fields or properties with no fields as well as conventions for using the fields and a mechanism to provide control over when the property is used and when the field is used.

Also see #6266, #6267, and #6268.
@ajcvickers ajcvickers added consider-for-current-release size~1-day help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. labels May 14, 2018
@ajcvickers ajcvickers self-assigned this May 17, 2018
@divega divega added good first issue This issue should be relatively straightforward to fix. help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. and removed help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. good first issue This issue should be relatively straightforward to fix. labels May 31, 2019
@ajcvickers ajcvickers added good first issue This issue should be relatively straightforward to fix. and removed help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. labels Aug 5, 2019
@lajones lajones self-assigned this Feb 14, 2020
@lajones
Copy link
Contributor

lajones commented Feb 18, 2020

@ajcvickers Let me know what you think of this design for the new attribute (I was thinking of putting it in EFCore.Abstractions next to OwnedAttribute:

namespace Microsoft.EntityFrameworkCore
{
    /// <summary>
    ///     Names the backing field associated with this property.
    /// </summary>
    [AttributeUsage(AttributeTargets.Property)]
    public sealed class BackingFieldAttribute : Attribute
    {
        /// <summary>
        ///     Initializes a new instance of the <see cref="BackingFieldAttribute" /> class.
        /// </summary>
        /// <param name="name">The name of the backing field.</param>
        public BackingFieldAttribute([NotNull] string name)
        {
            Check.NotEmpty(name, nameof(name));

            Name = name;
        }

        /// <summary>
        ///     The name of the backing field.
        /// </summary>
        public string Name { get; }
    }
}

@ajcvickers
Copy link
Member Author

@dotnet/efcore Do people think this should be its own annotation? Or should we have one attribute for property mapping and facets? Or one attribute for property mapping, one for facets?

@smitpatel
Copy link
Member

What is property mapping? And which facets are we planning to add annotations for?

@ajcvickers
Copy link
Member Author

Maybe we should make a list of all the things that can be configured for with the fluent API for which there are no attributes?

@lajones
Copy link
Contributor

lajones commented Feb 18, 2020

Not sure this is exhaustive, but a quick look at the docs shows the following Fluent API with no attribute equivalent (note: some of these don't apply to properties and so may not be relevant here):

  • configuring composite keys
  • name of PK/FK constraints
  • HasPrincipalKey()
  • HasAlternateKey()
  • cascade delete
  • HasIndex() (and its name and included properties)
  • HasDiscriminator()
  • HasSequence()
  • HasField() and UsePropertyAccessMode()
  • value conversions
  • data seeding
  • HasNoKey()

@lajones
Copy link
Contributor

lajones commented Feb 19, 2020

Notes from team discussion 2/19/20:
In general we prefer small individual attributes over attributes which represent a combination of facets, e.g. we'd prefer Unicode and Precision as separate attributes, but we may make exceptions where concepts are closely tied together, e.g. Precision and Scale might well end up together.

For this particular issue we decided (at least for now) to use one attribute for BackingField which embodies just the name of the backing field and decided not to include the concept of PropertyAccessMode on the same one as that could also be applied to classes.

@lajones lajones added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Feb 20, 2020
@lajones lajones modified the milestones: Backlog, 5.0.0 Feb 20, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0, 5.0.0-preview2 Mar 13, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0-preview2, 5.0.0 Nov 7, 2020
@ajcvickers ajcvickers removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-model-building closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. good first issue This issue should be relatively straightforward to fix. type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants