fix: prefer embedded PackageId assembly metadata in packageManifest API

WHAT / WHY
----------
GetPackageManifestRequestHandler previously identified a loaded plugin's
NuGet PackageId by fuzzy-matching AssemblyTitle/AssemblyName against the
config's packageId - a chain that silently fails for any plugin whose
AssemblyTitle/AssemblyName doesn't happen to match its PackageId (verified
against real shipped plugin DLLs; see FINDINGS-nuget-packageid-gaps.md).

epi-symetrix-dsp and epi-shure-mxa have already backported a
Directory.Build.props change that embeds
`<AssemblyMetadata Include="PackageId" Value="$(PackageId)" />`, giving
Essentials an unambiguous, authoritative PackageId via
AssemblyMetadataAttribute("PackageId", ...) instead of guessing. Validated
by building both plugins and inspecting the generated AssemblyInfo.cs.

Changes:
- src/Directory.Build.props: add the same AssemblyMetadata PackageId item,
  so every Essentials-owned assembly (Core, Essentials, Devices.Common,
  MobileControl, MobileControl.Messengers) now embeds its real PackageId
  too - previously none of them did.
- GetPackageManifestRequestHandler.cs:
  - PopulatePackages: read AssemblyMetadataAttribute("PackageId", ...) from
    loaded plugin assemblies and use it as the first-priority match/identity
    signal, ahead of the AssemblyTitle -> AssemblyName -> AssemblyName-minus-
    ".4Series" fallback chain. Loaded-but-unconfigured plugins that carry
    this metadata now report a PackageId in the manifest instead of null.
  - PopulateEssentials: replace the hardcoded "PepperDash.Essentials"
    fallback (which matched none of the real PackageIds) with the reflected
    value from PepperDash.Essentials.Core's own assembly metadata.

Fully backward compatible: plugins without the updated Directory.Build.props
(most existing epi-* repos today) fall through to the prior fallback chain
unchanged.

RECOMMENDATIONS - Essentials & sub-projects (this repo)
--------------------------------------------------------
- AssemblyName/AssemblyTitle drift from PackageId across sub-projects
  (confirmed via generated AssemblyInfo.cs, not assumed):
    PepperDash.Essentials.Core:                 PackageId "PepperDash.Essentials.Core"
                                                 vs AssemblyName "PepperDash_Essentials_Core"
    PepperDash.Essentials.Devices.Common:        PackageId "PepperDash.Essentials.Devices.Common"
                                                 vs AssemblyName "Essentials Devices Common"
    PepperDash.Essentials.MobileControl:         PackageId "PepperDash.Essentials.MobileControl"
                                                 vs AssemblyName "epi-essentials-mobile-control"
    PepperDash.Essentials.MobileControl.Messengers: PackageId "...Messengers"
                                                 vs AssemblyName "mobile-control-messengers"
  Only PepperDash.Essentials and PepperDash.Core happen to agree. Fixing
  AssemblyName changes the physical .dll filename for existing consumers,
  so this needs a deliberate, versioned decision - not bundled here.
- Once this ships and bakes for a release or two, consider deleting the
  now-redundant "PepperDash.Essentials" hardcoded string entirely and the
  Product/AssemblyTitle-based Name fallback, since AssemblyMetadata PackageId
  supersedes both for any assembly built after this change.

RECOMMENDATIONS - EPI plugin repos (epi-*)
-------------------------------------------
- Backport `<AssemblyMetadata Include="PackageId" Value="$(PackageId)" />`
  into every existing epi-* repo's src/Directory.Build.props (recommendation
  E from FINDINGS-nuget-packageid-gaps.md). This is opt-in and additive -
  repos that skip it keep working via the existing fallback chain, but gain
  nothing until they backport it and cut a new release.
- Land the corresponding fix in EssentialsPluginTemplate
  (src/Directory.Build.props + src/epi-make-model.4Series.csproj) so all
  *new* plugin repos get this by default, and fix the template's own
  AssemblyTitle/PackageId drift ("Plugin" vs "Plugins") while there.
- Already-published plugin versions can't be retroactively fixed - this only
  takes effect on a plugin's next release after adopting the template change.

WORKFLOW RECOMMENDATIONS
-------------------------
- Extend workflow-templates' essentialsplugins-4Series-builds.yml "Check
  Package Name" step to validate the built DLL's embedded
  AssemblyMetadataAttribute("PackageId", ...) (and/or AssemblyTitle as a
  fallback) against the repo-derived expected package name - today it only
  compares the .nupkg filename, which would not have caught drift like
  epi-display-samsung-mdc's AssemblyTitle mismatch.
- Sequence this after the template + per-repo backports have landed and
  baked for a release cycle, otherwise it will fail CI for every epi-*
  repo that hasn't picked up the Directory.Build.props change yet. Gate it
  behind the existing bypassPackageCheck input for repos not yet ready.
This commit is contained in:
jkdevito 2026-07-07 16:22:23 -05:00
parent 588057f3ba
commit 2f7789b374
2 changed files with 27 additions and 8 deletions

View file

@ -20,4 +20,7 @@
<None Include="..\..\LICENSE.md" Pack="true" PackagePath=""/> <None Include="..\..\LICENSE.md" Pack="true" PackagePath=""/>
<None Include="..\..\README.md" Pack="true" PackagePath=""/> <None Include="..\..\README.md" Pack="true" PackagePath=""/>
</ItemGroup> </ItemGroup>
<ItemGroup>
<AssemblyMetadata Include="PackageId" Value="$(PackageId)" />
</ItemGroup>
</Project> </Project>

View file

@ -93,7 +93,15 @@ namespace PepperDash.Essentials.Core.Web.RequestHandlers
essentials.Name = name; essentials.Name = name;
} }
if (string.IsNullOrEmpty(essentials.PackageId)) // Prefer the PackageId embedded via Directory.Build.props' AssemblyMetadata item (the
// authoritative source, matching the actual published PackageId) over any config-supplied
// or hardcoded value.
var reflectedPackageId = GetAssemblyMetadataValue(essentialsAssembly, "PackageId");
if (!string.IsNullOrEmpty(reflectedPackageId))
{
essentials.PackageId = reflectedPackageId;
}
else if (string.IsNullOrEmpty(essentials.PackageId))
{ {
essentials.PackageId = "PepperDash.Essentials"; essentials.PackageId = "PepperDash.Essentials";
} }
@ -123,6 +131,11 @@ namespace PepperDash.Essentials.Core.Web.RequestHandlers
var reflectedRepoUrl = TrimTrailingGit(GetAssemblyMetadataValue(loaded.Assembly, "RepositoryUrl")); var reflectedRepoUrl = TrimTrailingGit(GetAssemblyMetadataValue(loaded.Assembly, "RepositoryUrl"));
var reflectedName = GetAssemblyProduct(loaded.Assembly); var reflectedName = GetAssemblyProduct(loaded.Assembly);
// Plugins built from a Directory.Build.props that embeds
// <AssemblyMetadata Include="PackageId" Value="$(PackageId)" /> carry their PackageId
// directly - this is authoritative and should be preferred over the title/name fallback chain.
var reflectedPackageId = GetAssemblyMetadataValue(loaded.Assembly, "PackageId");
var assemblyTitle = GetAssemblyTitle(loaded.Assembly); var assemblyTitle = GetAssemblyTitle(loaded.Assembly);
var assemblyName = loaded.Assembly.GetName().Name; var assemblyName = loaded.Assembly.GetName().Name;
var assemblyNameNoSeriesSuffix = StripTrailingSeriesSuffix(assemblyName); var assemblyNameNoSeriesSuffix = StripTrailingSeriesSuffix(assemblyName);
@ -130,7 +143,8 @@ namespace PepperDash.Essentials.Core.Web.RequestHandlers
var match = configPackages.FirstOrDefault(p => var match = configPackages.FirstOrDefault(p =>
!matchedConfigPackages.Contains(p) && !matchedConfigPackages.Contains(p) &&
!string.IsNullOrEmpty(p.PackageId) && !string.IsNullOrEmpty(p.PackageId) &&
(string.Equals(p.PackageId, assemblyTitle, StringComparison.OrdinalIgnoreCase) || (string.Equals(p.PackageId, reflectedPackageId, StringComparison.OrdinalIgnoreCase) ||
string.Equals(p.PackageId, assemblyTitle, StringComparison.OrdinalIgnoreCase) ||
string.Equals(p.PackageId, assemblyName, StringComparison.OrdinalIgnoreCase) || string.Equals(p.PackageId, assemblyName, StringComparison.OrdinalIgnoreCase) ||
string.Equals(p.PackageId, assemblyNameNoSeriesSuffix, StringComparison.OrdinalIgnoreCase))); string.Equals(p.PackageId, assemblyNameNoSeriesSuffix, StringComparison.OrdinalIgnoreCase)));
@ -140,20 +154,22 @@ namespace PepperDash.Essentials.Core.Web.RequestHandlers
mergedPackages.Add(new NugetVersion mergedPackages.Add(new NugetVersion
{ {
PackageId = match.PackageId, Name = !string.IsNullOrEmpty(match.Name) ? match.Name : reflectedName,
Version = reflectedVersion,
RepoUrl = !string.IsNullOrEmpty(match.RepoUrl) ? match.RepoUrl : reflectedRepoUrl, RepoUrl = !string.IsNullOrEmpty(match.RepoUrl) ? match.RepoUrl : reflectedRepoUrl,
Name = !string.IsNullOrEmpty(match.Name) ? match.Name : reflectedName PackageId = !string.IsNullOrEmpty(reflectedPackageId) ? reflectedPackageId : match.PackageId,
Version = reflectedVersion
}); });
} }
else else
{ {
// Loaded but not present (or not matched) in config - emit without a packageId // Loaded but not present (or not matched) in config - emit the reflected PackageId
// when the assembly carries one, otherwise leave it null as before.
mergedPackages.Add(new NugetVersion mergedPackages.Add(new NugetVersion
{ {
Version = reflectedVersion, Name = reflectedName,
RepoUrl = reflectedRepoUrl, RepoUrl = reflectedRepoUrl,
Name = reflectedName PackageId = reflectedPackageId,
Version = reflectedVersion,
}); });
} }
} }