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

Relationships of AppUser causes "The property 'AppUser.ExtraProperties' could not be mapped." #1414

Closed
gdlcf88 opened this issue Jul 1, 2019 · 10 comments

Comments

@gdlcf88
Copy link
Contributor

gdlcf88 commented Jul 1, 2019

AppUser:

    public class AppUser : FullAuditedAggregateRoot<Guid>, IUser
    {
        public virtual Guid? OrganizationId { get; protected set; }
        public virtual Organization Organization { get; set; }
    }

Organization:

    public class Organization : FullAuditedAggregateRoot<Guid>, IMultiTenant
    {
        [Required]
        public virtual Guid? TenantId { get; protected set; }
        public virtual AppTenant Tenant { get; set; }
        public virtual List<AppUser> Users { get; set; }
        public Organization()
        {
            Users = new List<AppUser>();
        }
    }

Run "Add-Migration" and it responses:
The property 'AppUser.ExtraProperties' could not be mapped, because it is of type 'Dictionary<string, object>' which is not a supported primitive type or a valid entity type. Either explicitly map this property, or ignore it using the '[NotMapped]' attribute or by using 'EntityTypeBuilder.Ignore' in 'OnModelCreating'.

I tried:

    public class AppUser : FullAuditedAggregateRoot<Guid>, IUser
    {
        [NotMapped]
        public override Dictionary<string, object> ExtraProperties { get; protected set; }

        public virtual Guid? OrganizationId { get; protected set; }
        public virtual Organization Organization { get; set; }
    }

And it works, but built a Table names "AppUser". (Not "AbpUsers")

DbContext:
image
MigrationsDbContext.cs:
image

Don't know what to do :(

@maliming
Copy link
Member

maliming commented Jul 1, 2019

see: #927
Please try searching before asking questions.

@maliming maliming closed this as completed Jul 1, 2019
@gdlcf88
Copy link
Contributor Author

gdlcf88 commented Jul 1, 2019

see: #927
Please try searching before asking questions.

As you can see, I added "b.ConfigureExtraProperties()", but it only works without entities relationships.
And this problem is only happens on "AppUser".

The following code is available:

    public class Order : FullAuditedAggregateRoot<Guid>
    {
        [Required]
        public virtual Guid FactoryId { get; protected set; }
        public virtual Factory Factory { get; set; }
    }
    public class Factory : FullAuditedAggregateRoot<Guid>, IMultiTenant
    {
        [Required]
        public virtual Guid OrderId { get; protected set; }
        public virtual Order Order { get; set; }
    }

To solve this problem, I even tried this:
In MigrationsDbContext.cs I use this code:

            builder.Entity<AppUser>(b =>
            {
                b.ToTable("AbpUsers"); //Sharing the same table "AbpUsers" with the IdentityUser
                //b.ConfigureFullAuditedAggregateRoot();
                b.ConfigureFullAudited();
                b.ConfigureExtraProperties();
                b.ConfigureConcurrencyStamp();
                b.ConfigureAbpUser();

                b.ConfigureCustomUserProperties();
            });

to replace:

            builder.Entity<IdentityUser>(b =>
            {
                b.ConfigureCustomUserProperties();
            });

or keep both.
But it respones:
Cannot use table 'AbpUsers' for entity type 'IdentityUser' since it is being used for entity type 'AppUser' and there is no relationship between their primary keys.

Maybe builder.Entity<AppUser>() in DbContext.cs doesn't work or something else.

Thank you for your help, but I still don't know how to solve it.

@gdlcf88
Copy link
Contributor Author

gdlcf88 commented Jul 1, 2019

@hikalkan @maliming I've found a way.
In MyProjectMigrationsDbContext.cs, use this code:

            builder.Entity<AppUser>(b =>
            {
                b.ToTable("AbpUsers");
                b.ConfigureAbpUser();
                b.ConfigureFullAuditedAggregateRoot();
                b.HasOne<IdentityUser>().WithOne().HasForeignKey<AppUser>(e => e.Id);

                b.ConfigureCustomUserProperties();
            });

to replace:

            builder.Entity<IdentityUser>(b =>
            {
                b.ConfigureCustomUserProperties();
            });

It solves the problem.
Will it cause other problems? If not, maybe the template need it.

And by the way, setting properties in ConfigureCustomUserProperties() is no longer necessary. #1336 (comment)

@hikalkan
Copy link
Member

hikalkan commented Jul 5, 2019

Hi @gdlcf88,

We will document various ways of extending entities of a reusable module (like Identity here). But I will provide a solution for your case:

Do not add navigation properties to other aggregate roots. For your case, Don't add navigation property to User from Organization. This is an important rule for DDD. ABP doesn't force every DDD rule, however, for this reusable modules, this is the way to create dbcontext per model and bring all modules together as a single db migration.

So, change your entities like:

AppUser:

    public class AppUser : FullAuditedAggregateRoot<Guid>, IUser
    {
        public virtual Guid? OrganizationId { get; protected set; }
    }

Organization:

    public class Organization : FullAuditedAggregateRoot<Guid>, IMultiTenant
    {
        [Required]
        public virtual Guid? TenantId { get; protected set; }
    }

Change your ConfigureCustomUserProperties method like this:

public static void ConfigureCustomUserProperties<TUser>(this EntityTypeBuilder<TUser> b)
    where TUser: class, IUser
{
    b.Property<Guid?>(nameof(AppUser.OrganizationId));
    b.HasOne<Organization>().WithMany().HasForeignKey(nameof(AppUser.OrganizationId));
}

Add this to ConfigurePhoneBook method:

builder.Entity<Organization>(b =>
{
    b.ToTable(PhoneBookConsts.DbTablePrefix + "Organizations", PhoneBookConsts.DbSchema);
    b.ConfigureFullAuditedAggregateRoot();
});

The resulting migration for me is:

protected override void Up(MigrationBuilder migrationBuilder)
{
    migrationBuilder.AddColumn<Guid>(
        name: "OrganizationId",
        table: "AbpUsers",
        nullable: true);

    migrationBuilder.CreateTable(
        name: "AppOrganizations",
        columns: table => new
        {
            Id = table.Column<Guid>(nullable: false),
            ExtraProperties = table.Column<string>(nullable: true),
            ConcurrencyStamp = table.Column<string>(nullable: true),
            CreationTime = table.Column<DateTime>(nullable: false),
            CreatorId = table.Column<Guid>(nullable: true),
            LastModificationTime = table.Column<DateTime>(nullable: true),
            LastModifierId = table.Column<Guid>(nullable: true),
            IsDeleted = table.Column<bool>(nullable: false, defaultValue: false),
            DeleterId = table.Column<Guid>(nullable: true),
            DeletionTime = table.Column<DateTime>(nullable: true),
            TenantId = table.Column<Guid>(nullable: false)
        },
        constraints: table =>
        {
            table.PrimaryKey("PK_AppOrganizations", x => x.Id);
        });

    migrationBuilder.CreateIndex(
        name: "IX_AbpUsers_OrganizationId",
        table: "AbpUsers",
        column: "OrganizationId");

    migrationBuilder.AddForeignKey(
        name: "FK_AbpUsers_AppOrganizations_OrganizationId",
        table: "AbpUsers",
        column: "OrganizationId",
        principalTable: "AppOrganizations",
        principalColumn: "Id",
        onDelete: ReferentialAction.Restrict);
}

Creating a separated dbcontext per module, but managing a single unified db migration path is a big problem with EF core and we provide a model to solve it.

@gdlcf88
Copy link
Contributor Author

gdlcf88 commented Jul 5, 2019

@hikalkan Learned a lot from you.

@abllyboy
Copy link

Why this code does'nt work in micoService's AuthorHost?But when I add the similar code in the project generate by CLI,it works fine.
2
1
3

@maliming
Copy link
Member

@abllyboy
Please create a new issue and explain your problem in detail. Screenshot is hard to see the problem, Thanks.

@SamQuest
Copy link

b.HasOne().WithOne().HasForeignKey(e => e.Id);

@hikalkan What is this witchcraft. can you explain this.

besides the above code helped me to do this DomainUser.AppUser and configure it as
b.HasOne(x => x.AppUser);

DDD is broken at this point, but this was the only way I could consume the AppUser without making it FAT (When adding columns to AppUser in each modules is confusing for any 'rookie' who will later takeover the code, and the whole point of DDD is making life easy, ABP is all about making things simpler)

Are there any serious issues if I keep FKey apart from Identity not being able to delete the IdentityUser.

@SamQuest
Copy link

SamQuest commented Jan 22, 2020

b.HasOne().WithOne().HasForeignKey(e => e.Id);

So is it OwnedEntity, would it make sense if shared properties like DateOfBirth can be set using ConfigureCustomUserProperties (because shared columns will be registered in one place probably the main module) and other module specific properties can use separate table using the FKey

I am on my first few days of ABP Framework and it took a while to wrap my head around the ConfigureCustomUserProperties

@jtoniolo
Copy link

Why is it so hard to understand that we (the consumers of ABP) need to own the domain for the application user. And we need to extend that domain using simple Entities.

Common use cases:

  • User specific settings
  • User relationships
  • Additional user profile info
  • I'm sure others exists

These extra entities easily fit into the whole idea of a Domain. These headings may not, in all circumstances deserve their own domain.

FFS guys! Quit being soup (or, rather, DDD) nazis!

How about this, why don't you just stop trying to ram navigation properties into your Extra Properties collection?

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

6 participants