Content step in a migration recipe fails

[ArieGato]

Hi,

A taxonomy field is not added to a custom part when I use a TaxonomyContentItemId that was added by a recipe migration in the same data migration.

I created a repo for this: https://github.com/ArieGato/OrchardCore/tree/ariegato/recipe-migration

To reproduce follow these steps:

1. Create a new web site by using the Coming Soon theme.
2. Activate the 'Recipe Migration Issue' module.
   The migration checks for the existence of a taxonomy content item by alias. When it's not present it creates it by using a recipe migration.
3. Check the content types. The 'Custom' type has been created with a Custom part, but the part cannot be found at the Content Parts page. When editing the Custom Part from the Custom type details a 404 page not found error is shown.

Now to see how it should work:

When the Taxonomy content item with the alias has already been created, the migration works fine.

1. Create a new web site by using the Blog recipe (which creates the taxonomy categories)
2. Activate the module Recipe Migration Issue.
3. Check the Custom Part in the Content Parts page. The part is created correctly with the Taxonomy field.

Thnx,
Arjan

[hishamco]

/cc @deanmarcussen

[ArieGato]

Is there any progress on this issue? Can it be that there are two transactions? One for the recipe migration and one for the migration. Or maybe some content item cache isn't invalidated after the migration recipe.

I want to help on this, but I would need some pointers where to look.

[ArieGato]

I did some more investigation and I checked the contents of the database table Document. After enabling the feature there are two records for OrchardCore.ContentManagement.Metadata.Records.ContentDefinitionRecord, OrchardCore.ContentManagement.Abstractions.
The first one doesn't include the CustomPart with the TaxonomyField and the second one does. I asume that the first record needed to be updated, but a new record was inserted.

[ArieGato]

I reduced the issue to a bare minimum and it doesn't have anything to do with Taxonomies. I updated the repo: https://github.com/ArieGato/OrchardCore/tree/ariegato/recipe-migration

@hishamco can you remove the Taxonomies label?

It comes down to this:
When using the RecipeMigrator in the DataMigration, a new record is added to the Document table for the ContentDefinitionRecord.

@sebastienros can this be caused by two transactions in the orm?

The reason the problem didn't occur when using TheBlogTheme is that TheBlogTheme is using the feature OrchardCore.Contents.FileContentDefinition. So the file with the ContentDefinitionRecord is just overwritten and all is well.

[jtkech]

@ArieGato

Yes you're right

Each recipe step are excuted in a new scope where the session is committed at the end of the scope, so normally it is okay. But migrations are done first when activating the shell, and idem for migration recipes (through the RecipeMigrator) that are only intended to act on content definitions. And all of these need and are done in the same scope, that's why it doesn't work.

See this line in RecipeMigrator.cs

        recipeDescriptor.RequireNewScope = false;

[ArieGato]

@jtkech

So, is this a bug?

[jtkech]

@ArieGato

No, the migration recipe executed in your Migrations so through the RecipeMigrator (and with other automatic migrations) should not include a Content step.

[ArieGato]

@jtkech
I have to disagree on this one. In the docs it clearly states that anything from a normal recipe can be used in a recipie migration.

A recipe migration is a way to perform updates via a recipe file. The most common uses for this would be to update metadata like content types or workflows, but one could update anything that is updateable via a recipe.

[jtkech]

@ArieGato

Yes regarding the doc you're right ;)

The technical problem is that the first time we create a scope on a shell we activate it by calling activating handlers and this is where migrations are executed, so if in migrations we create a new scope on this shell whose activation is not completed we would call activating handlers again and again.

That said, in the same scope it may work because when we do a session save and then a query there is an auto flush so that you get updated data (even if not yet commited) and, since a recent change, without having to redo a session save

But migrations create tables / indexes, so if your content step needs them not sure it can work.

Anyway, as soon as possible i will try your repo directly and see what happens, i will let you know

[jtkech]

@ArieGato

Okay i could repro, there are 2 definition records and the 1rst one doesn't include the Custom type

If in your migrations i add the Custom type before executing your migration recipe, so that all migrations stuff is done before the content step (included in your migration recipe), it works there is only one definition and with the custom type

That said i found 3 ways to fix it, not so easy to comment, i will do a PR tomorrow with more comments

Anyway will be fixed soon ;)

[ArieGato]

Anyway will be fixed soon ;)

@jtkech

that's great!

Do you want me to create a more complex migration with multiple steps and the original TaxonomyField included in the CustomPart?

Also, maybe you can change the title of the issue? It has nothing to do with Taxonomies, but with the Content step in the recipe migration.

[ArieGato]

@jtkech

I cherry picked the commit and tested it. It looks like something doesn't work anymore. The newly created content by the recipe migrator is not available to the ContentAliasManager.

1. Add a content item with a recipe migration.
2. Get the ContentItemId of the created content with the ContentAliasManager
3. The id is null

    // create the categories content item
    await _recipeMigrator.ExecuteAsync("addcategory.recipe.json", this);
    categoriesContentItemId = await _contentAliasManager.GetContentItemIdAsync("alias:categories");

[jtkech]

@ArieGato

See my comment in the related PR

... Then, when the migrations of a given feature want to execute a migration recipe, the RecipeMigrator uses a DeferredTask so that the migration recipe will be executed after disposing the scope used to activate the shell and where all other regular migations has been done and then committed ...

So in my PR, when you call _recipeMigrator.ExecuteAsync() the steps will be executed in a deferred task at the end of the current scope, so when you call GetContentItemIdAsync() your recipe has not been executed yet.

If you use it in a DataMigration the current scope is the one used to activate the shell and normally here you don't have to do things like GetContentItemIdAsync().

If you want to use what you did somewhere else (not in a DataMigration), you would need to rather use the IRecipeExecutor that still doesn't use a deferred task, in place of the IRecipeMigrator.

[ArieGato]

@jtkech

If you use it in a DataMigration the current scope is the one used to activate the shell and normally here you don't have to do things like GetContentItemIdAsync()

I need the Id of the new Taxonomy content item for the Taxonomy field settings. If I understand correctly this is not possible.

Then how can I add a Taxonomy field to a ContentPart in a DataMigration when the Taxonomy content item has not yet been created?

[jtkech]

@ArieGato

I found a better and simpler way to fix it, see my last comment in #6648, here the main part

The simpler solution in that case is to just return the loaded document but indicating that it should not be cached. It is still thread safe as the loaded document is not put in a shared cache and then only used in a given scope.

And without doing a session detach on this already loaded document, so it fixes the issue and without having to execute a migration recipe in a deferred task, as before. I retried your module in this context and it was working

[ArieGato]

Very nice!! I tested a more complex scenario and all works well. I'll try to come up with some more edge cases, but I'm confident it works nice!

Thnx. I really appreciate this.

[jtkech]

@ArieGato Cool, thanks for having tried it