polygene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Niclas Hedhman <nic...@hedhman.org>
Subject Re: Default assemblers (Re: Issue with null association)
Date Mon, 22 May 2017 23:20:38 GMT
Very good analysis, Paul!

And you are right about the difference between "Module Default" and
"Fallback" differentiation... And what matters now is the way to implement
it. I am not convinced that a default layer is the way to go, although it
seems to be the most easy way to implement it. As a start, I have no
recollection that defined semantics of multiple reachable layers are
anything but "Ambiguous Type". In Visibility.layer, multiple visible
Composites in same layer would result AmbiguousTypeException, and I suspect
that currently that is also the case if found in multiple layers. And for
your approach to work, that would need to be changed to some type of
ordering/priority of layers, or special handling of the polygene-runtime
layer. Neither seems tempting to me right now (before coffee). That change
is also non-reversible in a compatible manner...

What other choices are there?

We could remove the said default types, and instead do the equivalent in
AbstractPolygeneTest (maybe one of the super classes), where it is really
convenient and doesn't impact production code. That would be the quickest
route for a 3.0 and keep the door open for compatible changes in 3.1

We could introduce something like;

    module.asFallbackModule().visibleIn( Visibility.application);


where the semantics are; If no type is found in layer, check the default
module of the layer. If no type found in underlying layers, check default
modules in those underlying layers.

But then again, WHY? Why have two priority levels in TypeLookUp? Why not
have N levels of priority? And that question to me sounds like a slippery
slope, and I would prefer to not go there.


So, right now (half way through coffee cup), I don't think we need anything
conceptually new, just

    1. Remove IdentityGenerator and Serialization from defaultAssemblers
    2. Change AbstractPolygeneTest to add those by default.


Cheers
Niclas

On Mon, May 22, 2017 at 11:38 PM, Paul Merlin <paulmerlin@apache.org> wrote:

> Gang,
>
> Le 2017-05-21 19:45, Tibor Mlynarik a écrit :
>
>> I have another one. Is it ok to report here or better create JIRA issue ?
>>
>
> What Niclas said, JIRAs are better. Keep them coming Tibor!
>
>
> My custom IdentityGenerator ( inherited from layer hierarchy ) is
>> probably overwritten with default one.
>> My guess is that ModuleAssemblyImpl.addDefaultAssemblers() is not
>> respecting hierarchy inheritance.
>>
>> I have custom IdentityGenerator declared in BottomLayer but used in
>> UpperLayer.
>>
>
>
> The issue is not about type hierarchy but structure and visibility.
>
> Default assemblers are added to ALL modules for
> - IdentityGenerator,
> - Serialization
> - and UnitOfWorkFactory
> if no service assignable to each was assembled.
>
> This makes sense for UnitOfWorkFactory, UnitOfWorkFactory is bound to its
> module and one always create an uow from a given
> module.
> But it doesn't for the two other types for which one will want to get the
> closest in the app structure instead.
>
> I think this mixes two use cases.
>
> Assembly of all modules must contain a service of a given type. That's the
> UnitOfWorkFactory case and current
> "default assemblers" do the job.
>
> If a service of a given type cannot be found, then fallback to a framework
> default. That's the IdentityGenerator and
> Serialization cases. Both because they are core api services (e.g. in
> Module) and almost all extensions and a bunch
> of libraries use them. But assembling them in all modules prevent
> reasonable assemblies, as I see it it's a
> regression. And requiring users to assemble some special services in all
> their modules feels wrong.
>
> In 2.1, serialization and metrics have a fallback, identity generation
> doesn't:
> - Module.identityGenerator() throws NoSuchServiceException if not found
> - Module.valueSerialization() instantiate OrgJsonValueSerialization
> directly and cache
> - Module.metricsProvider() is package protected, it instantiate
> MetricsProviderAdapter directly and cache
>
> On develop: metrics have a fallback, the others don't:
> - Module.identityGenerator() throws NoSuchServiceException if not found,
> but is always found (default assemblers)
> - Module.serialization() throws NoSuchServiceException if not found, but
> is always found (default assemblers)
> - Module.metricsProvider() instantiate MetricsProviderAdapter directly and
> cache
> but the first two are assembled in all modules anyway.
>
> A noop implementation is a good fit for metrics.
> UuidGeneratorMixin is core and a simple class so it could easily be
> instantiated directly as a fallback.
> Our default serialization requires composition but it could be reworked to
> be instantiatable, requiring injection.
>
> But these services are not bound to their assembly module, so they could
> be shared across modules.
> Declaration in all modules is cheap, but it pollutes the application model
> in all modules, which is not nice.
>
> And returning plain Objects where one would expect services feels weird to
> me. Plus it makes getting these core
> services injected in composites impossible.
>
> If my assembly misses these core services, it's very easy to fall into the
> cryptic assembly error trap.
> In most cases you want to fix the assembly issue holistically according to
> your application needs.
> That's how 2.x works.
>
> The intent of "default assemblers" for IdentityGenerator and Serialization
> is convenience.
> When starting an application or writing Polygene tests one wants simple
> defaults.
> But the way they are done now gets in the way when assembling non trivial
> applications.
>
> One way to provide these fallbacks while respecting the structure and
> visibility concept, for the sake of least
> surprise, would be to add an arbitrary layer with these default services
> with application visibility and make all
> other "leaf" layers use this arbitrary layer. Effectively making these
> default services visible to the whole
> application while allowing any custom implementations assembled to
> override them.
>
> We could also add a way to opt-out of this during assembly, for a stricter
> structure.
>
> Metrics could then work the same way for the sake of least surprise again,
> still providing a noop implementation.
>
> I gave the idea a try in the `pm/bootstrap/support-layer` branch, it
> passes the whole test suite.
> I opened a GitHub PR so it's easier to look at:
> https://github.com/apache/polygene-java/pull/6/files
>
> The controversial aspect of this solution is that it automatically adds an
> arbitrary layer to support Polygene
> core services that an application can override.
> But I feel that's far less invasive than adding several assembly
> declarations to all modules.
> And it makes structure reflect the real intent.
>
> Here are some Envisage screenshots to illustrate the change:
> - current `develop` behaviour: https://pasteboard.co/9hJfzPf4r.png
> - with the added `polygene-runtime` layer: https://pasteboard.co/9hJJnHL4
> h.png
> - with explicit assembly of base services in the application assembly, the
> `polygene-runtime` layer is not needed so
> it is not added to the assembly: https://pasteboard.co/9hKatWHMc.png
>
> WDYAT?
>
>


-- 
Niclas Hedhman, Software Developer
http://polygene.apache.org - New Energy for Java

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message