[MGNLUI-3458] Backwards compatibility in DialogDefinitionRegistry and providers Created: 09/Jun/15  Updated: 01/Jul/15  Resolved: 26/Jun/15

Status: Closed
Project: Magnolia UI
Component/s: dialogs
Affects Version/s: 5.4
Fix Version/s: 5.4

Type: New Feature Priority: Neutral
Reporter: Tobias Mattsson Assignee: Magnolia International
Resolution: Fixed Votes: 0
Labels: blossom, config, dialog
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
causality
is causing BLOSSOM-214 Adapt to changed LocaleProvider in Ma... Closed
is causing BLOSSOM-215 Adapt to changes in template and dial... Closed
relation
is related to MGNLUI-3335 Implement new registry apis for dialogs Closed
is related to MAGNOLIA-6203 Inject the Context to ContextLocalePr... Closed
Template:
Acceptance criteria:
Empty
Date of First Response:

 Description   

Priority set to critical because it prevents using Blossom with 5.4.

Background

Blossom creates its dialogs on demand when they're needed. Every time a dialog is to be displayed its definition is created from scratch. This allows the dialog creation code to adapt to the current context, for instance who's using it and what content it is being used for. The context is captured by a custom FormDialogPresenter and picked up by the dialog provider. It is therefore impossible to get the definition of a blossom dialog without a context. The definition can only be acquired from the provider when called from the custom FormDialogPresenter.

Magnolia 5.4

In 5.4 the whole registry and provider concept changes. These changes break Blossom because:

  • There's no longer a getPresenterClass() on the provider. It is instead read directly from the definition, which cannot be acquired. This is done in FormDialogPresenterFactoryImpl and in DialogDefinitionRegistry.
  • There's no way to register a legacy dialog definition provider. There needs to be a register() method in DialogDefintionRegistry for the legacy provider interface. The method should wrap the legacy interface to adapt it to the new interface. The new provider keeps an in memory representation of the definition which is acquired when the provider is created. This must not happen when registering a legacy provider.

In 5.4 TemplateDefinitionRegistry has a backwards compatible register method. It works fine for Blossom because its template definitions are created once at startup and then never changed. Having said that, its design makes it impossible for implementations to have a provider that created the definition on demand.

I have implemented very hackish workarounds for these issues and will commit them on branches. Should at least serve as some kind of example.



 Comments   
Comment by Mikaël Geljić [ 15/Jun/15 ]

Linking to the related ticket in UI re: refactoring of the DialogDefinitionRegistry.
Linking as well to the other introduced API change re: LocaleProvider. We'll restore the interface here as well.

Comment by Mikaël Geljić [ 15/Jun/15 ]

pushed current progress respectively to branches:

  • fix/DialogDefinitionRegistry-compatibility (ui)
  • fix/LocaleProvider-compatibility (main)
    • to be checked what we can do there, in particular because we'd likely keep LocaleProvider extending Provider<Locale>
    • main blossom incompatibility seems to lie around MagnoliaMessageSource;
      backporting a FixedLocaleProvider to 5.3 series could be an option (thus using it instead of the StaticLocaleProvider in there)
    • to be checked as well: re-introduction of ContextLocaleProvider's no-arg ctor.
Comment by Tobias Mattsson [ 15/Jun/15 ]

Regarding MagnoliaMessageSource this would work in both 5.3 and 5.4 if LocaleProvider remains an interface.

private static class StaticLocaleProvider implements LocaleProvider {

    private final Locale locale;

    public StaticLocaleProvider(Locale locale) {
        this.locale = locale;
    }

    @Override
    public Locale getLocale() {
        return locale;
    }

    public Locale get() {
        return getLocale();
    }
}

As for ContextLocaleProvider I could change the code to use ComponentProvider.newInstance which would work in both 5.3 och 5.4. So that's not a problem.

Comment by Tobias Mattsson [ 15/Jun/15 ]

Regarding DialogDefinitionRegistry I have now pushed my hackish workarounds on branch MGNLUI-3458.

https://git.magnolia-cms.com/gitweb/?p=magnolia_ui.git;a=commit;h=74e55dd35e799a1dfa59fddbb832cd600b642d69

It solves these problems:

  • Uses a custom provider implementation instead of using builders so it can be found behind wrappers in front of it.
  • The presenter class is acquired without getting the definition.
  • The custom provider doesn't keep a definition in memory, instead calls the legacy provider every time.

I know this hacks around the new design to reestablish the earlier design exactly. Hopefully we can find a more elegant solution.

I'm very curious to see how this behaves in the new tooling. Does it just break down horribly and burn to the ground? Maybe it should be presented using some kind of placeholder and the provider should return that when called from anywhere but BlossomFormDialogPresenter.

Mika could you give me some hints on how to test this with the tooling?

Comment by Tobias Mattsson [ 26/Jun/15 ]

I've lowered the priority to neutral because after investigating this more I've realized the best option is to do a Blossom 3.1 release for 5.4 which adopts the new API both for registries + providers and for locale provider.

To fit the new registry api Blossom will return an empty dialog when not called from BlossomFormDialogPresenter. The definition returned then will have presenterClass set so it can be acquired safely.

Created BLOSSOM-214 and BLOSSOM-215 for these changes.

Comment by Magnolia International [ 26/Jun/15 ]

Merged in the fix/DialogDefinitionRegistry-compatibility branch to provide a "compatibility" method in DialogRegistry, similar to what we did in TemplateRegistry.
Since Blossom is doing a new release, fix/LocaleProvider-compatibility branch is not needed.

Comment by Michael Mühlebach [ 29/Jun/15 ]

The code for backward compatibility in DialogDefinitionRegistry and TemplateDefinitionRegistry could be improved because it is duplicated and uses assumptions about the definition Id we want to get rid of.
But as it is already deprecated code and will be removed soon it makes no sense to invest in it ...

Generated at Mon Feb 12 09:06:49 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.