From 9941abef3a2be7c72eae96668f3773ab2c881a3d Mon Sep 17 00:00:00 2001 From: TaoziZ03 <106975749+TaoziZ03@users.noreply.github.com> Date: Mon, 14 Sep 2026 10:10:02 +0000 Subject: [PATCH 01/10] Add opt-in same-Web publishing conversion --- ...ublishingPageInPlaceTransformationTests.cs | 168 ++++++++++++++++++ ...PublishingPageTransformationInformation.cs | 6 + .../PublishingPageTransformationValidator.cs | 69 +++++++ .../Publishing/PublishingPageTransformator.cs | 91 ++++++++-- .../Modernization/Telemetry/LogStrings.cs | 4 + 5 files changed, 327 insertions(+), 11 deletions(-) create mode 100644 src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs create mode 100644 src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationValidator.cs diff --git a/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs b/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs new file mode 100644 index 000000000..bfcf2309e --- /dev/null +++ b/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs @@ -0,0 +1,168 @@ +using System; +using Microsoft.SharePoint.Client; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using PnP.Framework.Modernization.Cache; +using PnP.Framework.Modernization.Publishing; +using PnP.Framework.Modernization.Transform; +using PnP.Framework.Utilities.UnitTests.Model; +using PnP.Framework.Utilities.UnitTests.Web; + +namespace PnP.Framework.Modernization.Tests.Transform.Publishing +{ + [TestClass] + public class PublishingPageInPlaceTransformationTests + { + private static readonly Guid SourceSiteId = Guid.Parse("c33f44c0-e1ce-4d87-a961-98669e76bca6"); + private static readonly Guid SourceWebId = Guid.Parse("a467c0f4-dcf4-49f3-8a08-e33de32d3202"); + + [TestMethod] + public void InPlacePublishingPageDefaultsToFalse() + { + var information = new PublishingPageTransformationInformation(null); + + Assert.IsFalse(information.InPlacePublishingPage); + } + + [TestMethod] + public void SameWebRequiresExplicitOptIn() + { + var result = ValidateSameWeb(inPlacePublishingPage: false, targetWebTemplate: "ENTERWIKI", hasWritableSitePages: true); + + Assert.AreEqual(PublishingPageTransformationTarget.SameSiteCollectionNotAllowed, result); + } + + [TestMethod] + public void SameSiteCollectionDifferentWebFailsClosed() + { + var result = PublishingPageTransformationValidator.ValidateTarget( + true, + SourceSiteId, + SourceSiteId, + SourceWebId, + Guid.Parse("7896708f-f5ea-4f22-aeb3-6223bf8897b0"), + "ENTERWIKI", + true); + + Assert.AreEqual(PublishingPageTransformationTarget.SameSiteCollectionDifferentWeb, result); + } + + [TestMethod] + public void SameWebRequiresEnterpriseWikiAndWritableSitePages() + { + Assert.AreEqual( + PublishingPageTransformationTarget.SameWebRequiresEnterpriseWiki, + ValidateSameWeb(true, "STS", true)); + Assert.AreEqual( + PublishingPageTransformationTarget.SameWebRequiresWritableSitePages, + ValidateSameWeb(true, "ENTERWIKI", false)); + Assert.AreEqual( + PublishingPageTransformationTarget.SameWeb, + ValidateSameWeb(true, "enterwiki", true)); + } + + [TestMethod] + public void CrossSiteCollectionKeepsExistingTargetPolicyAndPermissionSemantics() + { + var result = PublishingPageTransformationValidator.ValidateTarget( + false, + SourceSiteId, + Guid.Parse("2e33ad34-f43a-4d75-af55-e0deba981252"), + SourceWebId, + Guid.Empty, + "UNSUPPORTED", + false); + + Assert.AreEqual(PublishingPageTransformationTarget.CrossSiteCollection, result); + Assert.IsTrue(PublishingPageTransformationValidator.CanOverwriteTarget(result, true)); + Assert.IsTrue(PublishingPageTransformationValidator.UsesCrossSitePermissionSemantics(result)); + } + + [TestMethod] + public void SameWebNeverOverwritesAndUsesInPlacePermissionSemantics() + { + var result = ValidateSameWeb(true, "ENTERWIKI", true); + + Assert.IsFalse(PublishingPageTransformationValidator.CanOverwriteTarget(result, true)); + Assert.IsFalse(PublishingPageTransformationValidator.UsesCrossSitePermissionSemantics(result)); + } + + [TestMethod] + public void SameWebUrlTransformationIsANoOp() + { + const string webUrl = "https://contoso.sharepoint.com/sites/enterprise-wiki"; + const string sourceUrl = "/sites/enterprise-wiki/Pages/source.aspx"; + + using (var sourceContext = CreateContext(webUrl)) + using (var targetContext = CreateContext(webUrl)) + { + var information = new PublishingPageTransformationInformation(null); + var transformator = new UrlTransformator(information, sourceContext, targetContext); + + Assert.AreEqual(sourceUrl, transformator.Transform(sourceUrl)); + } + } + + [TestMethod] + public void SameWebAssetTransferIsANoOp() + { + const string webUrl = "https://contoso.sharepoint.com/sites/enterprise-wiki"; + const string sourceAssetUrl = "/sites/enterprise-wiki/PublishingImages/image.png"; + + using (var sourceContext = CreateContext(webUrl)) + using (var targetContext = CreateContext(webUrl)) + { + var transfer = new AssetTransfer(sourceContext, targetContext); + + Assert.AreEqual(sourceAssetUrl, transfer.TransferAsset(sourceAssetUrl, "target.aspx")); + } + } + + private static PublishingPageTransformationTarget ValidateSameWeb( + bool inPlacePublishingPage, + string targetWebTemplate, + bool hasWritableSitePages) + { + return PublishingPageTransformationValidator.ValidateTarget( + inPlacePublishingPage, + SourceSiteId, + SourceSiteId, + SourceWebId, + SourceWebId, + targetWebTemplate, + hasWritableSitePages); + } + + private static ClientContext CreateContext(string webUrl) + { + var responseProvider = new MockEntryResponseProvider(); + responseProvider.ResponseEntries.Add(new MockResponseEntry + { + Url = webUrl, + PropertyName = "Web", + ReturnValue = new + { + Url = webUrl, + ServerRelativeUrl = "/sites/enterprise-wiki", + }, + }); + responseProvider.ResponseEntries.Add(new MockResponseEntry + { + Url = webUrl, + PropertyName = "Site", + ReturnValue = new + { + Url = webUrl, + ServerRelativeUrl = "/sites/enterprise-wiki", + }, + }); + + CacheManager.Instance.SetSharePointVersion(new Uri(webUrl), SPVersion.SPO); + CacheManager.Instance.SetExactSharePointVersion(new Uri(webUrl), "16.0.0.26000"); + + return new ClientContext(webUrl) + { + WebRequestExecutorFactory = new MockWebRequestExecutorFactory(responseProvider), + }; + } + } +} diff --git a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationInformation.cs b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationInformation.cs index c5b306da2..579ab87d0 100644 --- a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationInformation.cs +++ b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationInformation.cs @@ -47,6 +47,7 @@ public PublishingPageTransformationInformation(ListItem sourcePage, bool overwri DisablePageComments = false; SkipUserMapping = false; TargetPageFolderOverridesDefaultFolder = false; + InPlacePublishingPage = false; // Populate with OOB mapping properties MappingProperties = new Dictionary(5) { @@ -95,6 +96,11 @@ public virtual bool IsFieldUsed(string fieldName) #endregion #region Page Properties + /// + /// Explicitly allows an Enterprise Wiki publishing page to be transformed to Site Pages in the same Web. + /// The source publishing page is retained and an existing target page is never overwritten. + /// + public bool InPlacePublishingPage { get; set; } #endregion } } diff --git a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationValidator.cs b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationValidator.cs new file mode 100644 index 000000000..7aca6bda0 --- /dev/null +++ b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationValidator.cs @@ -0,0 +1,69 @@ +using System; + +namespace PnP.Framework.Modernization.Publishing +{ + internal enum PublishingPageTransformationTarget + { + CrossSiteCollection, + SameWeb, + SameSiteCollectionNotAllowed, + SameSiteCollectionDifferentWeb, + SameWebRequiresEnterpriseWiki, + SameWebRequiresWritableSitePages, + } + + /// + /// Pure validation rules for choosing the publishing page transformation target mode. + /// + internal static class PublishingPageTransformationValidator + { + private const string EnterpriseWikiWebTemplate = "ENTERWIKI"; + + internal static PublishingPageTransformationTarget ValidateTarget( + bool inPlacePublishingPage, + Guid sourceSiteId, + Guid targetSiteId, + Guid sourceWebId, + Guid targetWebId, + string targetWebTemplate, + bool hasWritableSitePages) + { + if (sourceSiteId != targetSiteId) + { + return PublishingPageTransformationTarget.CrossSiteCollection; + } + + if (sourceWebId != targetWebId) + { + return PublishingPageTransformationTarget.SameSiteCollectionDifferentWeb; + } + + if (!inPlacePublishingPage) + { + return PublishingPageTransformationTarget.SameSiteCollectionNotAllowed; + } + + if (!string.Equals(targetWebTemplate, EnterpriseWikiWebTemplate, StringComparison.OrdinalIgnoreCase)) + { + return PublishingPageTransformationTarget.SameWebRequiresEnterpriseWiki; + } + + if (!hasWritableSitePages) + { + return PublishingPageTransformationTarget.SameWebRequiresWritableSitePages; + } + + return PublishingPageTransformationTarget.SameWeb; + } + + internal static bool CanOverwriteTarget(PublishingPageTransformationTarget target, bool overwriteRequested) + { + return target == PublishingPageTransformationTarget.CrossSiteCollection && overwriteRequested; + } + + internal static bool UsesCrossSitePermissionSemantics(PublishingPageTransformationTarget target) + { + return target == PublishingPageTransformationTarget.CrossSiteCollection; + } + } +} diff --git a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs index 3a7c1ba61..71c30b041 100644 --- a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs +++ b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs @@ -175,17 +175,52 @@ public string Transform(PublishingPageTransformationInformation publishingPageTr PopulateGlobalProperties(sourceClientContext, targetClientContext); - if (sourceClientContext.Site.Id.Equals(targetClientContext.Site.Id)) + var sameSiteCollection = sourceClientContext.Site.Id.Equals(targetClientContext.Site.Id); + var sourceWebId = sameSiteCollection ? sourceClientContext.Web.EnsureProperty(p => p.Id) : Guid.Empty; + var hasWritableSitePages = false; + + if (sameSiteCollection && + sourceWebId.Equals(targetClientContext.Web.Id) && + publishingPageTransformationInformation.InPlacePublishingPage && + string.Equals(targetClientContext.Web.WebTemplate, "ENTERWIKI", StringComparison.InvariantCultureIgnoreCase)) { - // Oops, seems source and target point to the same site collection...that's a no go for publishing portal page transformation! - LogError(LogStrings.Error_SameSiteTransferNoAllowedForPublishingPages, LogStrings.Heading_SharePointConnection); - throw new ArgumentNullException(LogStrings.Error_SameSiteTransferNoAllowedForPublishingPages); + hasWritableSitePages = HasWritableSitePagesLibrary(sourceClientContext); + } + + var transformationTarget = PublishingPageTransformationValidator.ValidateTarget( + publishingPageTransformationInformation.InPlacePublishingPage, + sourceClientContext.Site.Id, + targetClientContext.Site.Id, + sourceWebId, + targetClientContext.Web.Id, + targetClientContext.Web.WebTemplate, + hasWritableSitePages); + + switch (transformationTarget) + { + case PublishingPageTransformationTarget.SameSiteCollectionNotAllowed: + LogError(LogStrings.Error_SameSiteTransferNoAllowedForPublishingPages, LogStrings.Heading_SharePointConnection); + throw new ArgumentNullException(LogStrings.Error_SameSiteTransferNoAllowedForPublishingPages); + case PublishingPageTransformationTarget.SameSiteCollectionDifferentWeb: + LogError(LogStrings.Error_InPlacePublishingPageDifferentWeb, LogStrings.Heading_SharePointConnection); + throw new ArgumentException(LogStrings.Error_InPlacePublishingPageDifferentWeb); + case PublishingPageTransformationTarget.SameWebRequiresEnterpriseWiki: + LogError(LogStrings.Error_InPlacePublishingPageRequiresEnterpriseWiki, LogStrings.Heading_SharePointConnection); + throw new ArgumentException(LogStrings.Error_InPlacePublishingPageRequiresEnterpriseWiki); + case PublishingPageTransformationTarget.SameWebRequiresWritableSitePages: + LogError(LogStrings.Error_InPlacePublishingPageRequiresWritableSitePages, LogStrings.Heading_SharePointConnection); + throw new ArgumentException(LogStrings.Error_InPlacePublishingPageRequiresWritableSitePages); + case PublishingPageTransformationTarget.SameWeb: + // Use the source context for all writes so existing in-place permission, URL and asset semantics apply. + targetClientContext = sourceClientContext; + break; } LogInfo($"{targetClientContext.Web.GetUrl()}", LogStrings.Heading_Summary, LogEntrySignificance.TargetSiteUrl); // Need to add further validation for target template - if (targetClientContext.Web.WebTemplate != "SITEPAGEPUBLISHING" && targetClientContext.Web.WebTemplate != "STS" && + if (transformationTarget == PublishingPageTransformationTarget.CrossSiteCollection && + targetClientContext.Web.WebTemplate != "SITEPAGEPUBLISHING" && targetClientContext.Web.WebTemplate != "STS" && targetClientContext.Web.WebTemplate != "GROUP" && targetClientContext.Web.WebTemplate != "BDR" && targetClientContext.Web.WebTemplate != "DEV") { @@ -323,11 +358,19 @@ public string Transform(PublishingPageTransformationInformation publishingPageTr } catch (Exception ex) { - if(ex is ArgumentException) + var targetPageDoesNotExist = ex is ArgumentException && + ex.Message.EndsWith(LogStrings.TransformPageDoesNotExistInWeb, StringComparison.InvariantCulture); + + if (targetPageDoesNotExist) { //Non-critical error generated LogInfo(LogStrings.CheckPageExistsError, LogStrings.Heading_PageCreation); } + else if (transformationTarget == PublishingPageTransformationTarget.SameWeb) + { + // The in-place opt-in must fail closed when the target collision probe is inconclusive. + throw; + } else { //Something else occurred @@ -343,9 +386,11 @@ public string Transform(PublishingPageTransformationInformation publishingPageTr { LogInfo(LogStrings.PageAlreadyExistsInTargetLocation, LogStrings.Heading_PageCreation); - if (!publishingPageTransformationInformation.Overwrite) + if (!PublishingPageTransformationValidator.CanOverwriteTarget(transformationTarget, publishingPageTransformationInformation.Overwrite)) { - var message = $"{LogStrings.PageNotOverwriteIfExists} {publishingPageTransformationInformation.TargetPageName}."; + var message = transformationTarget == PublishingPageTransformationTarget.SameWeb + ? $"{LogStrings.Error_InPlacePublishingPageTargetExists} {publishingPageTransformationInformation.TargetPageName}." + : $"{LogStrings.PageNotOverwriteIfExists} {publishingPageTransformationInformation.TargetPageName}."; LogError(message, LogStrings.Heading_PageCreation); throw new ArgumentException(message); } @@ -534,10 +579,10 @@ public string Transform(PublishingPageTransformationInformation publishingPageTr Start(); #endif // Check if we do have item level permissions we want to take over - listItemPermissionsToKeep = GetItemLevelPermissions(true, pagesLibrary, publishingPageTransformationInformation.SourcePage, savedTargetPage.ListItemAllFields); + var usesCrossSitePermissionSemantics = PublishingPageTransformationValidator.UsesCrossSitePermissionSemantics(transformationTarget); + listItemPermissionsToKeep = GetItemLevelPermissions(usesCrossSitePermissionSemantics, pagesLibrary, publishingPageTransformationInformation.SourcePage, savedTargetPage.ListItemAllFields); - // When creating the page in another site collection we'll always want to copy item level permissions if specified - ApplyItemLevelPermissions(true, savedTargetPage.ListItemAllFields, listItemPermissionsToKeep); + ApplyItemLevelPermissions(usesCrossSitePermissionSemantics, savedTargetPage.ListItemAllFields, listItemPermissionsToKeep); #if DEBUG && MEASURE Stop("Permission handling"); #endif @@ -668,6 +713,30 @@ public static string LoadDefaultPageLayoutMappingFile() } #region Helper methods + private bool HasWritableSitePagesLibrary(ClientContext context) + { + try + { + var sitePagesServerRelativeUrl = UrlUtility.Combine(context.Web.ServerRelativeUrl, "SitePages"); + var sitePagesLibrary = context.Web.GetList(sitePagesServerRelativeUrl); + context.Load(sitePagesLibrary, + p => p.Id, + p => p.BaseTemplate, + p => p.EffectiveBasePermissions); + context.ExecuteQueryRetry(); + + return sitePagesLibrary.Id != Guid.Empty && + sitePagesLibrary.BaseTemplate == (int)ListTemplateType.WebPageLibrary && + sitePagesLibrary.EffectiveBasePermissions.Has(PermissionKind.AddListItems) && + sitePagesLibrary.EffectiveBasePermissions.Has(PermissionKind.EditListItems); + } + catch (Exception ex) + { + LogWarning($"{LogStrings.Error_InPlacePublishingPageRequiresWritableSitePages} {ex.Message}", LogStrings.Heading_SharePointConnection); + return false; + } + } + private void SetPageTitle(PublishingPageTransformationInformation publishingPageTransformationInformation, PnPCore.IPage targetPage) { string titleValue = ""; diff --git a/src/lib/PnP.Framework/Modernization/Telemetry/LogStrings.cs b/src/lib/PnP.Framework/Modernization/Telemetry/LogStrings.cs index 3f64dbca7..4969acc9b 100644 --- a/src/lib/PnP.Framework/Modernization/Telemetry/LogStrings.cs +++ b/src/lib/PnP.Framework/Modernization/Telemetry/LogStrings.cs @@ -78,6 +78,10 @@ public static class LogStrings public const string Error_OverridingTagePageTakesSourcePageName = "Overriding 'TargetPageTakesSourcePageName' to ensure that the newly created page in the other site collection gets the same name as the source page"; public const string Error_FallBackToSameSiteTransfer = "Oops, seems source and target point to the same site collection...switch back the 'source only' mode"; public const string Error_SameSiteTransferNoAllowedForPublishingPages = "Oops, seems source and target point to the same site collection...that's a no go for publishing portal page transformation!"; + public const string Error_InPlacePublishingPageDifferentWeb = "In-place publishing page transformation requires source and target to be the same Web."; + public const string Error_InPlacePublishingPageRequiresEnterpriseWiki = "In-place publishing page transformation is only supported for an Enterprise Wiki Web."; + public const string Error_InPlacePublishingPageRequiresWritableSitePages = "In-place publishing page transformation requires an existing Site Pages library with Add Items and Edit Items permissions."; + public const string Error_InPlacePublishingPageTargetExists = "In-place publishing page transformation cannot overwrite an existing target page."; public const string Error_CrossSiteTransferTargetsNonModernSite = "Page transformation for targeting non-modern sites is currently not supported."; public const string Error_GetVersionError = "Setting version stamp error"; public const string Error_MissingSitePagesLibrary = "Site does not have a sitepages library and therefore this page can't be a client side page."; From ecc61dbb296613f9249bdbca2e8b4d8e21fc781c Mon Sep 17 00:00:00 2001 From: TaoziZ03 <106975749+TaoziZ03@users.noreply.github.com> Date: Mon, 14 Sep 2026 11:58:22 +0000 Subject: [PATCH 02/10] Fail closed in-place publishing targets before writes --- ...ublishingPageInPlaceTransformationTests.cs | 202 ++++++++++++++++++ .../PublishingPageTransformationValidator.cs | 15 +- .../Publishing/PublishingPageTransformator.cs | 160 ++++++++------ .../Modernization/Telemetry/LogStrings.cs | 1 + 4 files changed, 308 insertions(+), 70 deletions(-) diff --git a/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs b/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs index bfcf2309e..c37fba580 100644 --- a/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs +++ b/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs @@ -1,8 +1,11 @@ using System; +using System.Collections.Generic; +using System.Linq; using Microsoft.SharePoint.Client; using Microsoft.VisualStudio.TestTools.UnitTesting; using PnP.Framework.Modernization.Cache; using PnP.Framework.Modernization.Publishing; +using PnP.Framework.Modernization.Telemetry; using PnP.Framework.Modernization.Transform; using PnP.Framework.Utilities.UnitTests.Model; using PnP.Framework.Utilities.UnitTests.Web; @@ -46,6 +49,38 @@ public void SameSiteCollectionDifferentWebFailsClosed() Assert.AreEqual(PublishingPageTransformationTarget.SameSiteCollectionDifferentWeb, result); } + [TestMethod] + public void InPlaceDifferentSiteCollectionFailsClosedBeforeOverwritePolicy() + { + var result = PublishingPageTransformationValidator.ValidateTarget( + true, + SourceSiteId, + Guid.Parse("2e33ad34-f43a-4d75-af55-e0deba981252"), + SourceWebId, + Guid.Parse("7896708f-f5ea-4f22-aeb3-6223bf8897b0"), + "STS", + true); + + Assert.AreEqual(PublishingPageTransformationTarget.InPlaceDifferentSiteCollection, result); + Assert.IsFalse(PublishingPageTransformationValidator.CanOverwriteTarget(result, true)); + Assert.IsFalse(PublishingPageTransformationValidator.UsesCrossSitePermissionSemantics(result)); + } + + [TestMethod] + public void DefaultSameSiteDifferentWebKeepsExistingSameSiteRejection() + { + var result = PublishingPageTransformationValidator.ValidateTarget( + false, + SourceSiteId, + SourceSiteId, + SourceWebId, + Guid.Parse("7896708f-f5ea-4f22-aeb3-6223bf8897b0"), + "STS", + false); + + Assert.AreEqual(PublishingPageTransformationTarget.SameSiteCollectionNotAllowed, result); + } + [TestMethod] public void SameWebRequiresEnterpriseWikiAndWritableSitePages() { @@ -117,6 +152,136 @@ public void SameWebAssetTransferIsANoOp() } } + [TestMethod] + public void TransformSeamKnownCollisionNeverInvokesAddAndKeepsSourceUnchanged() + { + using (var sourceContext = CreateContext("https://contoso.sharepoint.com/sites/enterprise-wiki")) + using (var targetContext = CreateContext("https://contoso.sharepoint.com/sites/enterprise-wiki")) + { + var information = CreateInPlaceInformation(sourceContext, overwrite: true); + var sourceBefore = SnapshotSource(information.SourcePage); + var sourceHadPendingRequest = sourceContext.HasPendingRequest; + var addCalls = 0; + var transformator = CreateTransformator(sourceContext, targetContext); + + Assert.ThrowsException(() => transformator.CreateTargetPageAfterCollisionProbe( + PublishingPageTransformationTarget.SameWeb, + information, + () => null, + _ => + { + addCalls++; + return null; + }, + out _)); + + Assert.AreEqual(0, addCalls, "AddClientSidePage seam must not run for a known in-place collision."); + AssertSourceUnchanged(sourceBefore, information.SourcePage); + Assert.AreEqual(sourceHadPendingRequest, sourceContext.HasPendingRequest, "The source context request state changed."); + } + } + + [TestMethod] + public void TransformSeamRejectedInPlaceTargetsNeverProbeOrAddAndKeepSourceUnchanged() + { + using (var sourceContext = CreateContext("https://contoso.sharepoint.com/sites/enterprise-wiki")) + using (var targetContext = CreateContext("https://contoso.sharepoint.com/sites/enterprise-wiki")) + { + var information = CreateInPlaceInformation(sourceContext, overwrite: true); + var sourceBefore = SnapshotSource(information.SourcePage); + var sourceHadPendingRequest = sourceContext.HasPendingRequest; + var transformator = CreateTransformator(sourceContext, targetContext); + + foreach (var rejectedTarget in new[] + { + PublishingPageTransformationTarget.InPlaceDifferentSiteCollection, + PublishingPageTransformationTarget.SameSiteCollectionDifferentWeb, + }) + { + var probeCalls = 0; + var addCalls = 0; + + Assert.ThrowsException(() => transformator.CreateTargetPageAfterCollisionProbe( + rejectedTarget, + information, + () => + { + probeCalls++; + return null; + }, + _ => + { + addCalls++; + return null; + }, + out _)); + + Assert.AreEqual(0, probeCalls, $"Target probe ran for rejected target '{rejectedTarget}'."); + Assert.AreEqual(0, addCalls, $"AddClientSidePage seam ran for rejected target '{rejectedTarget}'."); + AssertSourceUnchanged(sourceBefore, information.SourcePage); + Assert.AreEqual(sourceHadPendingRequest, sourceContext.HasPendingRequest, "The source context request state changed."); + } + } + } + + [TestMethod] + public void TransformSeamInconclusiveProbeNeverInvokesAddAndKeepsSourceUnchanged() + { + using (var sourceContext = CreateContext("https://contoso.sharepoint.com/sites/enterprise-wiki")) + using (var targetContext = CreateContext("https://contoso.sharepoint.com/sites/enterprise-wiki")) + { + var information = CreateInPlaceInformation(sourceContext, overwrite: true); + var sourceBefore = SnapshotSource(information.SourcePage); + var sourceHadPendingRequest = sourceContext.HasPendingRequest; + var addCalls = 0; + var transformator = CreateTransformator(sourceContext, targetContext); + + var exception = Assert.ThrowsException(() => transformator.CreateTargetPageAfterCollisionProbe( + PublishingPageTransformationTarget.SameWeb, + information, + () => throw new InvalidOperationException("target probe inconclusive"), + _ => + { + addCalls++; + return null; + }, + out _)); + + Assert.AreEqual("target probe inconclusive", exception.Message); + Assert.AreEqual(0, addCalls, "AddClientSidePage seam must not run after an inconclusive in-place probe."); + AssertSourceUnchanged(sourceBefore, information.SourcePage); + Assert.AreEqual(sourceHadPendingRequest, sourceContext.HasPendingRequest, "The source context request state changed."); + } + } + + [TestMethod] + public void TransformSeamConfirmedMissingTargetInvokesAddOnce() + { + using (var sourceContext = CreateContext("https://contoso.sharepoint.com/sites/enterprise-wiki")) + using (var targetContext = CreateContext("https://contoso.sharepoint.com/sites/enterprise-wiki")) + { + var information = CreateInPlaceInformation(sourceContext, overwrite: false); + var addCalls = 0; + var addedPageName = string.Empty; + var transformator = CreateTransformator(sourceContext, targetContext); + + transformator.CreateTargetPageAfterCollisionProbe( + PublishingPageTransformationTarget.SameWeb, + information, + () => throw new ArgumentException($"{information.TargetPageName} - {LogStrings.TransformPageDoesNotExistInWeb}"), + pageName => + { + addCalls++; + addedPageName = pageName; + return null; + }, + out _); + + Assert.AreEqual(1, addCalls); + Assert.AreEqual("converted/source.aspx", addedPageName); + } + } + private static PublishingPageTransformationTarget ValidateSameWeb( bool inPlacePublishingPage, string targetWebTemplate, @@ -132,6 +297,43 @@ private static PublishingPageTransformationTarget ValidateSameWeb( hasWritableSitePages); } + private static PublishingPageTransformator CreateTransformator(ClientContext sourceContext, ClientContext targetContext) + { + return new PublishingPageTransformator(sourceContext, targetContext, new PageTransformation(), null); + } + + private static PublishingPageTransformationInformation CreateInPlaceInformation(ClientContext sourceContext, bool overwrite) + { + var sourcePage = sourceContext.Web.Lists.GetByTitle("Pages").GetItemById(7); + sourcePage["CCD_SourceIdentity"] = "Pages/7/v3"; + sourcePage["CCD_SourceDigest"] = "sha256:unchanged"; + + return new PublishingPageTransformationInformation(sourcePage, overwrite) + { + InPlacePublishingPage = true, + Folder = "converted/", + TargetPageName = "source.aspx", + }; + } + + private static Dictionary SnapshotSource(ListItem sourcePage) + { + return sourcePage.FieldValues.ToDictionary( + pair => pair.Key, + pair => pair.Value?.ToString()); + } + + private static void AssertSourceUnchanged(Dictionary expected, ListItem actual) + { + var actualSnapshot = SnapshotSource(actual); + CollectionAssert.AreEquivalent(expected.Keys.ToArray(), actualSnapshot.Keys.ToArray()); + + foreach (var pair in expected) + { + Assert.AreEqual(pair.Value, actualSnapshot[pair.Key], $"Source field '{pair.Key}' changed."); + } + } + private static ClientContext CreateContext(string webUrl) { var responseProvider = new MockEntryResponseProvider(); diff --git a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationValidator.cs b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationValidator.cs index 7aca6bda0..74bb9ade2 100644 --- a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationValidator.cs +++ b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationValidator.cs @@ -7,6 +7,7 @@ internal enum PublishingPageTransformationTarget CrossSiteCollection, SameWeb, SameSiteCollectionNotAllowed, + InPlaceDifferentSiteCollection, SameSiteCollectionDifferentWeb, SameWebRequiresEnterpriseWiki, SameWebRequiresWritableSitePages, @@ -28,19 +29,21 @@ internal static PublishingPageTransformationTarget ValidateTarget( string targetWebTemplate, bool hasWritableSitePages) { - if (sourceSiteId != targetSiteId) + if (!inPlacePublishingPage) { - return PublishingPageTransformationTarget.CrossSiteCollection; + return sourceSiteId != targetSiteId + ? PublishingPageTransformationTarget.CrossSiteCollection + : PublishingPageTransformationTarget.SameSiteCollectionNotAllowed; } - if (sourceWebId != targetWebId) + if (sourceSiteId != targetSiteId) { - return PublishingPageTransformationTarget.SameSiteCollectionDifferentWeb; + return PublishingPageTransformationTarget.InPlaceDifferentSiteCollection; } - if (!inPlacePublishingPage) + if (sourceWebId != targetWebId) { - return PublishingPageTransformationTarget.SameSiteCollectionNotAllowed; + return PublishingPageTransformationTarget.SameSiteCollectionDifferentWeb; } if (!string.Equals(targetWebTemplate, EnterpriseWikiWebTemplate, StringComparison.OrdinalIgnoreCase)) diff --git a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs index 71c30b041..6a9a68edd 100644 --- a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs +++ b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs @@ -196,24 +196,12 @@ public string Transform(PublishingPageTransformationInformation publishingPageTr targetClientContext.Web.WebTemplate, hasWritableSitePages); - switch (transformationTarget) - { - case PublishingPageTransformationTarget.SameSiteCollectionNotAllowed: - LogError(LogStrings.Error_SameSiteTransferNoAllowedForPublishingPages, LogStrings.Heading_SharePointConnection); - throw new ArgumentNullException(LogStrings.Error_SameSiteTransferNoAllowedForPublishingPages); - case PublishingPageTransformationTarget.SameSiteCollectionDifferentWeb: - LogError(LogStrings.Error_InPlacePublishingPageDifferentWeb, LogStrings.Heading_SharePointConnection); - throw new ArgumentException(LogStrings.Error_InPlacePublishingPageDifferentWeb); - case PublishingPageTransformationTarget.SameWebRequiresEnterpriseWiki: - LogError(LogStrings.Error_InPlacePublishingPageRequiresEnterpriseWiki, LogStrings.Heading_SharePointConnection); - throw new ArgumentException(LogStrings.Error_InPlacePublishingPageRequiresEnterpriseWiki); - case PublishingPageTransformationTarget.SameWebRequiresWritableSitePages: - LogError(LogStrings.Error_InPlacePublishingPageRequiresWritableSitePages, LogStrings.Heading_SharePointConnection); - throw new ArgumentException(LogStrings.Error_InPlacePublishingPageRequiresWritableSitePages); - case PublishingPageTransformationTarget.SameWeb: - // Use the source context for all writes so existing in-place permission, URL and asset semantics apply. - targetClientContext = sourceClientContext; - break; + EnsureTransformationTargetAllowed(transformationTarget); + + if (transformationTarget == PublishingPageTransformationTarget.SameWeb) + { + // Use the source context for all writes so existing in-place permission, URL and asset semantics apply. + targetClientContext = sourceClientContext; } LogInfo($"{targetClientContext.Web.GetUrl()}", LogStrings.Heading_Summary, LogEntrySignificance.TargetSiteUrl); @@ -340,7 +328,6 @@ public string Transform(PublishingPageTransformationInformation publishingPageTr #if DEBUG && MEASURE Start(); #endif - bool pageExists = false; PnPCore.IPage targetPage = null; List pagesLibrary = null; Microsoft.SharePoint.Client.File existingFile = null; @@ -348,56 +335,17 @@ public string Transform(PublishingPageTransformationInformation publishingPageTr //The determines of the target client context has been specified and use that to generate the target page var context = targetClientContext; - try - { - LogDebug(LogStrings.LoadingExistingPageIfExists, LogStrings.Heading_PageCreation); - - // Just try to load the page in the fastest possible manner, we only want to see if the page exists or not - existingFile = Load(sourceClientContext, targetClientContext, publishingPageTransformationInformation, out pagesLibrary); - pageExists = true; - } - catch (Exception ex) - { - var targetPageDoesNotExist = ex is ArgumentException && - ex.Message.EndsWith(LogStrings.TransformPageDoesNotExistInWeb, StringComparison.InvariantCulture); + targetPage = CreateTargetPageAfterCollisionProbe( + transformationTarget, + publishingPageTransformationInformation, + () => Load(sourceClientContext, targetClientContext, publishingPageTransformationInformation, out pagesLibrary), + pageName => context.Web.AddClientSidePage(pageName), + out existingFile); - if (targetPageDoesNotExist) - { - //Non-critical error generated - LogInfo(LogStrings.CheckPageExistsError, LogStrings.Heading_PageCreation); - } - else if (transformationTarget == PublishingPageTransformationTarget.SameWeb) - { - // The in-place opt-in must fail closed when the target collision probe is inconclusive. - throw; - } - else - { - //Something else occurred - LogError(LogStrings.CheckPageExistsError, LogStrings.Heading_PageCreation, ex); - } - } - #if DEBUG && MEASURE Stop("Load Page"); #endif - if (pageExists) - { - LogInfo(LogStrings.PageAlreadyExistsInTargetLocation, LogStrings.Heading_PageCreation); - - if (!PublishingPageTransformationValidator.CanOverwriteTarget(transformationTarget, publishingPageTransformationInformation.Overwrite)) - { - var message = transformationTarget == PublishingPageTransformationTarget.SameWeb - ? $"{LogStrings.Error_InPlacePublishingPageTargetExists} {publishingPageTransformationInformation.TargetPageName}." - : $"{LogStrings.PageNotOverwriteIfExists} {publishingPageTransformationInformation.TargetPageName}."; - LogError(message, LogStrings.Heading_PageCreation); - throw new ArgumentException(message); - } - } - - // Create the client side page - targetPage = context.Web.AddClientSidePage($"{publishingPageTransformationInformation.Folder}{publishingPageTransformationInformation.TargetPageName}"); LogInfo($"{LogStrings.ModernPageCreated} ", LogStrings.Heading_PageCreation); #endregion @@ -713,6 +661,90 @@ public static string LoadDefaultPageLayoutMappingFile() } #region Helper methods + /// + /// Production seam that keeps the collision probe and the first target-page operation in one fail-closed flow. + /// + internal PnPCore.IPage CreateTargetPageAfterCollisionProbe( + PublishingPageTransformationTarget transformationTarget, + PublishingPageTransformationInformation publishingPageTransformationInformation, + Func targetPageProbe, + Func addClientSidePage, + out Microsoft.SharePoint.Client.File existingFile) + { + // Keep the first target operation fail-closed even if a future caller bypasses the earlier routing guard. + EnsureTransformationTargetAllowed(transformationTarget); + + var pageExists = false; + existingFile = null; + + try + { + LogDebug(LogStrings.LoadingExistingPageIfExists, LogStrings.Heading_PageCreation); + + // A successful probe means the target name is already occupied, even if the caller does not need the file object. + existingFile = targetPageProbe(); + pageExists = true; + } + catch (Exception ex) + { + var targetPageDoesNotExist = ex is ArgumentException && + ex.Message.EndsWith(LogStrings.TransformPageDoesNotExistInWeb, StringComparison.InvariantCulture); + + if (targetPageDoesNotExist) + { + LogInfo(LogStrings.CheckPageExistsError, LogStrings.Heading_PageCreation); + } + else if (transformationTarget == PublishingPageTransformationTarget.SameWeb) + { + // The in-place opt-in must fail closed when the target collision probe is inconclusive. + throw; + } + else + { + // Preserve the existing default cross-site behavior. + LogError(LogStrings.CheckPageExistsError, LogStrings.Heading_PageCreation, ex); + } + } + + if (pageExists) + { + LogInfo(LogStrings.PageAlreadyExistsInTargetLocation, LogStrings.Heading_PageCreation); + + if (!PublishingPageTransformationValidator.CanOverwriteTarget(transformationTarget, publishingPageTransformationInformation.Overwrite)) + { + var message = transformationTarget == PublishingPageTransformationTarget.SameWeb + ? $"{LogStrings.Error_InPlacePublishingPageTargetExists} {publishingPageTransformationInformation.TargetPageName}." + : $"{LogStrings.PageNotOverwriteIfExists} {publishingPageTransformationInformation.TargetPageName}."; + LogError(message, LogStrings.Heading_PageCreation); + throw new ArgumentException(message); + } + } + + return addClientSidePage($"{publishingPageTransformationInformation.Folder}{publishingPageTransformationInformation.TargetPageName}"); + } + + private void EnsureTransformationTargetAllowed(PublishingPageTransformationTarget transformationTarget) + { + switch (transformationTarget) + { + case PublishingPageTransformationTarget.SameSiteCollectionNotAllowed: + LogError(LogStrings.Error_SameSiteTransferNoAllowedForPublishingPages, LogStrings.Heading_SharePointConnection); + throw new ArgumentNullException(LogStrings.Error_SameSiteTransferNoAllowedForPublishingPages); + case PublishingPageTransformationTarget.InPlaceDifferentSiteCollection: + LogError(LogStrings.Error_InPlacePublishingPageDifferentSiteCollection, LogStrings.Heading_SharePointConnection); + throw new ArgumentException(LogStrings.Error_InPlacePublishingPageDifferentSiteCollection); + case PublishingPageTransformationTarget.SameSiteCollectionDifferentWeb: + LogError(LogStrings.Error_InPlacePublishingPageDifferentWeb, LogStrings.Heading_SharePointConnection); + throw new ArgumentException(LogStrings.Error_InPlacePublishingPageDifferentWeb); + case PublishingPageTransformationTarget.SameWebRequiresEnterpriseWiki: + LogError(LogStrings.Error_InPlacePublishingPageRequiresEnterpriseWiki, LogStrings.Heading_SharePointConnection); + throw new ArgumentException(LogStrings.Error_InPlacePublishingPageRequiresEnterpriseWiki); + case PublishingPageTransformationTarget.SameWebRequiresWritableSitePages: + LogError(LogStrings.Error_InPlacePublishingPageRequiresWritableSitePages, LogStrings.Heading_SharePointConnection); + throw new ArgumentException(LogStrings.Error_InPlacePublishingPageRequiresWritableSitePages); + } + } + private bool HasWritableSitePagesLibrary(ClientContext context) { try diff --git a/src/lib/PnP.Framework/Modernization/Telemetry/LogStrings.cs b/src/lib/PnP.Framework/Modernization/Telemetry/LogStrings.cs index 4969acc9b..8ab178b87 100644 --- a/src/lib/PnP.Framework/Modernization/Telemetry/LogStrings.cs +++ b/src/lib/PnP.Framework/Modernization/Telemetry/LogStrings.cs @@ -78,6 +78,7 @@ public static class LogStrings public const string Error_OverridingTagePageTakesSourcePageName = "Overriding 'TargetPageTakesSourcePageName' to ensure that the newly created page in the other site collection gets the same name as the source page"; public const string Error_FallBackToSameSiteTransfer = "Oops, seems source and target point to the same site collection...switch back the 'source only' mode"; public const string Error_SameSiteTransferNoAllowedForPublishingPages = "Oops, seems source and target point to the same site collection...that's a no go for publishing portal page transformation!"; + public const string Error_InPlacePublishingPageDifferentSiteCollection = "In-place publishing page transformation requires source and target to be in the same site collection and Web."; public const string Error_InPlacePublishingPageDifferentWeb = "In-place publishing page transformation requires source and target to be the same Web."; public const string Error_InPlacePublishingPageRequiresEnterpriseWiki = "In-place publishing page transformation is only supported for an Enterprise Wiki Web."; public const string Error_InPlacePublishingPageRequiresWritableSitePages = "In-place publishing page transformation requires an existing Site Pages library with Add Items and Edit Items permissions."; From 3cc9f018ce4bb714f40641846e281712cb53652e Mon Sep 17 00:00:00 2001 From: PnP Framework Lead Date: Mon, 14 Sep 2026 04:36:25 +0000 Subject: [PATCH 03/10] Fix ImageLookup defaults for missing dimensions --- .../Functions/BuiltInImageLookupTests.cs | 273 ++++++++++++++++++ .../Modernization/Functions/BuiltIn.cs | 20 +- 2 files changed, 283 insertions(+), 10 deletions(-) create mode 100644 src/lib/PnP.Framework.Modernization.Test/Transform/Functions/BuiltInImageLookupTests.cs diff --git a/src/lib/PnP.Framework.Modernization.Test/Transform/Functions/BuiltInImageLookupTests.cs b/src/lib/PnP.Framework.Modernization.Test/Transform/Functions/BuiltInImageLookupTests.cs new file mode 100644 index 000000000..b14fd9000 --- /dev/null +++ b/src/lib/PnP.Framework.Modernization.Test/Transform/Functions/BuiltInImageLookupTests.cs @@ -0,0 +1,273 @@ +using Microsoft.SharePoint.Client; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using PnP.Framework.Modernization.Cache; +using PnP.Framework.Modernization.Functions; +using PnP.Framework.Modernization.Transform; +using PnP.Framework.Utilities.UnitTests.Model; +using PnP.Framework.Utilities.UnitTests.Web; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text.Json; +using System.Xml.Linq; + +namespace PnP.Framework.Modernization.Test.Transform.Functions +{ + [TestClass] + public class BuiltInImageLookupTests + { + private const string SiteUrl = "https://contoso.sharepoint.com/sites/modernization"; + private const string WebServerRelativeUrl = "/sites/modernization"; + private const string ImageServerRelativeUrl = "/sites/modernization/SiteAssets/green.svg"; + private static readonly Guid ImageListId = Guid.Parse("f266cd97-f155-4360-b802-47bb9f0d9b55"); + private static readonly Guid ImageUniqueId = Guid.Parse("a9b5b53e-2014-47d9-b26c-fc8d52047d1e"); + + [TestMethod] + public void ImageLookup_WhenBothDimensionsAreMissing_ReturnsNumericDefaults() + { + var result = InvokeImageLookup(false, null, false, null); + + AssertDimensions(result, "-1", "-1"); + } + + [TestMethod] + public void ImageLookup_WhenOnlyWidthIsMissing_ReturnsWidthDefault() + { + var result = InvokeImageLookup(false, null, true, "360"); + + AssertDimensions(result, "-1", "360"); + } + + [TestMethod] + public void ImageLookup_WhenOnlyHeightIsMissing_ReturnsHeightDefault() + { + var result = InvokeImageLookup(true, "960", false, null); + + AssertDimensions(result, "960", "-1"); + } + + [TestMethod] + public void ImageLookup_WhenDimensionsAreEmptyOrZero_ReturnsNumericDefaults() + { + var result = InvokeImageLookup(true, string.Empty, true, "0"); + + AssertDimensions(result, "-1", "-1"); + } + + [TestMethod] + public void ImageLookup_WhenDimensionsAreNull_ReturnsNumericDefaults() + { + var result = InvokeImageLookup(true, null, true, null); + + AssertDimensions(result, "-1", "-1"); + } + + [TestMethod] + public void ImageLookup_WhenDimensionsAreValid_ReturnsPropertyValues() + { + var result = InvokeImageLookup(true, "960", true, 360); + + AssertDimensions(result, "960", "360"); + } + + [TestMethod] + public void ImageLookup_WhenLookupIsSkipped_PreservesExistingDefaults() + { + var provider = CreateProvider(false, null, false, null); + using (var context = CreateContext(provider)) + { + var result = CreateBuiltIn(context).ImageLookup(null); + + Assert.IsNotNull(result); + Assert.AreEqual("-1", result["ImageWidth"]); + Assert.AreEqual("-1", result["ImageHeight"]); + Assert.AreEqual(string.Empty, result["ImageListId"]); + Assert.AreEqual(string.Empty, result["ImageUniqueId"]); + } + } + + [TestMethod] + public void ImageLookup_WhenFileIsNotFound_PreservesNullResult() + { + using (var context = CreateContext(CreateProvider(false, null, false, null))) + { + var builtIn = CreateBuiltIn(context); + context.Load(context.Web, web => web.ServerRelativeUrl); + context.ExecuteQuery(); + context.WebRequestExecutorFactory = new MockWebRequestExecutorFactory(new StaticResponseProvider( + "[{\"SchemaVersion\":\"15.0.0.0\",\"LibraryVersion\":\"16.0.0.0\",\"ErrorInfo\":{\"ErrorMessage\":\"File Not Found.\",\"ErrorValue\":null,\"ErrorCode\":-2147024894,\"ErrorTypeName\":\"System.IO.FileNotFoundException\"},\"TraceCorrelationId\":\"d991e178-2b53-4ff8-b77f-6bb73516962a\"}]")); + + var result = builtIn.ImageLookup(ImageServerRelativeUrl); + + Assert.IsNull(result); + } + } + + [TestMethod] + public void WikiImageMapping_WithDefaultDimensions_ProducesNumericJsonProperties() + { + AssertMappingWithDefaultDimensions("SharePointPnP.Modernization.WikiImagePart"); + } + + [TestMethod] + public void ClassicImageWebPartMapping_WithDefaultDimensions_ProducesNumericJsonProperties() + { + AssertMappingWithDefaultDimensions("Microsoft.SharePoint.WebPartPages.ImageWebPart, Microsoft.SharePoint, Version=16.0.0.0, Culture=neutral, PublicKeyToken=71e9bce111e9429c"); + } + + private static void AssertMappingWithDefaultDimensions(string webPartType) + { + var lookupResult = InvokeImageLookup(false, null, false, null); + var mapping = LoadImageJsonControlData(webPartType); + var replacements = new Dictionary + { + ["{Caption}"] = string.Empty, + ["{ServerRelativeFileName}"] = ImageServerRelativeUrl, + ["{serverRelativeFileName}"] = ImageServerRelativeUrl, + ["{Anchor}"] = string.Empty, + ["{AlternativeText}"] = "Green image", + ["{FileName}"] = "green.svg", + ["{SiteId}"] = "c6815669-f27c-4ff6-b937-0825b2cdfa93", + ["{webId}"] = "9d03e0aa-cb94-4463-b885-15a55d03ed52", + ["{WebId}"] = "9d03e0aa-cb94-4463-b885-15a55d03ed52", + ["{ImageListId}"] = lookupResult["ImageListId"], + ["{ImageUniqueId}"] = lookupResult["ImageUniqueId"], + ["{ImageWidth}"] = lookupResult["ImageWidth"], + ["{ImageHeight}"] = lookupResult["ImageHeight"] + }; + + foreach (var replacement in replacements) + { + mapping = mapping.Replace(replacement.Key, replacement.Value); + } + + using (var document = JsonDocument.Parse(mapping)) + { + var properties = document.RootElement.GetProperty("properties"); + Assert.AreEqual(JsonValueKind.Number, properties.GetProperty("imgWidth").ValueKind); + Assert.AreEqual(JsonValueKind.Number, properties.GetProperty("imgHeight").ValueKind); + Assert.AreEqual(-1, properties.GetProperty("imgWidth").GetInt32()); + Assert.AreEqual(-1, properties.GetProperty("imgHeight").GetInt32()); + } + } + + private static Dictionary InvokeImageLookup(bool includeWidth, object width, bool includeHeight, object height) + { + using (var context = CreateContext(CreateProvider(includeWidth, width, includeHeight, height))) + { + return CreateBuiltIn(context).ImageLookup(ImageServerRelativeUrl); + } + } + + private static ClientContext CreateContext(IMockResponseProvider provider) + { + CacheManager.Instance.SetSharePointVersion(new Uri(SiteUrl), SPVersion.SPO); + var context = new ClientContext(SiteUrl); + context.WebRequestExecutorFactory = new MockWebRequestExecutorFactory(provider); + return context; + } + + private static BuiltIn CreateBuiltIn(ClientContext context) + { + return new BuiltIn(new PageTransformationInformation(null) + { + SkipTelemetry = true + }, context); + } + + private static MockEntryResponseProvider CreateProvider(bool includeWidth, object width, bool includeHeight, object height) + { + var properties = new Dictionary + { + ["_ObjectType_"] = "SP.PropertyValues" + }; + if (includeWidth) + { + properties["vti_lastwidth"] = width; + } + if (includeHeight) + { + properties["vti_lastheight"] = height; + } + + var provider = new MockEntryResponseProvider(); + provider.ResponseEntries.Add(new MockResponseEntry + { + Url = SiteUrl, + PropertyName = "Site", + ReturnValue = new Dictionary + { + ["_ObjectType_"] = "SP.Site", + ["ServerRelativeUrl"] = WebServerRelativeUrl, + ["Url"] = SiteUrl + } + }); + provider.ResponseEntries.Add(new MockResponseEntry + { + Url = SiteUrl, + PropertyName = "Web", + ReturnValue = new Dictionary + { + ["_ObjectType_"] = "SP.Web", + ["ServerRelativeUrl"] = WebServerRelativeUrl, + ["Url"] = SiteUrl + } + }); + provider.ResponseEntries.Add(new MockResponseEntry + { + Url = SiteUrl, + Method = "GetFileByServerRelativeUrl", + ReturnValue = new Dictionary + { + ["_ObjectType_"] = "SP.File", + ["ListId"] = $"/Guid({ImageListId})/", + ["UniqueId"] = $"/Guid({ImageUniqueId})/", + ["Properties"] = properties + } + }); + return provider; + } + + private static string LoadImageJsonControlData(string webPartType) + { + var assembly = typeof(BuiltIn).Assembly; + var resourceName = assembly.GetManifestResourceNames() + .Single(name => name.EndsWith("Modernization.webpartmapping.xml", StringComparison.Ordinal)); + using (var stream = assembly.GetManifestResourceStream(resourceName)) + { + var document = XDocument.Load(stream); + var schemaNamespace = document.Root.Name.Namespace; + return document.Descendants(schemaNamespace + "WebPart") + .Single(element => (string)element.Attribute("Type") == webPartType) + .Descendants(schemaNamespace + "ClientSideWebPart") + .Single() + .Attribute("JsonControlData") + .Value; + } + } + + private static void AssertDimensions(Dictionary result, string expectedWidth, string expectedHeight) + { + Assert.IsNotNull(result); + Assert.AreEqual(ImageListId.ToString(), result["ImageListId"]); + Assert.AreEqual(ImageUniqueId.ToString(), result["ImageUniqueId"]); + Assert.AreEqual(expectedWidth, result["ImageWidth"]); + Assert.AreEqual(expectedHeight, result["ImageHeight"]); + } + + private sealed class StaticResponseProvider : IMockResponseProvider + { + private readonly string response; + + public StaticResponseProvider(string response) + { + this.response = response; + } + + public string GetResponse(string url, string verb, string body) + { + return response; + } + } + } +} diff --git a/src/lib/PnP.Framework/Modernization/Functions/BuiltIn.cs b/src/lib/PnP.Framework/Modernization/Functions/BuiltIn.cs index d6b0e0957..81e5c45d7 100644 --- a/src/lib/PnP.Framework/Modernization/Functions/BuiltIn.cs +++ b/src/lib/PnP.Framework/Modernization/Functions/BuiltIn.cs @@ -691,24 +691,24 @@ public Dictionary ImageLookup(string serverRelativeImagePath) results.Add("ImageListId", pageHeaderImage.ListId.ToString()); results.Add("ImageUniqueId", pageHeaderImage.UniqueId.ToString()); + results.Add("ImageHeight", "-1"); + results.Add("ImageWidth", "-1"); - if (pageHeaderImage.Properties.FieldValues.ContainsKey("vti_lastheight")) + if (pageHeaderImage.Properties.FieldValues.TryGetValue("vti_lastheight", out var heightValue)) { - var height = pageHeaderImage.Properties.FieldValues["vti_lastheight"].ToString(); - if (string.IsNullOrEmpty(height) || height == "0") + var height = heightValue?.ToString(); + if (!string.IsNullOrEmpty(height) && height != "0") { - height = "-1"; + results["ImageHeight"] = height; } - results.Add("ImageHeight", height); } - if (pageHeaderImage.Properties.FieldValues.ContainsKey("vti_lastwidth")) + if (pageHeaderImage.Properties.FieldValues.TryGetValue("vti_lastwidth", out var widthValue)) { - var width = pageHeaderImage.Properties.FieldValues["vti_lastwidth"].ToString(); - if (string.IsNullOrEmpty(width) || width == "0") + var width = widthValue?.ToString(); + if (!string.IsNullOrEmpty(width) && width != "0") { - width = "-1"; + results["ImageWidth"] = width; } - results.Add("ImageWidth", width); } From c417af5f6e41ed3596315907aaf631a2a4f64f07 Mon Sep 17 00:00:00 2001 From: TaoziZ03 <106975749+TaoziZ03@users.noreply.github.com> Date: Tue, 15 Sep 2026 03:56:29 +0000 Subject: [PATCH 04/10] Refine in-place publishing validation --- ...ublishingPageInPlaceTransformationTests.cs | 19 ++----- .../Modernization/Functions/BuiltIn.cs | 4 ++ ...PublishingPageTransformationInformation.cs | 2 +- .../PublishingPageTransformationValidator.cs | 9 ---- .../Publishing/PublishingPageTransformator.cs | 50 ++++++++++++++----- .../Modernization/Telemetry/LogStrings.cs | 6 ++- 6 files changed, 52 insertions(+), 38 deletions(-) diff --git a/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs b/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs index c37fba580..3c8d514d9 100644 --- a/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs +++ b/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs @@ -29,7 +29,7 @@ public void InPlacePublishingPageDefaultsToFalse() [TestMethod] public void SameWebRequiresExplicitOptIn() { - var result = ValidateSameWeb(inPlacePublishingPage: false, targetWebTemplate: "ENTERWIKI", hasWritableSitePages: true); + var result = ValidateSameWeb(inPlacePublishingPage: false, hasWritableSitePages: true); Assert.AreEqual(PublishingPageTransformationTarget.SameSiteCollectionNotAllowed, result); } @@ -43,7 +43,6 @@ public void SameSiteCollectionDifferentWebFailsClosed() SourceSiteId, SourceWebId, Guid.Parse("7896708f-f5ea-4f22-aeb3-6223bf8897b0"), - "ENTERWIKI", true); Assert.AreEqual(PublishingPageTransformationTarget.SameSiteCollectionDifferentWeb, result); @@ -58,7 +57,6 @@ public void InPlaceDifferentSiteCollectionFailsClosedBeforeOverwritePolicy() Guid.Parse("2e33ad34-f43a-4d75-af55-e0deba981252"), SourceWebId, Guid.Parse("7896708f-f5ea-4f22-aeb3-6223bf8897b0"), - "STS", true); Assert.AreEqual(PublishingPageTransformationTarget.InPlaceDifferentSiteCollection, result); @@ -75,24 +73,20 @@ public void DefaultSameSiteDifferentWebKeepsExistingSameSiteRejection() SourceSiteId, SourceWebId, Guid.Parse("7896708f-f5ea-4f22-aeb3-6223bf8897b0"), - "STS", false); Assert.AreEqual(PublishingPageTransformationTarget.SameSiteCollectionNotAllowed, result); } [TestMethod] - public void SameWebRequiresEnterpriseWikiAndWritableSitePages() + public void SameWebSupportsGeneralPublishingPagesAndRequiresWritableSitePages() { - Assert.AreEqual( - PublishingPageTransformationTarget.SameWebRequiresEnterpriseWiki, - ValidateSameWeb(true, "STS", true)); Assert.AreEqual( PublishingPageTransformationTarget.SameWebRequiresWritableSitePages, - ValidateSameWeb(true, "ENTERWIKI", false)); + ValidateSameWeb(true, false)); Assert.AreEqual( PublishingPageTransformationTarget.SameWeb, - ValidateSameWeb(true, "enterwiki", true)); + ValidateSameWeb(true, true)); } [TestMethod] @@ -104,7 +98,6 @@ public void CrossSiteCollectionKeepsExistingTargetPolicyAndPermissionSemantics() Guid.Parse("2e33ad34-f43a-4d75-af55-e0deba981252"), SourceWebId, Guid.Empty, - "UNSUPPORTED", false); Assert.AreEqual(PublishingPageTransformationTarget.CrossSiteCollection, result); @@ -115,7 +108,7 @@ public void CrossSiteCollectionKeepsExistingTargetPolicyAndPermissionSemantics() [TestMethod] public void SameWebNeverOverwritesAndUsesInPlacePermissionSemantics() { - var result = ValidateSameWeb(true, "ENTERWIKI", true); + var result = ValidateSameWeb(true, true); Assert.IsFalse(PublishingPageTransformationValidator.CanOverwriteTarget(result, true)); Assert.IsFalse(PublishingPageTransformationValidator.UsesCrossSitePermissionSemantics(result)); @@ -284,7 +277,6 @@ public void TransformSeamConfirmedMissingTargetInvokesAddOnce() private static PublishingPageTransformationTarget ValidateSameWeb( bool inPlacePublishingPage, - string targetWebTemplate, bool hasWritableSitePages) { return PublishingPageTransformationValidator.ValidateTarget( @@ -293,7 +285,6 @@ private static PublishingPageTransformationTarget ValidateSameWeb( SourceSiteId, SourceWebId, SourceWebId, - targetWebTemplate, hasWritableSitePages); } diff --git a/src/lib/PnP.Framework/Modernization/Functions/BuiltIn.cs b/src/lib/PnP.Framework/Modernization/Functions/BuiltIn.cs index 81e5c45d7..56db75b9d 100644 --- a/src/lib/PnP.Framework/Modernization/Functions/BuiltIn.cs +++ b/src/lib/PnP.Framework/Modernization/Functions/BuiltIn.cs @@ -691,6 +691,10 @@ public Dictionary ImageLookup(string serverRelativeImagePath) results.Add("ImageListId", pageHeaderImage.ListId.ToString()); results.Add("ImageUniqueId", pageHeaderImage.UniqueId.ToString()); + + // Pages and image files created entirely through programmatic flows can bypass the + // post-save processing that populates vti_lastheight and vti_lastwidth. Treat those + // dimensions as unknown so transformation can continue for these customer edge cases. results.Add("ImageHeight", "-1"); results.Add("ImageWidth", "-1"); diff --git a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationInformation.cs b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationInformation.cs index 579ab87d0..102c1688e 100644 --- a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationInformation.cs +++ b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationInformation.cs @@ -97,7 +97,7 @@ public virtual bool IsFieldUsed(string fieldName) #region Page Properties /// - /// Explicitly allows an Enterprise Wiki publishing page to be transformed to Site Pages in the same Web. + /// Explicitly allows a publishing page to be transformed to Site Pages in the same Web. /// The source publishing page is retained and an existing target page is never overwritten. /// public bool InPlacePublishingPage { get; set; } diff --git a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationValidator.cs b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationValidator.cs index 74bb9ade2..77fc90fbd 100644 --- a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationValidator.cs +++ b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationValidator.cs @@ -9,7 +9,6 @@ internal enum PublishingPageTransformationTarget SameSiteCollectionNotAllowed, InPlaceDifferentSiteCollection, SameSiteCollectionDifferentWeb, - SameWebRequiresEnterpriseWiki, SameWebRequiresWritableSitePages, } @@ -18,15 +17,12 @@ internal enum PublishingPageTransformationTarget /// internal static class PublishingPageTransformationValidator { - private const string EnterpriseWikiWebTemplate = "ENTERWIKI"; - internal static PublishingPageTransformationTarget ValidateTarget( bool inPlacePublishingPage, Guid sourceSiteId, Guid targetSiteId, Guid sourceWebId, Guid targetWebId, - string targetWebTemplate, bool hasWritableSitePages) { if (!inPlacePublishingPage) @@ -46,11 +42,6 @@ internal static PublishingPageTransformationTarget ValidateTarget( return PublishingPageTransformationTarget.SameSiteCollectionDifferentWeb; } - if (!string.Equals(targetWebTemplate, EnterpriseWikiWebTemplate, StringComparison.OrdinalIgnoreCase)) - { - return PublishingPageTransformationTarget.SameWebRequiresEnterpriseWiki; - } - if (!hasWritableSitePages) { return PublishingPageTransformationTarget.SameWebRequiresWritableSitePages; diff --git a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs index 6a9a68edd..f4261a512 100644 --- a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs +++ b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs @@ -181,8 +181,7 @@ public string Transform(PublishingPageTransformationInformation publishingPageTr if (sameSiteCollection && sourceWebId.Equals(targetClientContext.Web.Id) && - publishingPageTransformationInformation.InPlacePublishingPage && - string.Equals(targetClientContext.Web.WebTemplate, "ENTERWIKI", StringComparison.InvariantCultureIgnoreCase)) + publishingPageTransformationInformation.InPlacePublishingPage) { hasWritableSitePages = HasWritableSitePagesLibrary(sourceClientContext); } @@ -193,7 +192,6 @@ public string Transform(PublishingPageTransformationInformation publishingPageTr targetClientContext.Site.Id, sourceWebId, targetClientContext.Web.Id, - targetClientContext.Web.WebTemplate, hasWritableSitePages); EnsureTransformationTargetAllowed(transformationTarget); @@ -736,9 +734,6 @@ private void EnsureTransformationTargetAllowed(PublishingPageTransformationTarge case PublishingPageTransformationTarget.SameSiteCollectionDifferentWeb: LogError(LogStrings.Error_InPlacePublishingPageDifferentWeb, LogStrings.Heading_SharePointConnection); throw new ArgumentException(LogStrings.Error_InPlacePublishingPageDifferentWeb); - case PublishingPageTransformationTarget.SameWebRequiresEnterpriseWiki: - LogError(LogStrings.Error_InPlacePublishingPageRequiresEnterpriseWiki, LogStrings.Heading_SharePointConnection); - throw new ArgumentException(LogStrings.Error_InPlacePublishingPageRequiresEnterpriseWiki); case PublishingPageTransformationTarget.SameWebRequiresWritableSitePages: LogError(LogStrings.Error_InPlacePublishingPageRequiresWritableSitePages, LogStrings.Heading_SharePointConnection); throw new ArgumentException(LogStrings.Error_InPlacePublishingPageRequiresWritableSitePages); @@ -747,9 +742,10 @@ private void EnsureTransformationTargetAllowed(PublishingPageTransformationTarge private bool HasWritableSitePagesLibrary(ClientContext context) { + var sitePagesServerRelativeUrl = UrlUtility.Combine(context.Web.ServerRelativeUrl, "SitePages"); + try { - var sitePagesServerRelativeUrl = UrlUtility.Combine(context.Web.ServerRelativeUrl, "SitePages"); var sitePagesLibrary = context.Web.GetList(sitePagesServerRelativeUrl); context.Load(sitePagesLibrary, p => p.Id, @@ -757,16 +753,44 @@ private bool HasWritableSitePagesLibrary(ClientContext context) p => p.EffectiveBasePermissions); context.ExecuteQueryRetry(); - return sitePagesLibrary.Id != Guid.Empty && - sitePagesLibrary.BaseTemplate == (int)ListTemplateType.WebPageLibrary && - sitePagesLibrary.EffectiveBasePermissions.Has(PermissionKind.AddListItems) && - sitePagesLibrary.EffectiveBasePermissions.Has(PermissionKind.EditListItems); + if (sitePagesLibrary.Id == Guid.Empty) + { + LogError($"{LogStrings.Error_InPlacePublishingPageSitePagesNotFound} Path: {sitePagesServerRelativeUrl}.", LogStrings.Heading_SharePointConnection); + return false; + } + + if (sitePagesLibrary.BaseTemplate != (int)ListTemplateType.WebPageLibrary) + { + LogError($"{LogStrings.Error_InPlacePublishingPageSitePagesInvalidTemplate} Path: {sitePagesServerRelativeUrl}; BaseTemplate: {sitePagesLibrary.BaseTemplate}.", LogStrings.Heading_SharePointConnection); + return false; + } + + var canAdd = sitePagesLibrary.EffectiveBasePermissions.Has(PermissionKind.AddListItems); + var canEdit = sitePagesLibrary.EffectiveBasePermissions.Has(PermissionKind.EditListItems); + if (!canAdd || !canEdit) + { + LogError($"{LogStrings.Error_InPlacePublishingPageSitePagesInsufficientPermissions} Path: {sitePagesServerRelativeUrl}; AddListItems: {canAdd}; EditListItems: {canEdit}.", LogStrings.Heading_SharePointConnection); + return false; + } + + return true; } - catch (Exception ex) + catch (ServerUnauthorizedAccessException ex) + { + LogError($"{LogStrings.Error_InPlacePublishingPageSitePagesProbeUnauthorized} Path: {sitePagesServerRelativeUrl}; {ex.Message}", LogStrings.Heading_SharePointConnection); + return false; + } + catch (ServerException ex) when (string.Equals(ex.ServerErrorTypeName, "System.IO.FileNotFoundException", StringComparison.Ordinal)) { - LogWarning($"{LogStrings.Error_InPlacePublishingPageRequiresWritableSitePages} {ex.Message}", LogStrings.Heading_SharePointConnection); + LogError($"{LogStrings.Error_InPlacePublishingPageSitePagesNotFound} Path: {sitePagesServerRelativeUrl}; {ex.Message}", LogStrings.Heading_SharePointConnection); return false; } + catch (Exception ex) + { + var message = $"{LogStrings.Error_InPlacePublishingPageSitePagesProbeFailed} Path: {sitePagesServerRelativeUrl}; {ex.Message}"; + LogError(message, LogStrings.Heading_SharePointConnection); + throw new InvalidOperationException(message, ex); + } } private void SetPageTitle(PublishingPageTransformationInformation publishingPageTransformationInformation, PnPCore.IPage targetPage) diff --git a/src/lib/PnP.Framework/Modernization/Telemetry/LogStrings.cs b/src/lib/PnP.Framework/Modernization/Telemetry/LogStrings.cs index 8ab178b87..67725ff11 100644 --- a/src/lib/PnP.Framework/Modernization/Telemetry/LogStrings.cs +++ b/src/lib/PnP.Framework/Modernization/Telemetry/LogStrings.cs @@ -80,8 +80,12 @@ public static class LogStrings public const string Error_SameSiteTransferNoAllowedForPublishingPages = "Oops, seems source and target point to the same site collection...that's a no go for publishing portal page transformation!"; public const string Error_InPlacePublishingPageDifferentSiteCollection = "In-place publishing page transformation requires source and target to be in the same site collection and Web."; public const string Error_InPlacePublishingPageDifferentWeb = "In-place publishing page transformation requires source and target to be the same Web."; - public const string Error_InPlacePublishingPageRequiresEnterpriseWiki = "In-place publishing page transformation is only supported for an Enterprise Wiki Web."; public const string Error_InPlacePublishingPageRequiresWritableSitePages = "In-place publishing page transformation requires an existing Site Pages library with Add Items and Edit Items permissions."; + public const string Error_InPlacePublishingPageSitePagesNotFound = "The Site Pages library required for in-place publishing page transformation was not found."; + public const string Error_InPlacePublishingPageSitePagesInvalidTemplate = "The Site Pages path does not resolve to a Web Page Library."; + public const string Error_InPlacePublishingPageSitePagesInsufficientPermissions = "The current user cannot add and edit items in the Site Pages library."; + public const string Error_InPlacePublishingPageSitePagesProbeUnauthorized = "The Site Pages capability probe was denied."; + public const string Error_InPlacePublishingPageSitePagesProbeFailed = "The Site Pages capability probe could not determine whether the library is writable; transformation was stopped without creating a target page."; public const string Error_InPlacePublishingPageTargetExists = "In-place publishing page transformation cannot overwrite an existing target page."; public const string Error_CrossSiteTransferTargetsNonModernSite = "Page transformation for targeting non-modern sites is currently not supported."; public const string Error_GetVersionError = "Setting version stamp error"; From 2fe4aa57f8870cd60bf98a99cdc99a5578016533 Mon Sep 17 00:00:00 2001 From: titao Date: Sat, 10 Oct 2026 16:17:53 +0800 Subject: [PATCH 05/10] Infer publishing transformation mode from target context --- ...ublishingPageInPlaceTransformationTests.cs | 86 +++++++++++-------- ...PublishingPageTransformationInformation.cs | 6 -- .../PublishingPageTransformationValidator.cs | 12 +-- .../Publishing/PublishingPageTransformator.cs | 17 +--- .../Modernization/Telemetry/LogStrings.cs | 3 +- 5 files changed, 56 insertions(+), 68 deletions(-) diff --git a/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs b/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs index 3c8d514d9..96b6a785a 100644 --- a/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs +++ b/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.IO; using System.Linq; using Microsoft.SharePoint.Client; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -18,27 +19,55 @@ public class PublishingPageInPlaceTransformationTests private static readonly Guid SourceSiteId = Guid.Parse("c33f44c0-e1ce-4d87-a961-98669e76bca6"); private static readonly Guid SourceWebId = Guid.Parse("a467c0f4-dcf4-49f3-8a08-e33de32d3202"); + public TestContext TestContext { get; set; } + + [TestMethod] + public void ModelConstructorWithoutTargetUsesSourceContext() + { + using (var sourceContext = CreateContext("https://contoso.sharepoint.com/sites/enterprise-wiki")) + { + var transformator = CreateTransformator(sourceContext, null); + + Assert.AreSame(sourceContext, transformator.sourceClientContext); + Assert.AreSame(sourceContext, transformator.targetClientContext); + } + } + [TestMethod] - public void InPlacePublishingPageDefaultsToFalse() + public void FileMappingConstructorWithoutTargetUsesSourceContext() { - var information = new PublishingPageTransformationInformation(null); + Directory.CreateDirectory(TestContext.TestResultsDirectory); + var mappingPath = Path.Combine(TestContext.TestResultsDirectory, $"webpartmapping-{Guid.NewGuid():N}.xml"); + System.IO.File.WriteAllText(mappingPath, BasePageTransformator.LoadDefaultWebPartMappingFile()); - Assert.IsFalse(information.InPlacePublishingPage); + try + { + using (var sourceContext = CreateContext("https://contoso.sharepoint.com/sites/enterprise-wiki")) + { + var transformator = new PublishingPageTransformator(sourceContext, null, mappingPath, null); + + Assert.AreSame(sourceContext, transformator.sourceClientContext); + Assert.AreSame(sourceContext, transformator.targetClientContext); + } + } + finally + { + System.IO.File.Delete(mappingPath); + } } [TestMethod] - public void SameWebRequiresExplicitOptIn() + public void SameWebTargetIsAcceptedWithoutAdditionalOptions() { - var result = ValidateSameWeb(inPlacePublishingPage: false, hasWritableSitePages: true); + var result = ValidateSameWeb(hasWritableSitePages: true); - Assert.AreEqual(PublishingPageTransformationTarget.SameSiteCollectionNotAllowed, result); + Assert.AreEqual(PublishingPageTransformationTarget.SameWeb, result); } [TestMethod] public void SameSiteCollectionDifferentWebFailsClosed() { var result = PublishingPageTransformationValidator.ValidateTarget( - true, SourceSiteId, SourceSiteId, SourceWebId, @@ -49,33 +78,22 @@ public void SameSiteCollectionDifferentWebFailsClosed() } [TestMethod] - public void InPlaceDifferentSiteCollectionFailsClosedBeforeOverwritePolicy() + public void ConstructorRetainsAnExplicitTargetContext() { - var result = PublishingPageTransformationValidator.ValidateTarget( - true, - SourceSiteId, - Guid.Parse("2e33ad34-f43a-4d75-af55-e0deba981252"), - SourceWebId, - Guid.Parse("7896708f-f5ea-4f22-aeb3-6223bf8897b0"), - true); + using (var sourceContext = CreateContext("https://contoso.sharepoint.com/sites/enterprise-wiki")) + using (var targetContext = CreateContext("https://contoso.sharepoint.com/sites/modern-target")) + { + var transformator = CreateTransformator(sourceContext, targetContext); - Assert.AreEqual(PublishingPageTransformationTarget.InPlaceDifferentSiteCollection, result); - Assert.IsFalse(PublishingPageTransformationValidator.CanOverwriteTarget(result, true)); - Assert.IsFalse(PublishingPageTransformationValidator.UsesCrossSitePermissionSemantics(result)); + Assert.AreSame(sourceContext, transformator.sourceClientContext); + Assert.AreSame(targetContext, transformator.targetClientContext); + } } [TestMethod] - public void DefaultSameSiteDifferentWebKeepsExistingSameSiteRejection() + public void ConstructorStillRequiresSourceContext() { - var result = PublishingPageTransformationValidator.ValidateTarget( - false, - SourceSiteId, - SourceSiteId, - SourceWebId, - Guid.Parse("7896708f-f5ea-4f22-aeb3-6223bf8897b0"), - false); - - Assert.AreEqual(PublishingPageTransformationTarget.SameSiteCollectionNotAllowed, result); + Assert.ThrowsException(() => CreateTransformator(null, null)); } [TestMethod] @@ -83,17 +101,16 @@ public void SameWebSupportsGeneralPublishingPagesAndRequiresWritableSitePages() { Assert.AreEqual( PublishingPageTransformationTarget.SameWebRequiresWritableSitePages, - ValidateSameWeb(true, false)); + ValidateSameWeb(false)); Assert.AreEqual( PublishingPageTransformationTarget.SameWeb, - ValidateSameWeb(true, true)); + ValidateSameWeb(true)); } [TestMethod] public void CrossSiteCollectionKeepsExistingTargetPolicyAndPermissionSemantics() { var result = PublishingPageTransformationValidator.ValidateTarget( - false, SourceSiteId, Guid.Parse("2e33ad34-f43a-4d75-af55-e0deba981252"), SourceWebId, @@ -108,7 +125,7 @@ public void CrossSiteCollectionKeepsExistingTargetPolicyAndPermissionSemantics() [TestMethod] public void SameWebNeverOverwritesAndUsesInPlacePermissionSemantics() { - var result = ValidateSameWeb(true, true); + var result = ValidateSameWeb(true); Assert.IsFalse(PublishingPageTransformationValidator.CanOverwriteTarget(result, true)); Assert.IsFalse(PublishingPageTransformationValidator.UsesCrossSitePermissionSemantics(result)); @@ -187,8 +204,8 @@ public void TransformSeamRejectedInPlaceTargetsNeverProbeOrAddAndKeepSourceUncha foreach (var rejectedTarget in new[] { - PublishingPageTransformationTarget.InPlaceDifferentSiteCollection, PublishingPageTransformationTarget.SameSiteCollectionDifferentWeb, + PublishingPageTransformationTarget.SameWebRequiresWritableSitePages, }) { var probeCalls = 0; @@ -276,11 +293,9 @@ public void TransformSeamConfirmedMissingTargetInvokesAddOnce() } private static PublishingPageTransformationTarget ValidateSameWeb( - bool inPlacePublishingPage, bool hasWritableSitePages) { return PublishingPageTransformationValidator.ValidateTarget( - inPlacePublishingPage, SourceSiteId, SourceSiteId, SourceWebId, @@ -301,7 +316,6 @@ private static PublishingPageTransformationInformation CreateInPlaceInformation( return new PublishingPageTransformationInformation(sourcePage, overwrite) { - InPlacePublishingPage = true, Folder = "converted/", TargetPageName = "source.aspx", }; diff --git a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationInformation.cs b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationInformation.cs index 102c1688e..c5b306da2 100644 --- a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationInformation.cs +++ b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationInformation.cs @@ -47,7 +47,6 @@ public PublishingPageTransformationInformation(ListItem sourcePage, bool overwri DisablePageComments = false; SkipUserMapping = false; TargetPageFolderOverridesDefaultFolder = false; - InPlacePublishingPage = false; // Populate with OOB mapping properties MappingProperties = new Dictionary(5) { @@ -96,11 +95,6 @@ public virtual bool IsFieldUsed(string fieldName) #endregion #region Page Properties - /// - /// Explicitly allows a publishing page to be transformed to Site Pages in the same Web. - /// The source publishing page is retained and an existing target page is never overwritten. - /// - public bool InPlacePublishingPage { get; set; } #endregion } } diff --git a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationValidator.cs b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationValidator.cs index 77fc90fbd..d2ec57bcb 100644 --- a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationValidator.cs +++ b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformationValidator.cs @@ -6,8 +6,6 @@ internal enum PublishingPageTransformationTarget { CrossSiteCollection, SameWeb, - SameSiteCollectionNotAllowed, - InPlaceDifferentSiteCollection, SameSiteCollectionDifferentWeb, SameWebRequiresWritableSitePages, } @@ -18,23 +16,15 @@ internal enum PublishingPageTransformationTarget internal static class PublishingPageTransformationValidator { internal static PublishingPageTransformationTarget ValidateTarget( - bool inPlacePublishingPage, Guid sourceSiteId, Guid targetSiteId, Guid sourceWebId, Guid targetWebId, bool hasWritableSitePages) { - if (!inPlacePublishingPage) - { - return sourceSiteId != targetSiteId - ? PublishingPageTransformationTarget.CrossSiteCollection - : PublishingPageTransformationTarget.SameSiteCollectionNotAllowed; - } - if (sourceSiteId != targetSiteId) { - return PublishingPageTransformationTarget.InPlaceDifferentSiteCollection; + return PublishingPageTransformationTarget.CrossSiteCollection; } if (sourceWebId != targetWebId) diff --git a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs index f4261a512..d06b21f21 100644 --- a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs +++ b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs @@ -40,7 +40,7 @@ public PublishingPageTransformator(ClientContext sourceClientContext, ClientCont #endif this.sourceClientContext = sourceClientContext ?? throw new ArgumentException("sourceClientContext must be provided."); - this.targetClientContext = targetClientContext ?? throw new ArgumentException("targetClientContext must be provided."); ; + this.targetClientContext = targetClientContext ?? this.sourceClientContext; this.version = GetVersion(); this.pageTelemetry = new PageTelemetry(version); @@ -79,7 +79,7 @@ public PublishingPageTransformator(ClientContext sourceClientContext, ClientCont #endif this.sourceClientContext = sourceClientContext ?? throw new ArgumentException("sourceClientContext must be provided."); - this.targetClientContext = targetClientContext ?? throw new ArgumentException("targetClientContext must be provided."); ; + this.targetClientContext = targetClientContext ?? this.sourceClientContext; this.version = GetVersion(); this.pageTelemetry = new PageTelemetry(version); @@ -179,15 +179,12 @@ public string Transform(PublishingPageTransformationInformation publishingPageTr var sourceWebId = sameSiteCollection ? sourceClientContext.Web.EnsureProperty(p => p.Id) : Guid.Empty; var hasWritableSitePages = false; - if (sameSiteCollection && - sourceWebId.Equals(targetClientContext.Web.Id) && - publishingPageTransformationInformation.InPlacePublishingPage) + if (sameSiteCollection && sourceWebId.Equals(targetClientContext.Web.Id)) { hasWritableSitePages = HasWritableSitePagesLibrary(sourceClientContext); } var transformationTarget = PublishingPageTransformationValidator.ValidateTarget( - publishingPageTransformationInformation.InPlacePublishingPage, sourceClientContext.Site.Id, targetClientContext.Site.Id, sourceWebId, @@ -694,7 +691,7 @@ internal PnPCore.IPage CreateTargetPageAfterCollisionProbe( } else if (transformationTarget == PublishingPageTransformationTarget.SameWeb) { - // The in-place opt-in must fail closed when the target collision probe is inconclusive. + // Stop in-place transformation when the target collision probe is inconclusive. throw; } else @@ -725,12 +722,6 @@ private void EnsureTransformationTargetAllowed(PublishingPageTransformationTarge { switch (transformationTarget) { - case PublishingPageTransformationTarget.SameSiteCollectionNotAllowed: - LogError(LogStrings.Error_SameSiteTransferNoAllowedForPublishingPages, LogStrings.Heading_SharePointConnection); - throw new ArgumentNullException(LogStrings.Error_SameSiteTransferNoAllowedForPublishingPages); - case PublishingPageTransformationTarget.InPlaceDifferentSiteCollection: - LogError(LogStrings.Error_InPlacePublishingPageDifferentSiteCollection, LogStrings.Heading_SharePointConnection); - throw new ArgumentException(LogStrings.Error_InPlacePublishingPageDifferentSiteCollection); case PublishingPageTransformationTarget.SameSiteCollectionDifferentWeb: LogError(LogStrings.Error_InPlacePublishingPageDifferentWeb, LogStrings.Heading_SharePointConnection); throw new ArgumentException(LogStrings.Error_InPlacePublishingPageDifferentWeb); diff --git a/src/lib/PnP.Framework/Modernization/Telemetry/LogStrings.cs b/src/lib/PnP.Framework/Modernization/Telemetry/LogStrings.cs index 67725ff11..0aadc0e17 100644 --- a/src/lib/PnP.Framework/Modernization/Telemetry/LogStrings.cs +++ b/src/lib/PnP.Framework/Modernization/Telemetry/LogStrings.cs @@ -78,8 +78,7 @@ public static class LogStrings public const string Error_OverridingTagePageTakesSourcePageName = "Overriding 'TargetPageTakesSourcePageName' to ensure that the newly created page in the other site collection gets the same name as the source page"; public const string Error_FallBackToSameSiteTransfer = "Oops, seems source and target point to the same site collection...switch back the 'source only' mode"; public const string Error_SameSiteTransferNoAllowedForPublishingPages = "Oops, seems source and target point to the same site collection...that's a no go for publishing portal page transformation!"; - public const string Error_InPlacePublishingPageDifferentSiteCollection = "In-place publishing page transformation requires source and target to be in the same site collection and Web."; - public const string Error_InPlacePublishingPageDifferentWeb = "In-place publishing page transformation requires source and target to be the same Web."; + public const string Error_InPlacePublishingPageDifferentWeb = "Publishing page transformation between different Webs in the same site collection is not supported."; public const string Error_InPlacePublishingPageRequiresWritableSitePages = "In-place publishing page transformation requires an existing Site Pages library with Add Items and Edit Items permissions."; public const string Error_InPlacePublishingPageSitePagesNotFound = "The Site Pages library required for in-place publishing page transformation was not found."; public const string Error_InPlacePublishingPageSitePagesInvalidTemplate = "The Site Pages path does not resolve to a Web Page Library."; From a253fbe2b74458a74adc54692d9fd2a2f42ba7a5 Mon Sep 17 00:00:00 2001 From: titao Date: Sat, 10 Oct 2026 18:13:43 +0800 Subject: [PATCH 06/10] Load modernization root web as an independent CSOM object --- .../Modernization/Transform/BasePageTransformator.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lib/PnP.Framework/Modernization/Transform/BasePageTransformator.cs b/src/lib/PnP.Framework/Modernization/Transform/BasePageTransformator.cs index cd959ea89..463378147 100644 --- a/src/lib/PnP.Framework/Modernization/Transform/BasePageTransformator.cs +++ b/src/lib/PnP.Framework/Modernization/Transform/BasePageTransformator.cs @@ -1102,7 +1102,9 @@ internal void LoadClientObject(ClientContext clientContext, bool isTargetContext { clientContext.Load(clientContext.Web, p => p.Id, p => p.ServerRelativeUrl, p => p.RootFolder.WelcomePage, p => p.Language); } - clientContext.Load(clientContext.Site, p => p.RootWeb.ServerRelativeUrl, p => p.Id, p => p.Url); + clientContext.Load(clientContext.Site, p => p.Id, p => p.Url); + // Load RootWeb as its own CSOM object instead of expanding it through Site. + clientContext.Load(clientContext.Site.RootWeb, p => p.ServerRelativeUrl); // Use regular ExecuteQuery as we want to send this custom clienttag clientContext.ExecuteQuery(); } From 1551b37ebdb3b248adf71ebf57e841d94dbcab84 Mon Sep 17 00:00:00 2001 From: titao Date: Sat, 10 Oct 2026 19:06:26 +0800 Subject: [PATCH 07/10] Resolve image mapping defaults when the source file is missing --- .../Functions/BuiltInImageLookupTests.cs | 15 +++++++++++++-- .../Modernization/Functions/BuiltIn.cs | 12 +++++++++--- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/lib/PnP.Framework.Modernization.Test/Transform/Functions/BuiltInImageLookupTests.cs b/src/lib/PnP.Framework.Modernization.Test/Transform/Functions/BuiltInImageLookupTests.cs index b14fd9000..776bb6b5f 100644 --- a/src/lib/PnP.Framework.Modernization.Test/Transform/Functions/BuiltInImageLookupTests.cs +++ b/src/lib/PnP.Framework.Modernization.Test/Transform/Functions/BuiltInImageLookupTests.cs @@ -87,7 +87,7 @@ public void ImageLookup_WhenLookupIsSkipped_PreservesExistingDefaults() } [TestMethod] - public void ImageLookup_WhenFileIsNotFound_PreservesNullResult() + public void ImageLookup_WhenFileIsNotFound_ReturnsNumericDefaults() { using (var context = CreateContext(CreateProvider(false, null, false, null))) { @@ -99,7 +99,13 @@ public void ImageLookup_WhenFileIsNotFound_PreservesNullResult() var result = builtIn.ImageLookup(ImageServerRelativeUrl); - Assert.IsNull(result); + Assert.IsNotNull(result); + Assert.AreEqual(string.Empty, result["ImageListId"]); + Assert.AreEqual(string.Empty, result["ImageUniqueId"]); + Assert.AreEqual("-1", result["ImageWidth"]); + Assert.AreEqual("-1", result["ImageHeight"]); + AssertMappingWithLookupResult("SharePointPnP.Modernization.WikiImagePart", result); + AssertMappingWithLookupResult("Microsoft.SharePoint.WebPartPages.ImageWebPart, Microsoft.SharePoint, Version=16.0.0.0, Culture=neutral, PublicKeyToken=71e9bce111e9429c", result); } } @@ -118,6 +124,11 @@ public void ClassicImageWebPartMapping_WithDefaultDimensions_ProducesNumericJson private static void AssertMappingWithDefaultDimensions(string webPartType) { var lookupResult = InvokeImageLookup(false, null, false, null); + AssertMappingWithLookupResult(webPartType, lookupResult); + } + + private static void AssertMappingWithLookupResult(string webPartType, Dictionary lookupResult) + { var mapping = LoadImageJsonControlData(webPartType); var replacements = new Dictionary { diff --git a/src/lib/PnP.Framework/Modernization/Functions/BuiltIn.cs b/src/lib/PnP.Framework/Modernization/Functions/BuiltIn.cs index 56db75b9d..4a1795e4d 100644 --- a/src/lib/PnP.Framework/Modernization/Functions/BuiltIn.cs +++ b/src/lib/PnP.Framework/Modernization/Functions/BuiltIn.cs @@ -647,7 +647,7 @@ public string ListDetectUsedView(Guid listId, string xmlDefinition) /// /// Server relative path of the image /// A set of image properties - [FunctionDocumentation(Description = "Does lookup a file based on the given server relative path and return needed properties of the file. Returns null if file was not found.", + [FunctionDocumentation(Description = "Looks up image properties by server relative path. Returns empty file identities and numeric defaults when the image file was not found.", Example = "ImageLookup({ServerRelativeFileName})")] [InputDocumentation(Name = "{ServerRelativeFileName}", Description = "Server relative file name of the image")] [OutputDocumentation(Name = "{ImageListId}", Description = "Id of the list holding the file")] @@ -722,8 +722,14 @@ public Dictionary ImageLookup(string serverRelativeImagePath) { if (ex.ServerErrorTypeName == "System.IO.FileNotFoundException") { - // Provided image was not found, should not happen - return null; + // Keep the original image URL when its file is missing, but still resolve + // the required numeric tokens in the modern Image web part mapping. + LogWarning($"Image file was not found; retaining its source URL: {serverRelativeImagePath}", LogStrings.Heading_BuiltInFunctions); + results["ImageListId"] = ""; + results["ImageUniqueId"] = ""; + results["ImageHeight"] = "-1"; + results["ImageWidth"] = "-1"; + return results; } else { From 0aadaebf806c4c3d7f34fea95c1d31b9987530e8 Mon Sep 17 00:00:00 2001 From: Tianyi Tao Date: Thu, 15 Oct 2026 11:12:35 +0800 Subject: [PATCH 08/10] Require an explicit opt-in for in-place publishing page transformation Restore the ArgumentException that both PublishingPageTransformator constructors threw for a null targetClientContext. Aliasing null to the source context silently turned existing callers that relied on that guard into in-place transformations against their source Web. In-place is now opted into explicitly through the CreateInPlace factory overloads, which pass the source context as the target. Transform() already routes that to SameWeb through the site and Web identities, so no additional flag is needed. Signal a missing target page with PageDoesNotExistInWebException instead of matching LogStrings text. LogStrings are English-only constants that are documented as pending localization, so the previous Message.EndsWith check would have silently stopped routing in-place collisions. The new type derives from ArgumentException, so existing handlers that test for ArgumentException keep their current behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e36329ef-32f5-4a3e-945a-ecc5f1f68f9d --- ...ublishingPageInPlaceTransformationTests.cs | 40 +++++++++++-- .../Publishing/PublishingPageTransformator.cs | 56 +++++++++++++++++-- .../PageDoesNotExistInWebException.cs | 27 +++++++++ .../Transform/PageTransformator.cs | 2 +- 4 files changed, 114 insertions(+), 11 deletions(-) create mode 100644 src/lib/PnP.Framework/Modernization/Transform/PageDoesNotExistInWebException.cs diff --git a/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs b/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs index 96b6a785a..f66b20c92 100644 --- a/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs +++ b/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs @@ -22,11 +22,11 @@ public class PublishingPageInPlaceTransformationTests public TestContext TestContext { get; set; } [TestMethod] - public void ModelConstructorWithoutTargetUsesSourceContext() + public void ModelCreateInPlaceUsesSourceContextAsTarget() { using (var sourceContext = CreateContext("https://contoso.sharepoint.com/sites/enterprise-wiki")) { - var transformator = CreateTransformator(sourceContext, null); + var transformator = PublishingPageTransformator.CreateInPlace(sourceContext, new PageTransformation(), null); Assert.AreSame(sourceContext, transformator.sourceClientContext); Assert.AreSame(sourceContext, transformator.targetClientContext); @@ -34,7 +34,7 @@ public void ModelConstructorWithoutTargetUsesSourceContext() } [TestMethod] - public void FileMappingConstructorWithoutTargetUsesSourceContext() + public void FileMappingCreateInPlaceUsesSourceContextAsTarget() { Directory.CreateDirectory(TestContext.TestResultsDirectory); var mappingPath = Path.Combine(TestContext.TestResultsDirectory, $"webpartmapping-{Guid.NewGuid():N}.xml"); @@ -44,7 +44,7 @@ public void FileMappingConstructorWithoutTargetUsesSourceContext() { using (var sourceContext = CreateContext("https://contoso.sharepoint.com/sites/enterprise-wiki")) { - var transformator = new PublishingPageTransformator(sourceContext, null, mappingPath, null); + var transformator = PublishingPageTransformator.CreateInPlace(sourceContext, mappingPath, null); Assert.AreSame(sourceContext, transformator.sourceClientContext); Assert.AreSame(sourceContext, transformator.targetClientContext); @@ -56,6 +56,36 @@ public void FileMappingConstructorWithoutTargetUsesSourceContext() } } + [TestMethod] + public void ModelConstructorStillRejectsANullTargetContext() + { + using (var sourceContext = CreateContext("https://contoso.sharepoint.com/sites/enterprise-wiki")) + { + Assert.ThrowsException(() => CreateTransformator(sourceContext, null)); + } + } + + [TestMethod] + public void FileMappingConstructorStillRejectsANullTargetContext() + { + Directory.CreateDirectory(TestContext.TestResultsDirectory); + var mappingPath = Path.Combine(TestContext.TestResultsDirectory, $"webpartmapping-{Guid.NewGuid():N}.xml"); + System.IO.File.WriteAllText(mappingPath, BasePageTransformator.LoadDefaultWebPartMappingFile()); + + try + { + using (var sourceContext = CreateContext("https://contoso.sharepoint.com/sites/enterprise-wiki")) + { + Assert.ThrowsException( + () => new PublishingPageTransformator(sourceContext, null, mappingPath, null)); + } + } + finally + { + System.IO.File.Delete(mappingPath); + } + } + [TestMethod] public void SameWebTargetIsAcceptedWithoutAdditionalOptions() { @@ -278,7 +308,7 @@ public void TransformSeamConfirmedMissingTargetInvokesAddOnce() transformator.CreateTargetPageAfterCollisionProbe( PublishingPageTransformationTarget.SameWeb, information, - () => throw new ArgumentException($"{information.TargetPageName} - {LogStrings.TransformPageDoesNotExistInWeb}"), + () => throw new PageDoesNotExistInWebException($"{information.TargetPageName} - {LogStrings.TransformPageDoesNotExistInWeb}"), pageName => { addCalls++; diff --git a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs index d06b21f21..768b69ae1 100644 --- a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs +++ b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs @@ -40,7 +40,7 @@ public PublishingPageTransformator(ClientContext sourceClientContext, ClientCont #endif this.sourceClientContext = sourceClientContext ?? throw new ArgumentException("sourceClientContext must be provided."); - this.targetClientContext = targetClientContext ?? this.sourceClientContext; + this.targetClientContext = targetClientContext ?? throw new ArgumentException("targetClientContext must be provided."); this.version = GetVersion(); this.pageTelemetry = new PageTelemetry(version); @@ -79,7 +79,7 @@ public PublishingPageTransformator(ClientContext sourceClientContext, ClientCont #endif this.sourceClientContext = sourceClientContext ?? throw new ArgumentException("sourceClientContext must be provided."); - this.targetClientContext = targetClientContext ?? this.sourceClientContext; + this.targetClientContext = targetClientContext ?? throw new ArgumentException("targetClientContext must be provided."); this.version = GetVersion(); this.pageTelemetry = new PageTelemetry(version); @@ -96,6 +96,51 @@ public PublishingPageTransformator(ClientContext sourceClientContext, ClientCont this.publishingPageTransformation = this.pageLayoutManager.MergePageLayoutMappingFiles(this.publishingPageTransformation, customPublishingPageTransformationModel); } } + + /// + /// Creates a transformator that writes the modern page into the Web that holds the publishing page. + /// + /// Client context of the Web holding the publishing page + /// A transformator configured for in-place transformation + public static PublishingPageTransformator CreateInPlace(ClientContext sourceClientContext) + { + return new PublishingPageTransformator(sourceClientContext, sourceClientContext); + } + + /// + /// Creates a transformator that writes the modern page into the Web that holds the publishing page. + /// + /// Client context of the Web holding the publishing page + /// Page layout mapping file + /// A transformator configured for in-place transformation + public static PublishingPageTransformator CreateInPlace(ClientContext sourceClientContext, string publishingPageTransformationFile) + { + return new PublishingPageTransformator(sourceClientContext, sourceClientContext, publishingPageTransformationFile); + } + + /// + /// Creates a transformator that writes the modern page into the Web that holds the publishing page. + /// + /// Client context of the Web holding the publishing page + /// Web part mapping file + /// Page layout mapping file + /// A transformator configured for in-place transformation + public static PublishingPageTransformator CreateInPlace(ClientContext sourceClientContext, string pageTransformationFile, string publishingPageTransformationFile) + { + return new PublishingPageTransformator(sourceClientContext, sourceClientContext, pageTransformationFile, publishingPageTransformationFile); + } + + /// + /// Creates a transformator that writes the modern page into the Web that holds the publishing page. + /// + /// Client context of the Web holding the publishing page + /// Web part mapping model + /// Page layout mapping model + /// A transformator configured for in-place transformation + public static PublishingPageTransformator CreateInPlace(ClientContext sourceClientContext, PageTransformation pageTransformationModel, PublishingPageTransformation customPublishingPageTransformationModel) + { + return new PublishingPageTransformator(sourceClientContext, sourceClientContext, pageTransformationModel, customPublishingPageTransformationModel); + } #endregion /// @@ -682,8 +727,9 @@ internal PnPCore.IPage CreateTargetPageAfterCollisionProbe( } catch (Exception ex) { - var targetPageDoesNotExist = ex is ArgumentException && - ex.Message.EndsWith(LogStrings.TransformPageDoesNotExistInWeb, StringComparison.InvariantCulture); + // Match the "page not found" signal structurally. LogStrings are English-only constants that are + // intended to be localized, so matching on the message text would silently break this routing. + var targetPageDoesNotExist = ex is PageDoesNotExistInWebException; if (targetPageDoesNotExist) { @@ -850,7 +896,7 @@ private Microsoft.SharePoint.Client.File Load(ClientContext sourceContext, Clien if (!file.Exists) { LogInfo(LogStrings.TransformPageDoesNotExistInWeb, LogStrings.Heading_Load); //Not an error this is a check - throw new ArgumentException($"{publishingPageTransformationInformation.TargetPageName} - {LogStrings.TransformPageDoesNotExistInWeb}"); + throw new PageDoesNotExistInWebException($"{publishingPageTransformationInformation.TargetPageName} - {LogStrings.TransformPageDoesNotExistInWeb}"); } return file; diff --git a/src/lib/PnP.Framework/Modernization/Transform/PageDoesNotExistInWebException.cs b/src/lib/PnP.Framework/Modernization/Transform/PageDoesNotExistInWebException.cs new file mode 100644 index 000000000..fe13964b3 --- /dev/null +++ b/src/lib/PnP.Framework/Modernization/Transform/PageDoesNotExistInWebException.cs @@ -0,0 +1,27 @@ +using System; + +namespace PnP.Framework.Modernization.Transform +{ +#pragma warning disable CA1032 + /// + /// Exception class thrown when a probed page does not exist in the web. Derives from + /// so that existing handlers catching + /// keep observing the previous behavior. + /// + public class PageDoesNotExistInWebException : ArgumentException + { + /// + /// Throws a PageDoesNotExistInWebException message + /// + /// Error message + public PageDoesNotExistInWebException(string message) : base(message) { } + + /// + /// Throws a PageDoesNotExistInWebException message + /// + /// Error message + /// Inner exception object + public PageDoesNotExistInWebException(string message, Exception innerException) : base(message, innerException) { } + } +#pragma warning restore CA1032 +} diff --git a/src/lib/PnP.Framework/Modernization/Transform/PageTransformator.cs b/src/lib/PnP.Framework/Modernization/Transform/PageTransformator.cs index 8b4203348..0f3991697 100644 --- a/src/lib/PnP.Framework/Modernization/Transform/PageTransformator.cs +++ b/src/lib/PnP.Framework/Modernization/Transform/PageTransformator.cs @@ -1278,7 +1278,7 @@ private Microsoft.SharePoint.Client.File Load(ClientContext sourceContext, PageT if (!file.Exists) { LogInfo(LogStrings.TransformPageDoesNotExistInWeb, LogStrings.Heading_Load); - throw new ArgumentException($"{pageTransformationInformation.TargetPageName} - {LogStrings.TransformPageDoesNotExistInWeb}"); + throw new PageDoesNotExistInWebException($"{pageTransformationInformation.TargetPageName} - {LogStrings.TransformPageDoesNotExistInWeb}"); } return file; From 452483525f28f9bb8d8d11f23f31eab24e818211 Mon Sep 17 00:00:00 2001 From: Tianyi Tao Date: Thu, 15 Oct 2026 16:12:28 +0800 Subject: [PATCH 09/10] Canonicalize the publishing target page name before the collision probe Saving a client side page appends the .aspx extension when it is missing, while the collision probe, the save call and the readback all used the caller supplied target page name verbatim. A name such as 'existing' therefore probed /SitePages/existing, reported the target as free, and then saved over the existing /SitePages/existing.aspx, because the save path has no overwrite concept of its own. The readback used the uncanonicalized name again, so the transformation reported a failure after the existing page had already been updated. Canonicalize TargetPageName once, directly after the source page default is applied, so the probe, the save and the readback all resolve to the same file. This also closes the same hole on the cross site path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e36329ef-32f5-4a3e-945a-ecc5f1f68f9d --- ...ublishingPageInPlaceTransformationTests.cs | 36 +++++++++++++++++++ .../Publishing/PublishingPageTransformator.cs | 24 +++++++++++++ 2 files changed, 60 insertions(+) diff --git a/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs b/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs index f66b20c92..5d09440a5 100644 --- a/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs +++ b/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs @@ -322,6 +322,42 @@ public void TransformSeamConfirmedMissingTargetInvokesAddOnce() } } + [TestMethod] + public void TargetPageNameWithoutExtensionIsCanonicalized() + { + // Saving appends .aspx, so an uncanonicalized name would probe and read back a different + // file than the one written, letting an existing target page be overwritten unnoticed. + Assert.AreEqual("existing.aspx", PublishingPageTransformator.EnsureAspxExtension("existing")); + } + + [TestMethod] + public void TargetPageNameKeepsAnExistingExtensionRegardlessOfCasing() + { + Assert.AreEqual("existing.aspx", PublishingPageTransformator.EnsureAspxExtension("existing.aspx")); + Assert.AreEqual("existing.ASPX", PublishingPageTransformator.EnsureAspxExtension("existing.ASPX")); + } + + [TestMethod] + public void TargetPageNameCanonicalizationIsIdempotent() + { + var canonicalized = PublishingPageTransformator.EnsureAspxExtension("existing"); + + Assert.AreEqual(canonicalized, PublishingPageTransformator.EnsureAspxExtension(canonicalized)); + } + + [TestMethod] + public void TargetPageNameCanonicalizationKeepsEmbeddedDots() + { + Assert.AreEqual("report.v2.aspx", PublishingPageTransformator.EnsureAspxExtension("report.v2")); + } + + [TestMethod] + public void TargetPageNameCanonicalizationLeavesAnAbsentNameToTheSourcePageDefault() + { + Assert.IsNull(PublishingPageTransformator.EnsureAspxExtension(null)); + Assert.AreEqual(string.Empty, PublishingPageTransformator.EnsureAspxExtension(string.Empty)); + } + private static PublishingPageTransformationTarget ValidateSameWeb( bool hasWritableSitePages) { diff --git a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs index 768b69ae1..4ecff7e72 100644 --- a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs +++ b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs @@ -364,6 +364,12 @@ public string Transform(PublishingPageTransformationInformation publishingPageTr publishingPageTransformationInformation.TargetPageName = $"{publishingPageTransformationInformation.SourcePage[Constants.FileLeafRefField].ToString()}"; } + // Canonicalize the target page name once, before it is used to probe for a collision, to save the + // page and to read the saved page back. Saving appends the .aspx extension when it is missing, so a + // caller supplied name without the extension would otherwise probe and read back a different file + // than the one that is written, and an existing target page could be overwritten unnoticed. + publishingPageTransformationInformation.TargetPageName = EnsureAspxExtension(publishingPageTransformationInformation.TargetPageName); + // Check if page name is free to use #if DEBUG && MEASURE Start(); @@ -764,6 +770,24 @@ internal PnPCore.IPage CreateTargetPageAfterCollisionProbe( return addClientSidePage($"{publishingPageTransformationInformation.Folder}{publishingPageTransformationInformation.TargetPageName}"); } + /// + /// Appends the .aspx extension to a target page name when it is missing, matching the name the page is + /// actually saved under. The comparison is case insensitive, so an existing extension is never duplicated. + /// + /// Target page name supplied by the caller or derived from the source page + /// The target page name as it will exist in the Site Pages library + internal static string EnsureAspxExtension(string targetPageName) + { + if (string.IsNullOrEmpty(targetPageName)) + { + return targetPageName; + } + + return targetPageName.EndsWith(".aspx", StringComparison.InvariantCultureIgnoreCase) + ? targetPageName + : $"{targetPageName}.aspx"; + } + private void EnsureTransformationTargetAllowed(PublishingPageTransformationTarget transformationTarget) { switch (transformationTarget) From 2a3887d41c480a1463276100fafbbdecae93bd96 Mon Sep 17 00:00:00 2001 From: Tianyi Tao Date: Thu, 15 Oct 2026 16:51:20 +0800 Subject: [PATCH 10/10] Canonicalize the target page name for wiki and web part pages too The same uncanonicalized name that let a publishing transformation overwrite an existing target page also reaches PageTransformator. Its collision probe, its save call and its readback each use TargetPageName verbatim, so a cross site conversion with a name such as 'existing' probed /SitePages/existing, reported the target as free, then saved over /SitePages/existing.aspx and failed on the readback. Move EnsureAspxExtension to BasePageTransformator and apply it in PageTransformator directly after the target name default is resolved, so both transformators canonicalize once before the probe. Verified live against a publishing enabled tenant: before this change a cross site wiki conversion without -Overwrite replaced the target page content and then reported a failure; after it, the collision is rejected and the target page is byte identical. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e36329ef-32f5-4a3e-945a-ecc5f1f68f9d --- ...ublishingPageInPlaceTransformationTests.cs | 16 +++++++-------- .../Publishing/PublishingPageTransformator.cs | 18 ----------------- .../Transform/BasePageTransformator.cs | 20 +++++++++++++++++++ .../Transform/PageTransformator.cs | 6 ++++++ 4 files changed, 34 insertions(+), 26 deletions(-) diff --git a/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs b/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs index 5d09440a5..e38af419b 100644 --- a/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs +++ b/src/lib/PnP.Framework.Modernization.Test/Transform/Publishing/PublishingPageInPlaceTransformationTests.cs @@ -327,35 +327,35 @@ public void TargetPageNameWithoutExtensionIsCanonicalized() { // Saving appends .aspx, so an uncanonicalized name would probe and read back a different // file than the one written, letting an existing target page be overwritten unnoticed. - Assert.AreEqual("existing.aspx", PublishingPageTransformator.EnsureAspxExtension("existing")); + Assert.AreEqual("existing.aspx", BasePageTransformator.EnsureAspxExtension("existing")); } [TestMethod] public void TargetPageNameKeepsAnExistingExtensionRegardlessOfCasing() { - Assert.AreEqual("existing.aspx", PublishingPageTransformator.EnsureAspxExtension("existing.aspx")); - Assert.AreEqual("existing.ASPX", PublishingPageTransformator.EnsureAspxExtension("existing.ASPX")); + Assert.AreEqual("existing.aspx", BasePageTransformator.EnsureAspxExtension("existing.aspx")); + Assert.AreEqual("existing.ASPX", BasePageTransformator.EnsureAspxExtension("existing.ASPX")); } [TestMethod] public void TargetPageNameCanonicalizationIsIdempotent() { - var canonicalized = PublishingPageTransformator.EnsureAspxExtension("existing"); + var canonicalized = BasePageTransformator.EnsureAspxExtension("existing"); - Assert.AreEqual(canonicalized, PublishingPageTransformator.EnsureAspxExtension(canonicalized)); + Assert.AreEqual(canonicalized, BasePageTransformator.EnsureAspxExtension(canonicalized)); } [TestMethod] public void TargetPageNameCanonicalizationKeepsEmbeddedDots() { - Assert.AreEqual("report.v2.aspx", PublishingPageTransformator.EnsureAspxExtension("report.v2")); + Assert.AreEqual("report.v2.aspx", BasePageTransformator.EnsureAspxExtension("report.v2")); } [TestMethod] public void TargetPageNameCanonicalizationLeavesAnAbsentNameToTheSourcePageDefault() { - Assert.IsNull(PublishingPageTransformator.EnsureAspxExtension(null)); - Assert.AreEqual(string.Empty, PublishingPageTransformator.EnsureAspxExtension(string.Empty)); + Assert.IsNull(BasePageTransformator.EnsureAspxExtension(null)); + Assert.AreEqual(string.Empty, BasePageTransformator.EnsureAspxExtension(string.Empty)); } private static PublishingPageTransformationTarget ValidateSameWeb( diff --git a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs index 4ecff7e72..f04e5a0e7 100644 --- a/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs +++ b/src/lib/PnP.Framework/Modernization/Publishing/PublishingPageTransformator.cs @@ -770,24 +770,6 @@ internal PnPCore.IPage CreateTargetPageAfterCollisionProbe( return addClientSidePage($"{publishingPageTransformationInformation.Folder}{publishingPageTransformationInformation.TargetPageName}"); } - /// - /// Appends the .aspx extension to a target page name when it is missing, matching the name the page is - /// actually saved under. The comparison is case insensitive, so an existing extension is never duplicated. - /// - /// Target page name supplied by the caller or derived from the source page - /// The target page name as it will exist in the Site Pages library - internal static string EnsureAspxExtension(string targetPageName) - { - if (string.IsNullOrEmpty(targetPageName)) - { - return targetPageName; - } - - return targetPageName.EndsWith(".aspx", StringComparison.InvariantCultureIgnoreCase) - ? targetPageName - : $"{targetPageName}.aspx"; - } - private void EnsureTransformationTargetAllowed(PublishingPageTransformationTarget transformationTarget) { switch (transformationTarget) diff --git a/src/lib/PnP.Framework/Modernization/Transform/BasePageTransformator.cs b/src/lib/PnP.Framework/Modernization/Transform/BasePageTransformator.cs index 463378147..74f36ce38 100644 --- a/src/lib/PnP.Framework/Modernization/Transform/BasePageTransformator.cs +++ b/src/lib/PnP.Framework/Modernization/Transform/BasePageTransformator.cs @@ -1087,6 +1087,26 @@ internal void Stop(string method) /// /// /// + /// + /// Appends the .aspx extension to a target page name when it is missing, matching the name the page is + /// actually saved under. The comparison is case insensitive, so an existing extension is never duplicated. + /// Saving a client side page appends the extension on its own, so the collision probe, the save and the + /// readback must all be given the same canonical name or they resolve to different files. + /// + /// Target page name supplied by the caller or derived from the source page + /// The target page name as it will exist in the Site Pages library + internal static string EnsureAspxExtension(string targetPageName) + { + if (string.IsNullOrEmpty(targetPageName)) + { + return targetPageName; + } + + return targetPageName.EndsWith(".aspx", StringComparison.InvariantCultureIgnoreCase) + ? targetPageName + : $"{targetPageName}.aspx"; + } + internal void LoadClientObject(ClientContext clientContext, bool isTargetContext) { if (clientContext != null) diff --git a/src/lib/PnP.Framework/Modernization/Transform/PageTransformator.cs b/src/lib/PnP.Framework/Modernization/Transform/PageTransformator.cs index 0f3991697..ecd522a00 100644 --- a/src/lib/PnP.Framework/Modernization/Transform/PageTransformator.cs +++ b/src/lib/PnP.Framework/Modernization/Transform/PageTransformator.cs @@ -473,6 +473,12 @@ public string Transform(PageTransformationInformation pageTransformationInformat } + // Canonicalize the target page name once, before it is used to probe for a collision, to save the + // page and to read the saved page back. Saving appends the .aspx extension when it is missing, so a + // caller supplied name without the extension would otherwise probe and read back a different file + // than the one that is written, and an existing target page could be overwritten unnoticed. + pageTransformationInformation.TargetPageName = EnsureAspxExtension(pageTransformationInformation.TargetPageName); + // Check if page name is free to use #if DEBUG && MEASURE Start();