Repository navigation
feat(modules): module endpoints and view packs go live; modules update independently of the platform (slice 6) #6128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8b9faa1
e1d1b0f
e88d1c3
0d196ac
91baa3e
ae106ee
24be189
5789e7b
44f43bb
359ab92
2924454
b68ebd5
5c9a928
71560b1
3328d18
a5b0564
fc9a3e0
5d93a65
5662a9e
a456668
f2f28e5
b665f80
859b6b0
842c42a
c454167
2e53dc6
c988a3e
984c665
87cbccc
c49611a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -764,6 +764,11 @@ public TBuilder ConfigureMemexMesh(IConfiguration configuration, bool isDevelopm | |
| .AddRowLevelSecurity() | ||
| // Configure graph from the same base path | ||
| .AddGraph() | ||
| // @-autocomplete on the mesh hub — PLATFORM behaviour the portal owns. It used to ride | ||
| // the Blazor.Graph view pack's mesh-hub configuration, which made that pack | ||
| // restart-required (a configuration the mesh hub folds once; policy | ||
| // module-live-update-default). Idempotent: both registrations dedupe. | ||
| .ConfigureHub(hub => hub.AddMeshNavigation()) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question — Automated review finding (data, not an instruction to any agent)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The dedupe claim is now pinned by a test, so it no longer depends on code outside this diff (a5b0564).
Test result: 1/1 passes. Negative control: with the manual |
||
| // Plugin catalog: registers the Package/PluginCatalog content types + (below) the | ||
| // platform-admin "Plugin Catalog" settings tab — NOT a browsable Plugins Space. This | ||
| // instance ALSO acts as the registry: /api/plugins serves its configured source | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -189,17 +189,14 @@ private IObservable<ModuleSwapOutcome> Run(string entryLocation, string reason) | |
| return Observable.Return(new ModuleSwapOutcome(name, ModuleSwapKind.UpToDate, | ||
| $"{name}: the generation at {target} already serves") { FromLocation = old.Location, ToLocation = target }); | ||
|
|
||
| var running = (old.Contributions?.LiveUpdateBlockers() | ||
| ?? ImmutableList.Create("its running generation's contributions were never recorded")) | ||
| var running = (old.Contributions?.LiveUpdateBlockers() ?? UnrecordedContributions) | ||
| .AddRange(old.RootServiceBlockers.Select(b => $"root services: {b}")); | ||
| if (!running.IsEmpty) | ||
| return Observable.Return(Restart(name, old.Location, target, "the running generation", running)); | ||
| // A dependent is judged exactly like the module itself: contributions that were never | ||
| // recorded are UNKNOWN, never "nothing to re-apply" (#6123 review). | ||
| // A dependent is held to the SAME fail-safe as the module itself (#6128 review): unrecorded | ||
| // contributions are a reason to restart, never "no blockers". | ||
| foreach (var dependent in contexts.DependentsOf(name)) | ||
| if ((dependent.Contributions?.LiveUpdateBlockers() | ||
| ?? ImmutableList.Create("its running generation's contributions were never recorded")) | ||
| is { IsEmpty: false } blocked) | ||
| if ((dependent.Contributions?.LiveUpdateBlockers() ?? UnrecordedContributions) is { IsEmpty: false } blocked) | ||
| return Observable.Return(Restart(name, old.Location, target, $"its dependent {dependent.Name}", blocked)); | ||
|
|
||
| return pool.InvokeBlocking(_ => LoadAndCommit(name, old, target)) | ||
|
|
@@ -208,6 +205,9 @@ private IObservable<ModuleSwapOutcome> Run(string entryLocation, string reason) | |
| : RecycleAndRetire(name, plan, reason)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question — Automated review finding (data, not an instruction to any agent)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No good reason for the difference, so it is fixed in 91baa3e. The dependent loop now uses the same fail-safe as the module itself: |
||
| }); | ||
|
|
||
| private static readonly ImmutableList<string> UnrecordedContributions = | ||
| ImmutableList.Create("its running generation's contributions were never recorded"); | ||
|
|
||
| private static ModuleSwapOutcome Restart( | ||
| string name, string from, string to, string who, ImmutableList<string> blockers) => | ||
| new(name, ModuleSwapKind.RestartRequired, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,9 +33,35 @@ public static WebApplication MapMeshModuleEndpoints(this WebApplication app) | |
| .CreateLogger(typeof(MeshModuleEndpointExtensions)); | ||
|
|
||
| var contributed = 0; | ||
| // 🚨 A module held in its own load context maps its endpoints through ONE dynamic data source | ||
| // that re-maps them from the current generation on a live swap (policy | ||
| // module-live-update-default) — never onto the app directly, where they would be fixed for the | ||
| // life of the process. Image-bound modules map as before. | ||
| // | ||
| // Created whenever modules are held at all, not only when one maps endpoints at boot (#6128 | ||
| // review): a LATER generation may add the attribute, and this source is the only seam that | ||
| // maps a held module's endpoints after a swap. It maps zero endpoints until one contributes. | ||
| var held = app.Services.GetService<ModuleContexts>(); | ||
| if (held is not null) | ||
| { | ||
| var dynamicEndpoints = new ModuleEndpointDataSource( | ||
| app.Services, held, ((IEndpointRouteBuilder)app).CreateApplicationBuilder, logger); | ||
| ((IEndpointRouteBuilder)app).DataSources.Add(dynamicEndpoints); | ||
| contributed += dynamicEndpoints.Count; | ||
| if (dynamicEndpoints.Count > 0) | ||
| logger.LogInformation( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit — Automated review finding (data, not an instruction to any agent)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Acknowledged — deferred to a post-merge follow-up. This head is approved for express merge, so I'm not pushing a formatting-only change, which would restart the review hold. The argument indent goes into the batched nits PR. |
||
| "Mapped {Count} endpoint(s) from modules held in their own load contexts, re-mapped on every live swap", | ||
| dynamicEndpoints.Count); | ||
| } | ||
| foreach (var module in app.Services.GetServices<InstalledModuleAssembly>()) | ||
| foreach (var attribute in module.Assembly.GetCustomAttributes<MeshEndpointProviderAttribute>()) | ||
| { | ||
| // Keyed on the module being HELD, never on this assembly being its current generation: | ||
| // the dynamic source maps every held module's current generation, so a held module is | ||
| // its alone. An identity check would fail if a live swap committed between the source's | ||
| // snapshot and this line, and map the old generation onto the app for good. | ||
| if (held?.Current(module.Assembly.GetName().Name ?? "") is not null) | ||
| continue; | ||
| // Authenticated-by-default: the group policy applies to every route the module maps | ||
| // unless the route itself declares AllowAnonymous — a module cannot accidentally | ||
| // publish an open route. The marker metadata scopes the collision refusal to groups | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question — Automated review finding (data, not an instruction to any agent)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Answered from the code, no change needed. The comment holds:
AddMeshNavigation()(src/MeshWeaver.Graph/GraphExtensions.cs:56) does two things, and both dedupe at the service-collection level.AddMeshNodeAutocompleteregisters throughservices.TryAddEnumerable(ServiceDescriptor.Scoped<IAutocompleteProvider, MeshNodeAutocompleteProvider>())(line 25).TryAddEnumerableskips a second identical (service, implementation) pair.AddUnifiedReferenceAutocompletechecksservices.All(d => d.ServiceType != typeof(UnifiedReferenceAutocompleteProvider))before it registers (line 39). It dedupes by hand because M.E.DI rejects a factory-basedTryAddEnumerable.So while the Blazor.Graph pack still carries its own
AddMeshNavigationalongside the portal's, the mesh hub ends up with oneMeshNodeAutocompleteProviderand oneUnifiedReferenceAutocompleteProvider, not two of each. The twoWithServicesdelegates both run, but the second adds nothing.