OrchardCMS / OrchardCore

Orchard Core is an open-source modular and multi-tenant application framework built with ASP.NET Core, and a content management system (CMS) built on top of that framework.
https://orchardcore.net
BSD 3-Clause "New" or "Revised" License
7.36k stars 2.37k forks source link

Updating from OrchardCore.Application.Cms.Core.Targets 1.2.0 1.3.0 throws migration errors #11359

Closed jameswoodley closed 4 months ago

jameswoodley commented 2 years ago

Describe the bug

When updating OrchardCore.Application.Cms.Core.Targets from v1.2.0 to v1.3.0 and running the application we are met with the following error logged:

Npgsql.PostgresException (0x80004005): 23503: update or delete on table "Document" violates foreign key constraint "fk_openidappbyrolenameindex_document_documentid" on table "OpenIdAppByRoleNameIndex_Document"

DETAIL: Key (Id)=(1053) is still referenced from table "OpenIdAppByRoleNameIndex_Document".
   at Npgsql.Internal.NpgsqlConnector.<ReadMessage>g__ReadMessageLong|213_0(NpgsqlConnector connector, Boolean async, DataRowLoadingMode dataRowLoadingMode, Boolean readingNotifications, Boolean isReadingPrependedMessage)
   at Npgsql.NpgsqlDataReader.NextResult(Boolean async, Boolean isConsuming, CancellationToken cancellationToken)
   at Npgsql.NpgsqlDataReader.NextResult()
   at YesSql.Commands.CreateIndexCommand.<AddToBatch>b__6_0(DbDataReader dr)
   at YesSql.Commands.BatchCommand.ExecuteAsync(DbConnection connection, DbTransaction transaction, ISqlDialect dialect, ILogger logger)
   at YesSql.Session.FlushAsync()
   at YesSql.Session.FlushAsync()
   at OrchardCore.OpenId.YesSql.Migrations.OpenIdMigrations.UpdateFrom7Async()
   at OrchardCore.Data.Migration.DataMigrationManager.UpdateAsync(String featureId)
  Exception data:
    Severity: ERROR
    SqlState: 23503
    MessageText: update or delete on table "Document" violates foreign key constraint "fk_openidappbyrolenameindex_document_documentid" on table "OpenIdAppByRoleNameIndex_Document"
    Detail: Key (Id)=(1053) is still referenced from table "OpenIdAppByRoleNameIndex_Document".
    SchemaName: public
    TableName: OpenIdAppByRoleNameIndex_Document
    ConstraintName: fk_openidappbyrolenameindex_document_documentid
    File: ri_triggers.c
    Line: 2541
    Routine: ri_ReportViolation

The application appears to function, but I never like to just ignore errors. Are there any other steps required when upgrading apart from bumping the Nuget package?

To Reproduce

Steps to reproduce the behavior:

  1. Update Nuget package
  2. Run application
  3. Peruse logs

Expected behavior

Database logs should be empty

jameswoodley commented 2 years ago

Have also just got the following error when attempting to log out a user, so it feels to me there has been some issues with migrations?

Npgsql.PostgresException (0x80004005): 42703: column "IsLockoutEnabled" of relation "UserIndex" does not exist
Skrypt commented 2 years ago

I'm looking at the issue.

Skrypt commented 2 years ago

https://github.com/OrchardCMS/OrchardCore/blob/62eed89890b6b9b6d71a02e1465269b9bb512207/src/OrchardCore/OrchardCore.OpenId.Core/YesSql/Migrations/OpenIdMigrations.cs#L517

Here is where we execute this Drop Index SQL query on the last migration we had (11 months ago). Though, we did not create this migration between 1.2 to 1.3. So the issue is not there.

https://github.com/OrchardCMS/OrchardCore/blob/b705a0ac1443be7d9c6c9ecb1f79f42c01042132/src/OrchardCore.Modules/OrchardCore.Users/Migrations.cs#L213

This IsLockoutEnabled column got added from that migration. Which is UpdateFrom10() added 9 months ago. So not coming from 1.2 to 1.3.

The first thing will be to fix the issue with the IsLockoutEnabled missing column in the UserIndex table. You need to look at your database to see which migrations executed last for the OrchardCore.User module and set it back to 10 so that the migration executes again.

For the OrchardCore.OpenId.Core module migration that failed please look at the migration details to make sure the changes have been applied to the different indices/tables and you will probably need to drop the index manually. If the migration failed.

Here is the code lines that failed executing on this migration :

            // Retrieve all existing applications and scopes from original Document table.
            var applications = await _session.Query<OpenIdApplication>().ListAsync();
            var scopes = await _session.Query<OpenIdScope>().ListAsync();

            // Enlist the old documents in the new collection and remove from the old collections.
            foreach (var application in applications)
            {
                // Set the id to 0 or it will be considered an updated entity.
                application.Id = 0;
                _session.Save(application, collection: OpenIdApplicationCollection);

                // Delete from the original collection.
                _session.Delete(application);
            }

            // Enlist the old documents in the new collection and remove from the old collections.
            foreach (var scope in scopes)
            {
                // Set the id to 0 or it will be considered an updated entity.
                scope.Id = 0;
                _session.Save(scope, collection: OpenIdScopeCollection);

                // Delete from the original collection.
                _session.Delete(scope);
            }

            // Flush the saved documents so that the old reduced indexes will be calculated
            // and commited to the transaction before they are then dropped.
            await _session.FlushAsync();

            SchemaBuilder.DropReduceIndexTable<OpenIdAppByRedirectUriIndex>();
            SchemaBuilder.DropReduceIndexTable<OpenIdAppByRoleNameIndex>();

            SchemaBuilder.DropMapIndexTable<OpenIdScopeIndex>();
            SchemaBuilder.DropReduceIndexTable<OpenIdScopeByResourceIndex>();

You could try to execute this code manually somewhere else in custom code or clean up the database with SQL simply.

And always remember to do a backup of the database before anything.

jameswoodley commented 2 years ago

Thanks for the explanation and pointers. Can you help me as to where to find the migrations run, we don't seem to have anything in our database that points to migrations stored. Is it something in the Orchard CMS UI somewhere? (Definitely a noob question I know, still getting to grips with the data structure)

jameswoodley commented 2 years ago

Found it in the Document table..

jameswoodley commented 2 years ago

So it seems the migration is still on version 6?

{"Id":1,"DataMigrations":[{"DataMigrationClass":"OrchardCore.Liquid.Migrations","Version":1},{"DataMigrationClass":"OrchardCore.ContentManagement.Records.Migrations","Version":5},{"DataMigrationClass":"OrchardCore.Contents.Migrations","Version":1},{"DataMigrationClass":"OrchardCore.Alias.Migrations","Version":2},{"DataMigrationClass":"OrchardCore.ContentFields.Migrations","Version":2},{"DataMigrationClass":"OrchardCore.Deployment.Migrations","Version":1},{"DataMigrationClass":"OrchardCore.Widgets.Migrations","Version":1},{"DataMigrationClass":"OrchardCore.Flows.Migrations","Version":3},{"DataMigrationClass":"OrchardCore.Html.Migrations","Version":5},{"DataMigrationClass":"OrchardCore.Indexing.Migrations","Version":1},{"DataMigrationClass":"OrchardCore.Layers.Migrations","Version":1},{"DataMigrationClass":"OrchardCore.Lists.Migrations","Version":2},{"DataMigrationClass":"OrchardCore.Markdown.Migrations","Version":4},{"DataMigrationClass":"OrchardCore.Media.Migrations","Version":1},{"DataMigrationClass":"OrchardCore.Title.Migrations","Version":2},{"DataMigrationClass":"OrchardCore.Menu.Migrations","Version":2},{"DataMigrationClass":"OrchardCore.OpenId.YesSql.Migrations.OpenIdMigrations","Version":4},{"DataMigrationClass":"OrchardCore.Users.Migrations","Version":6},{"DataMigrationClass":"OrchardCore.Taxonomies.Migrations","Version":3},{"DataMigrationClass":"OrchardCore.Workflows.Migrations","Version":2}]}

Is there a way to run them again and get through to the latest?

Skrypt commented 2 years ago

If the Migration is not successfully run then it will run again on every application start. So, restarting the application pool or website will run the migrations again.

jameswoodley commented 2 years ago

So starting the application again gives

[20:12:28 ERR] Error while running migration version 7 for 'OrchardCore.OpenId'.
Npgsql.PostgresException (0x80004005): 23503: update or delete on table "Document" violates foreign key constraint "fk_openidappbyrolenameindex_document_documentid" on table "OpenIdAppByRoleNameIndex_Document"

DETAIL: Key (Id)=(1053) is still referenced from table "OpenIdAppByRoleNameIndex_Document".
   at Npgsql.Internal.NpgsqlConnector.<ReadMessage>g__ReadMessageLong|213_0(NpgsqlConnector connector, Boolean async, DataRowLoadingMode dataRowLoadingMode, Boolean readingNotifications, Boolean isReadingPrependedMessage)
   at Npgsql.NpgsqlDataReader.NextResult(Boolean async, Boolean isConsuming, CancellationToken cancellationToken)
   at Npgsql.NpgsqlDataReader.NextResult()
   at YesSql.Commands.CreateIndexCommand.<AddToBatch>b__6_0(DbDataReader dr)
   at YesSql.Commands.BatchCommand.ExecuteAsync(DbConnection connection, DbTransaction transaction, ISqlDialect dialect, ILogger logger)
   at YesSql.Session.FlushAsync()
   at YesSql.Session.FlushAsync()
   at OrchardCore.OpenId.YesSql.Migrations.OpenIdMigrations.UpdateFrom7Async()
   at OrchardCore.Data.Migration.DataMigrationManager.UpdateAsync(String featureId)
  Exception data:
    Severity: ERROR
    SqlState: 23503
    MessageText: update or delete on table "Document" violates foreign key constraint "fk_openidappbyrolenameindex_document_documentid" on table "OpenIdAppByRoleNameIndex_Document"
    Detail: Key (Id)=(1053) is still referenced from table "OpenIdAppByRoleNameIndex_Document".
    SchemaName: public
    TableName: OpenIdAppByRoleNameIndex_Document
    ConstraintName: fk_openidappbyrolenameindex_document_documentid
    File: ri_triggers.c
    Line: 2541
    Routine: ri_ReportViolation

So I believe because this migration is failing, it then doesn't attempt any more? Do I need to manually apply this migration in order to get past this point?

Skrypt commented 2 years ago

Normally it should run fine. Here, what we do is clean the Database of these records which could cause a constraint issue. It seems like it doesn't remove these records if you can't remove this index. Moreover, if this fails with PostgreSQL then there is an issue that needs to be fixed. But in the meantime, I think you will need to manually apply the migration unless you want to wait until we fixed the issue.

jameswoodley commented 2 years ago

Any idea on a timeframe for the fix? Understanding this is open source so not expecting minutes, but are we talking months?

Skrypt commented 2 years ago

Generally, if you are using a Nuget packages solution this could be quick. Else, you need to wait for the next release which could be months. Though, here, I have no idea what is happening because it works perfectly with SQL Server so the fix might be required in YesSQL. I need to be able to repro the issue and fix it here and there so it could take some time.

jameswoodley commented 2 years ago

Okay makes sense, we are using a Nuget packages solution so that bodes well. I'll fix it manually, but if you need any help reproing I'm happy to be guinea pig!

Thanks for the help today

jameswoodley commented 2 years ago

As a heads up, deleting the OpenID Application data from OpenIdAppByRoleNameIndex_Document and OpenIdApplicationIndex tables and then restarting the application meant no errors logged and the migrations then ran. Looks like an old fashioned foreign key constraint issue!

DELETE FROM "OpenIdAppByRoleNameIndex_Document"
DELETE FROM "OpenIdApplicationIndex"
mwpowellhtx commented 2 years ago

So what's the migration path for postgresql again? Delete content and start from scratch? We do not have months; we want to be productive with this YESTERDAY. That or perhaps OrchardCore is not a good fit for us after all.

Skrypt commented 2 years ago

@mwpowellhtx I explained how to do it manually and kindly took my own free time to do so. Normally this migration will work. Of course, this is an open-source project so everyone is welcome to contribute. I did not let @jameswoodley without answers at least. I'm currently working on also other important things.

That or perhaps OrchardCore is not a good fit for us after all.

This is your call, not everyone wants to dig in the code or fix issues when they happen. Though, we try to support each others here. If there is an urgency then try to reach out to people that can do it quickly and don't forget to give sponsorship to them. We do this for free most of the time.

If you read again what I said is that he needs to clean the database manually with some SQL Queries so that his Index Constraint doesn't block him when he tries to drop it. That doesn't mean to restart from scratch. And, I can't do this for him either. I would need to have his database and make it work. So that's a maintenance task that requires some SQL skills.

Also, about being productive with this YESTERDAY that is not up to me. Of course, there is a learning curve in Orchard Core like with any other CMS and/or framework out there.

Your comment looked a little cynical to me, sorry if we did not meet your standards yet. Issues happen in every complex systems and this is not how this will get fixed faster.

mwpowellhtx commented 2 years ago

Bit cynical perhaps, yes. With this issue trying to decide which is a core OrchardCore/CMS migration and which is not. Sounds like there is a customization which was inadvertently impacted, I do not know. Or how to express an UpdateFromN.5 i.e. perhaps before/after updatefrom events would be appropriate when there is a looming update. Of course with migrations, forward knowledge is the awkward part. I do not know of a great way to circumvent that moment of opportunity if we happened to extend the core framework for any reason.

jameswoodley commented 2 years ago

This is expected in FOS systems. I had an issue that my limited knowledge of the product couldn't solve. I reached out and got help, I solved my issue at the same time as raising there is a bug with the system.

Using this collaboration, we got to a positive outcome. I got my migration run, the maintainers found a bug to fix.

If anything, this incident should show reasons in favour of using OrchardCore rather than against! You think this sort of open collaboration would have come out of the big "paid for" solutions?

mwpowellhtx commented 2 years ago

@jameswoodley Sure that's not really the question I have. I'm still not clear what the issue actually was, in a word or two? Is it something that needs to be patched, when we might expect that in a nuget package?

Of course, depending on the concern, issue, opportunity at hand, justifies whether OSS is worth what commitment. The more time spent upstream is less time spent being productive in business related downstream opportunities. But you already know this, I'm sure.

jameswoodley commented 2 years ago

@jameswoodley Sure that's not really the question I have. I'm still not clear what the issue actually was, in a word or two? Is it something that needs to be patched, when we might expect that in a nuget package?

Of course, depending on the concern, issue, opportunity at hand, justifies whether OSS is worth what commitment. The more time spent upstream is less time spent being productive in business related downstream opportunities. But you already know this, I'm sure.

Would have to let the maintainers comment on what the actual issue is but for some reason data wasn't removed from a table before it was attempted to be removed, so an error was thrown.

Manually deleting said data then allowed the table to be dropped

mwpowellhtx commented 2 years ago

@jameswoodley Huh, which to me beggars the larger question. Is it somewhat the point that 'the data' may also be considered 'the source'? So a healthy migration should account for that, establish the side temp table, and re-insert data, should it not?

jameswoodley commented 2 years ago

And I believe that is the bug they are looking into.

mwpowellhtx commented 2 years ago

@jameswoodley Okay dokay then... fair enough, good to know.

mwpowellhtx commented 2 years ago

@sebastienros Per our gitter exchange, what is the resolution for this? If I read all this correctly, it effectively bricks existing/prospective postgres solution providers. So a fix for this is not coming for how long? 1.3.1 emergency patch seems like the most appropriate response to me, so what is the issue exactly?

PiemP commented 2 years ago

I think to had the same problem on SqlServer 15.0.2095.3, when I tried to move my code from the pre 1.0 OrchardCore version to the 1.4.0 .

I have solved the issue by dropping the constraint FK_OpenIdApplicationIndex (miss all the tables prefix) from the table OpenIdApplicationIndex an the constraint FK_OpenIdAppByRoleNameIndex_Document_DocumentId from the table OpenIdAppByRoleNameIndex_Document. After that the migration 7 continue to fail.

Then, have downloaded the 1.4.0 source code, I have edit the migration code by comment this 2 lines of code:

https://github.com/OrchardCMS/OrchardCore/blob/49cc18fcdeb409ec4ad71569a0e2d41a0d5cad17/src/OrchardCore/OrchardCore.OpenId.Core/YesSql/Migrations/OpenIdMigrations.cs#L514

https://github.com/OrchardCMS/OrchardCore/blob/49cc18fcdeb409ec4ad71569a0e2d41a0d5cad17/src/OrchardCore/OrchardCore.OpenId.Core/YesSql/Migrations/OpenIdMigrations.cs#L517

I believe exist a problem of execution order: probably it's necessary to drop the constraint before do operations on the tables but I don't have completely analyzed the consequences of my modifications and I'm not sure to have solved the problem.

Piedone commented 4 months ago

Please use the above workaround, and reopen if that's not suitable.