EntityFramework-Reverse-POCO-Code-First-Generator icon indicating copy to clipboard operation
EntityFramework-Reverse-POCO-Code-First-Generator copied to clipboard

Multiple column foreign key is not generated

Open hamuryen opened this issue 5 years ago • 4 comments

Hi There,

I used the POCO lovingly with v2.35.0. Now I had some problems when i want to use a new version(3.1.3) of POCO for .NET Core. I will explain the problem with a simple example. In my case, i have 2 table one of those name is Table1and other one is Table2. Below you can find SQL scripts to create tables.

CREATE TABLE dbo.Table2
(
    id  BIGINT IDENTITY(1, 1) NOT NULL,
    num BIGINT NOT NULL,
    CONSTRAINT PK_Table2 PRIMARY KEY CLUSTERED (id ASC),
    CONSTRAINT U_Table1 UNIQUE NONCLUSTERED (id ASC, num ASC)
);

CREATE TABLE dbo.Table1
(
    id   BIGINT IDENTITY(1, 1) NOT NULL,
    id_t BIGINT NOT NULL,
    num  BIGINT NOT NULL,
    CONSTRAINT PK_Table1 PRIMARY KEY CLUSTERED (id ASC)
);

ALTER TABLE dbo.Table1 WITH CHECK
ADD CONSTRAINT FK_Table1_Table2 FOREIGN KEY (id_t, num) REFERENCES dbo.Table2 (id, num);

ALTER TABLE dbo.Table1 CHECK CONSTRAINT FK_Table1_Table2;

So Table2 and Table1 are connected over 2 columns with foreign key(ALTER TABLE [dbo].[Table1] WITH CHECK ADD CONSTRAINT [FK_Table1_Table2] FOREIGN KEY([id_t], [num]) REFERENCES [dbo].[Table2] ([id], [num])).

Now, when i trying to use latest version of POCO for generation of entities, entity classes are generated by POCO as follows.

Table1:

public class Table1
{
	public long Id { get; set; } // id (Primary key)
	public long IdT { get; set; } // id_t
	public long Num { get; set; } // num
}

Table2:

public class Table2
{
	public long Id { get; set; } // id (Primary key)
	public long Num { get; set; } // num
}

Foreign key definitions should be included in Table1 and Table2 classes. But these definitions don't included in classes. When i trying same case with POCO v2.35.0 the definitions are generated with correctly, you can find the results below.

Table1 generated with v2.35.0:

public class Table1
{
	public static Table1 Find(int id) => _find(db => db.Table1.Where(v => v.Id == id).FirstOrDefault());
	private static Table1 _find(Func<MyDbContext, Table1> work) => work(new MyDbContext());
	public static async Task<Table1> FindAsync(int? id) => await (new MyDbContext()).Table1.FindAsync(id);
	public static List<Table1> Table1List() => (new MyDbContext()).Table1.ToList();
	public static async Task<List<Table1>> Table1ListAsync() => await new Task<List<Table1>>(() => (new MyDbContext()).Table1.ToList());

	public long Id { get; set; } // id (Primary key)
	public long IdT { get; set; } // id_t
	public long Num { get; set; } // num

	// Foreign keys

	/// <summary>
	/// Parent Table2 pointed by [Table1].([IdT], [Num]) (FK_Table1_Table2)
	/// </summary>
	public Table2 Table2 { get; set; } // FK_Table1_Table2
}

Table2 generated with v2.35.0:

public class Table2
{
	public static Table2 Find(int id) => _find(db => db.Table2.Where(v => v.Id == id).FirstOrDefault());
	private static Table2 _find(Func<MyDbContext, Table2> work) => work(new MyDbContext());
	public static async Task<Table2> FindAsync(int? id) => await (new MyDbContext()).Table2.FindAsync(id);
	public static List<Table2> Table2List() => (new MyDbContext()).Table2.ToList();
	public static async Task<List<Table2>> Table2ListAsync() => await new Task<List<Table2>>(() => (new MyDbContext()).Table2.ToList());

	public long Id { get; set; } // id (Primary key)
	public long Num { get; set; } // num

	// Reverse navigation

	/// <summary>
	/// Child Table1 where [Table1].([id_t], [num]) point to this entity (FK_Table1_Table2)
	/// </summary>
	public System.Collections.Generic.ICollection<Table1> Table1 { get; set; } // Table1.FK_Table1_Table2

	public Table2()
	{
		Table1 = new System.Collections.Generic.List<Table1>();
	}
}

As can be seen above, there are foreign key definitions in the table created with v2.35.0. Like: in Table1 public Table2 Table2 { get; set; } // FK_Table1_Table2 and in Table2 public System.Collections.Generic.ICollection<Table1> Table1 { get; set; } // Table1.FK_Table1_Table2

My question is, how can i generate these foreign keys with the latest version? Could I solve this problem with a change in the Database.tt file? Or how can i go about solving this problem?

Thanks.

hamuryen avatar May 30 '20 09:05 hamuryen

Confirmed. Thank you for finding this bug! I am amazed this exists as it's been beaten on for so long by so many. I will investigate and fix.

sjh37 avatar May 31 '20 10:05 sjh37

Still investigating. I have now created failing unit tests causing this exact issue. There are a few GitHub issues around naming of foreign keys, and I am re-writing how the foreign key names are calculated.

sjh37 avatar Jun 17 '20 08:06 sjh37

@sjh37 Any progress on this? I believe I am having a similar issue in RPG version 3.4.1.

afust003 avatar Sep 01 '21 13:09 afust003

Yes & No. I made a start with Settings.ForeignKeyNamingStrategy which can be set to Legacy or Latest. Latest is not ready for prime time yet and still needs a lot of work.

In SingleDatabaseReverseEngineerTests.cs there is a test called ReverseEngineerSqlServer which is there to test the Latest naming strategy. Those tests are commented out for now until I get back to working on this.

Another concern is when I release this should it default to legacy or latest? For a new project, latest is not a problem. However for legacy project the developer will have to remember to keep the legacy selected or they will have to change their code to make use of the newly named FK's, which could be a pain for a large project.

The factory to create the names:

public static class ForeignKeyNamingStrategyFactory
{
    public static IForeignKeyNamingStrategy Create(IDbContextFilter filter, Table table)
    {
        switch (Settings.ForeignKeyNamingStrategy)
        {
            case ForeignKeyNamingStrategy.Legacy:
                return new LegacyForeignKeyNamingStrategy(filter, table);

            default:
                return new LatestForeignKeyNamingStrategy(filter, table);
        }
    }
}

If you take a look at LatestForeignKeyNamingStrategy it's a long way off. For now, as the number of requests for a new FK strategy is almost non existent, and the problems with the current one are so low, I have put this case as low priority. It will get done at some point as I would like a 'perfect' solution, but for now, it could be some time.

sjh37 avatar Sep 02 '21 12:09 sjh37