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

Removing derived type does not update list of directly derived type in the base type #3894

Closed
ESPNSTI opened this issue Nov 25, 2015 · 7 comments
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@ESPNSTI
Copy link

ESPNSTI commented Nov 25, 2015

See #3555

I don't have a code sample yet, but I have some more information.
I reproduced this using EF7 RC1 final, .Net Framework 4.6 in VS2015.

The null reference happens in this section of Microsoft.Data.Entity.Metadata.Internal.InternalModelBuilder.Remove:

            foreach (var directlyDerivedType in entityType.GetDirectlyDerivedTypes().ToList())
            {
                var removed = Entity(directlyDerivedType.Name, ConfigurationSource.Convention)
                    .HasBaseType(entityType.BaseType, configurationSource);
                Debug.Assert(removed != null);
            }

"Entity(directlyDerivedType.Name, ConfigurationSource.Convention)" ends up returning null because "IsIgnored(name, configurationSource)" in the "Entity" method is true.

In the case that I encountered, these are the variable values in this section:

entityType - System.Xml.Linq.XNode
directlyDerivedType.Name - System.Xml.Linq.XDocument
entityType.BaseType - null
configurationSource - DataAnnotation


It appears that while the code has changed, the problem still exists in the Dev branch (as of commit 2627626), in this case, "directlyDerivedType.Builder" returns null.

            foreach (var directlyDerivedType in entityType.GetDirectlyDerivedTypes().ToList())
            {
                var derivedEntityTypeBuilder = directlyDerivedType.Builder
                    .HasBaseType(entityType.BaseType, configurationSource);
                Debug.Assert(derivedEntityTypeBuilder != null);
            }

For a workaround, I ended up adding the following to my code.
I did not need this in Beta8.

            modelBuilder.Ignore<Version>();
            modelBuilder.Ignore<XAttribute>();
            modelBuilder.Ignore<XElement>();
            modelBuilder.Ignore<XName>();
            modelBuilder.Ignore<XNamespace>();
            modelBuilder.Ignore<XNode>();
            modelBuilder.Ignore<XDocument>();

It appears that the types listed above come from entity properties in my model that are ignored using this method:

            modelBuilder.Entity<T>(
                c =>
                {
                    c.Ignore( x => x.Prop);
                } );
@AndriySvyryd
Copy link
Member

@ESPNSTI could you provide a complete repro? A NRE in that piece of code indicates that there's a bug in a different place, but it would be hard to find without a repro.

@ESPNSTI
Copy link
Author

ESPNSTI commented Dec 1, 2015

Sure, here is a test project that will reproduce it:
https://onedrive.live.com/redir?resid=5E5FB20A43816C86!138&authkey=!AMK11ZHwTeqqpGI&ithint=file%2czip

@smitpatel
Copy link
Contributor

Issue here is - while removing entity type, we do not remove it from the BaseType._directlyDerivedTypes. Therefore if the derived type is removed before, then removing base type throws NRE since Builder for derived type is set to null while removing it.

@smitpatel smitpatel changed the title NullReferenceException after the "Add IMutableModel API" changes (Reopened) Removing derived type does not update list of directly derived type in the base type Dec 1, 2015
@smitpatel
Copy link
Contributor

This also throws exception while removing derived type since when foreign keys are removed, relationship discovery convention will run and add new foreign key which disallows removing entity type.

@AndriySvyryd
Copy link
Member

The entity type should be ignored before it is removed, so the convention shouldn't discover any relationship involving it. Perhaps there's an IsIgnored call missing somewhere.

@smitpatel
Copy link
Contributor

There are IsIgnored calls about the target type and navigation but we do not check if the entity type which is running the convention is ignored or not. Should I put check for that? Or after Ignore is called we remove the entity type from _directlyderivedtypes?

@AndriySvyryd
Copy link
Member

Yes, if the current type is ignored no conventions should run on it. We should also call HasBaseType(null) which will remove the type from _directlyderivedtypes before removing anything else.

@smitpatel smitpatel modified the milestones: 7.0.0-rc2, 7.0.0 Dec 2, 2015
@ajcvickers ajcvickers modified the milestones: 1.0.0-rc2, 1.0.0 Oct 15, 2022
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Oct 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Projects
None yet
Development

No branches or pull requests

5 participants