[BLOSSOM-304] DialogDefinitionRegistry decoration context missing Created: 27/Jun/22 Updated: 26/Sep/22 |
|
| Status: | Open |
| Project: | Blossom |
| Component/s: | None |
| Affects Version/s: | 3.5.1 |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Critical |
| Reporter: | Björn Eschle | Assignee: | Unassigned |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Template: |
|
| Acceptance criteria: |
Empty
|
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
| Bug DoR: |
[ ]*
Steps to reproduce, expected, and actual results filled
[ ]*
Affected version filled
|
| Date of First Response: | |
| Epic Link: | Support |
| Team: |
| Description |
|
When using blossom with liveCopy, the required blossom dialog definition decoration is not executed, because the instantiation of AppController is throwing NoSuchComponentException ( DialogCreatorDefinitionDecorator.CompatibilityAppsPredicate). As a result, a legacy UI 5 ConfiguredFormDialogDefinition is returned. |
| Comments |
| Comment by Björn Eschle [ 28/Jun/22 ] |
|
I had a look at this issue with @Dominic. The root cause seems to be that there is no MgnlContext present in the dialog action command, which isrequired to build the dialogDefinition in blossom. |
| Comment by Björn Eschle [ 28/Jun/22 ] |
|
Found the cause: PropagateMasterContentChangesActionDefinition is configured as an asynchronous definition in the liveCopy page decoration. The MgnlContext is not set in the executing thread! Redecorating it to asynchronous: false works, but setting the context on the thread would be a nicer solution. Maybe @roman.kovarik@magnolia-cms.com
|
| Comment by Roman Kovařík [ 28/Jun/22 ] |
|
Hey Björn, looks like the blossom dialog definition creation and form creation is bound together. This works fine when opening the dialog from UI (in the thread with WebContext, request/response and AppController) but livecopy probably accesses definitions to get the field names etc. asynchronously (maybe from a command?). I can think of this workaround (not tested): DialogCreationContext newContext = new DialogCreationContext(); if (MgnlContext.isWebContext()) { newContext.setRequest(MgnlContext.getWebContext().getRequest()); newContext.setResponse(MgnlContext.getWebContext().getResponse()); Components.getComponent(AppController.class).getCurrentAppContext() .map(AppContext::getActiveSubAppContext) .map(activeSubAppContext -> SessionStore.access().getBeanStore(UiContextReference.ofSubApp(activeSubAppContext))) .map((Function<BeanStore, ValueContext<?>>) beanStore -> { ValueContext<?> valueContext = beanStore.get(ValueContext.class); newContext.setValueContext(valueContext); newContext.setParentView(beanStore.get(UIComponent.class)); return valueContext; }) .flatMap(ValueContext::getSingle) .filter(Node.class::isInstance) .map(Node.class::cast) .ifPresent(node -> { newContext.setContentNode(node); newContext.setContentPath(NodeUtil.getPathIfPossible(node)); }); } DialogCreationContextHolder.set(newContext); Looks like the request/response are not even used. Not sure about the node/path, you might know better than me, but I'd expect it's required only when dialogs are opened. The proper solution would be most likely moving the DialogCreationContext of out the registry mechanism as I can be accessed outside of UI. Best regards |
| Comment by Björn Eschle [ 28/Jun/22 ] |
|
Hey Roman, thanks for your feedback. I tried this as well. Unfortunately the MgnlContext (ServletContext) is required to build the DialogDefinition ( see UI6DialogCreator ). Is it possible to set the MgnlContext for the thread used in the async command? Kind regards, Björn |
| Comment by Roman Kovařík [ 29/Jun/22 ] |
|
Hi, I wasn't sure about the steps to reproduce so I've tried:
As PropagateMasterContentChangesAction has asynchronous=true, WebContext/AppController are not present in the executing thread.
Let me know if this workaround works for you or share steps to reproduce other use cases so I can test locally or schedule a call. |
| Comment by Björn Eschle [ 29/Jun/22 ] |
|
Hey @Roman, decorating the pages app action with async=false works, which is what we already did as a workaround. However I don't think it is the nicest solution as this can take quite some time if there are a lot of child pages. If you have some time I would like to have a quick meeting to brainstorm a sustainable solution. Regards, Björn |
| Comment by Björn Eschle [ 29/Jun/22 ] |
|
Call with @Roman: Setting the context in the async executor thread can have unwanted side effects, so we should adjust blossom to be able to create the definitions without requiring a context. I'll have another look with @Mika once he is back from his vacation. Idea for ArgumentResolverParameterResolver: Use ApplicationContextAware |
| Comment by Mikaël Geljić [ 14/Jul/22 ] |
|
Thank you both for digging and for the comments. I had a quick look yesterday, until then the dependency between Blossom and Live Copy was never quite clear nor anticipated when we developed either modules. On the one hand, Live Copy uses dialogs as the model to collect properties and sync changes between master and copies. On the other hand, Blossom allows for that model to be dynamic based on a number of context attributes. Meanwhile, if you use Blossom to produce mostly static definitions, it could make sense for us to try to invoke the dialog/tab-factories, except we don't know in advance if a parameter's gonna be missing for injection. Still the decorator fails on the #appliesTo predicate before that. If we support async invocation, I'd like to have an explicit condition to identify it as a legit caller.
As far as I see, the NoSuchComponentException suggests the dialog def should be empty/unprocessed, unclear where the legacy def might come from. Re: ApplicationContextAware, what do you have in mind? |
| Comment by Mikaël Geljić [ 22/Jul/22 ] |
|
beschle Can you please elaborate on steps to reproduce in description? Following same steps as rkovarik, I do observe a few warnings in logs 2022-07-22 17:37:55,151 WARN le.blossom.dialog.DialogCreatorDefinitionDecorator: No AppController available while invoking Blossom dialog with id custom-text-dialog. Not invoking dialog controller. However, content is successfully pushed/copied. |
| Comment by Björn Eschle [ 08/Aug/22 ] |
|
Hey @Mika, sry, for my late response, there was a lot on my plate before my vacation...
The main reason we need the context is to get the AutowireCapableBeanFactory in the DialogCreator, used in BeanParameterResolver to resolve the spring beans. We could implement ApplicationContextAware to get the applicationContext, which than can be used to resolve the beans: (Untested/-compiled)
public class BeanParameterResolver extends ParameterResolver implements ApplicationContextAware { private ApplicationContext applicationContext; public BeanParameterResolver(ParameterResolver parent) { super(parent); } @Override public Object resolveParameter(Class<?> parameterType) { return Optional .ofNullable(applicationContext) .map(context -> (Object)context.getBean(parameterType)) .orElseGet(() -> super.resolveParameter(parameterType)); } @Override public void setApplicationContext(final ApplicationContext applicationContext) throws BeansException { this.applicationContext = applicationContext; } }
Currently the incomplete legacy dialogDefinition is created in the BlossomDialogDefinitionProvider and then gets 'decorated' (if predicate applies) in the DialogCreatorDefinitionDecorator (Although in my opinion it would be nicer to move the 'decoration' to the dialogDefinitionProvider). Kind regards, Björn |