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

Ensure dependency order for Deploy artifacts #11265

Merged
merged 1 commit into from
Oct 4, 2021

Conversation

AndyButland
Copy link
Contributor

@AndyButland AndyButland commented Oct 4, 2021

The issue raised that this should resolve is here, where it seems to matter whether or not a template had a master template defined or not as to whether you could successfully deploy content that uses it.

After a bit of digging I found that Deploy was reporting a schema mismatch between two template entities that were identical, due to differences in how the dependencies of the template were ordered within the serialized output. This wasn't reproducible between two local environments, only between local and Cloud.

Initially I thought we weren't ensuring the order and were relying on the collection to come back in a particular way, but actually we are doing this. However the difference in behaviour looks to be due to differences in globalization settings (as we've seen in other contexts, related to this).

I've been able to demonstrate that the ordering of the string representations of these two UDIs comes back one way locally, and the opposite way in Cloud:

  var dependencies = new List<string>
  {
      "umb://template/d4651496fad24c1290a53ea4d55d945b",
      "umb://template-file/TestPage.cshtml"
  };
  foreach(var dependency in dependencies.OrderBy(x => x, StringComparer.InvariantCultureIgnoreCase))
  {
      <div>@dependency</div>
  }

Using OrdinalIgnoreCase makes them consistent though in both environments. Hence I've applied the same setting for the ordering we do in core.

In the end think we've just been quite unlucky in having two entities that happen to be ordered by - v /.

@AndyButland AndyButland requested a review from bergmania October 4, 2021 13:37
@AndyButland
Copy link
Contributor Author

I've tested this further by releasing the build from this PR as a nightly, updating my Cloud project to reference that, and can now transfer content for documents depending on a template that both has and doesn't have a master template.

@bergmania
Copy link
Member

Thanks.. That issue is ugly :P

I'll merge this, but I think the right thing is to ensure the environments run using same globalization.
E.g. by using this

  <ItemGroup Condition="'$(OS)' == 'Windows_NT'">
    <PackageReference Include="Microsoft.ICU.ICU4C.Runtime" Version="68.2.0.6" />
    <RuntimeHostConfigurationOption Include="System.Globalization.AppLocalIcu" Value="68.2" />
  </ItemGroup>

in the csproj

@bergmania bergmania merged commit eaa0f46 into v9/dev Oct 4, 2021
@bergmania bergmania deleted the v9/bugfix/enure-deploy-dependency-order branch October 4, 2021 16:44
@bergmania
Copy link
Member

I also cherry picked for 9.0.1

@bjarnef
Copy link
Contributor

bjarnef commented Oct 5, 2021

@bergmania I added your suggestion #11265 (comment) to the UmbracoProject.csproj file and deployed to Cloud environment, but it didn't seem to make a difference.

I still get same mismatch errors, when transfer content from local to remote environment.

Maybe it also need the changes in this PR as well?

Update: A bit later it seemed to work and on subsequent transfers I haven't had these mismatch errors.

@bjarnef
Copy link
Contributor

bjarnef commented Oct 27, 2021

@AndyButland @bergmania could this be the cause to the issues I am facing with entityResource.getByIds(Udi[]) in MNTP since I am seeing this in v9.0.1 on Umbraco Cloud, but it worked in v9.0.0?

See #11448

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants