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

DataType Update during packageMigration completed then overwritten by uSync import at startup #640

Open
mistyn8 opened this issue May 16, 2024 · 13 comments

Comments

@mistyn8
Copy link
Contributor

mistyn8 commented May 16, 2024

umb 13.1.1, uSync 13.1.1

I've written a package migration that as part of it's role updates a dataype configuration (blocklist)

However, although this works as expected, adding uSync into the mix causes the update to be reverted as importAtStartup : Settings is in use.

Is it that the notification service isn't present to let uSync know that a dataType has changed?
If so can we manually notify uSync? I do seem to remember there was a savewithevents in days gone past is that something that's missing?

Any pointers so that my migration supports uSync greatfully recieved

simplified code

public class PackageMigration1 : PackageMigrationBase
{
   public IglooPackageMigration1(IPackagingService packagingService, .... , IDataTypeService dataTypeService)
   {
       _dataTypeService = dataTypeService;
   }
   
   protected override void Migrate()
   {
      var contentBlockList = _dataTypeService.GetDataType(new Guid("e52e988a-65e8-45b0-87d7-dfa013357177"));
      // update our datatype and save it
      _dataTypeService.Save(contentBlockList);
      // not generating uSync config file....

   }
@KevinJump
Copy link
Owner

Hi,

I think the issue is probibly that the migrations are running before uSync has been able to register for the notification events, so it can't listen for them.

if you can you could look at triggering the migrations after the uSyncComposer has ran, but i don't think you can do that with a package migration, you would have to look at using the core migrations (which are very similar) as you have greater control over when they run in the sequence.

I don't think we can get uSync to be registred before package migrations because they happen in the core.

Kevin

@mistyn8
Copy link
Contributor Author

mistyn8 commented May 16, 2024

That makes perfect sense, thank you for the swift response.

@KevinJump
Copy link
Owner

@mistyn8
Copy link
Contributor Author

mistyn8 commented May 16, 2024

superstar! I presume I just loose the run package migrations bit and visibility in the back office? by it not being a package migration.. any thing else of note?

@KevinJump
Copy link
Owner

Also, if you add the ComposeAfter tag it will run after uSync's composer.

	[ComposeAfter(typeof(uSync.BackOffice.uSyncBackOfficeComposer))]
	public class MyComposer : IComposer
	{
		public void Compose(IUmbracoBuilder builder)
		{
			throw new System.NotImplementedException();
		}
	}

in order to ensure that uSync (and umbraco) has fully started up before you do anything, you should run your migration code in a UmbracoApplicationStartedNotification (not starting as in the examples).

Started happens slightly later, and after the DB and everything are installed and randomly the language service has been populated with all the correct translations (so will work after installs and upgrades everytime)

@KevinJump
Copy link
Owner

yes, you loose the 'run migration' button, your package will still showup in the back office just without that.

@mistyn8
Copy link
Contributor Author

mistyn8 commented May 16, 2024

Definately pushing my luck now.. ;-)
[ComposeAfter(typeof(uSync.BackOffice.uSyncBackOfficeComposer))] what happens if no uSync (optional) and not wanting to introduce a dependancy on uSync in my package?

@KevinJump
Copy link
Owner

Oh, I think you have to be very clever with reflection, but i am not 100% sure how.

@mistyn8
Copy link
Contributor Author

mistyn8 commented May 16, 2024

So looks like things get done in the right order now at least according to the logs.. but that no notifications are raised from a _dataTypeService.Save(dataType); in a migration plan

[00:32:16 INF] uSync Import: 8 handlers, processed 236 items, 0 changes in 1123ms
[00:32:16 INF] uSync: Startup Complete 1280ms
[00:32:16 INF] Starting 'DataTypeWidgetAddition'...
[00:32:16 INF] At fc40ffce-2e6a-4fe8-a0a6-afcca740baaa
[00:32:16 INF] Done

Even added a simple

public class DoStuffContentNotificationHandler : INotificationHandler<DataTypeSavingNotification>
{
    private readonly ILogger _logger;

    public DoStuffContentNotificationHandler(ILogger<DoStuffContentNotificationHandler> logger)
    {
        _logger = logger;
    }

    public void Handle(DataTypeSavingNotification notification)
    {
        _logger.LogDebug("Don't shout");
    }
}

public class DontShoutComposer : IComposer
{
    public void Compose(IUmbracoBuilder builder)
    {
        builder.AddNotificationHandler<DataTypeSavingNotification, DoStuffContentNotificationHandler>();
    }
}

with

    [ComposeAfter(typeof(DontShoutComposer))]
    public class HeroWidgetInnerOverlayStrengthMigrationComposer : IComposer
    {
        public void Compose(IUmbracoBuilder builder)
        {
            builder.AddNotificationHandler<UmbracoApplicationStartedNotification, HeroWidgetInnerOverlayStrengthMigration>();
        }
    }

Notification gets handled when using the backoffice, but not from the migration plan :-(

Think I exhausted the breath of my capability now.. will have to ship with some directions on uSync integration...

I also noticed that the core packagemigrator, persists dataypesection folder keys (guids) as well as names from the xml, uSync configs only carry the name..

<DocumentType Folders="Widgets/Grid/Accordion" FolderKeys="fadd2193-84ec-4cd5-96f7-d6901c842a7c/dad5dd1b-321a-4809-8942-5beea83b6c38/06a8a294-5815-4692-b270-3b9ef3f15d21"> just peeked my interest..

@mistyn8 mistyn8 closed this as completed May 16, 2024
@mistyn8 mistyn8 reopened this May 16, 2024
@mistyn8
Copy link
Contributor Author

mistyn8 commented May 17, 2024

FYI just tested with a cloud project and uda's aren't created by umbraco Deploy either...

@KevinJump
Copy link
Owner

Hi,

for the last bit yes, usync doesn't sync folders explicitly, rather it syncs items in folders and creates the folders as needed,
we don't sync the key of the folder because it doesn't matter, and we try to store the minimal amount of info, so we can reduce the amount of false changes (its all about performance, and portability.).

instead when a doctype is saved/created/etc the folder is located by name or created new if needed.

@mistyn8
Copy link
Contributor Author

mistyn8 commented May 17, 2024

I think there is a scenario where folder keys have merit. (much like why node keys are bestpractice UID) package installs a folder, user changes name of if.. package ships an update and wants to put item in the same folder, with a key we can find it. But name only we generate a new folder with the old name?
Edge case and a few moving parts between packagemigrartion/usync prob seen in a dev team, not the end of the world to have two folders now and sort after the fact ;-)

Also packagemigration created doctypes seem to get a uda created.. but not manual use of the datatypeservice.save() can't see anything extra going on in https://github.com/umbraco/Umbraco-CMS/blob/contrib/src/Umbraco.Infrastructure/Packaging/PackageDataInstallation.cs 🤔

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

2 participants