You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@polygene.apache.org by Paul Merlin <pa...@apache.org> on 2017/05/22 15:38:51 UTC
Default assemblers (Re: Issue with null association)
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/9hJJnHL4h.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?
Re: Default assemblers (Re: Issue with null association)
Posted by Paul Merlin <pa...@apache.org>.
Hi Tibor,
Tibor Mlynarik a écrit :
> Paul,
>
> I did workaround with commenting out
> //ModuleAssemblyImpl:127
> //defaultAssemblers.put( IdentityGenerator.class, new DefaultIdentityGeneratorAssembler() );
>
> that one line change helped me, so I am continuing with porting to 3.0 ( currently on Identity+HasIdentity changes )
>
> I am very interested with Modules/Layer visibility topic so thanks for detailed explanation.
> But I have to take deeper look into it yet.
>
> Let me know if it will be still useful to try with branch pm/bootstrap/support-layer .
No need to test the pm/bootstrap/support-layer branch, we decided
against it and the branch will be deleted.
The develop branch now contains the proper fix. Default
IdentityGenerator and Serialization are not assembled on all modules
anymore. ModuleAssembly.defaultServices() is introduced as a
convenience.See POLYGENE-257
Cheers
/Paul
Re: Default assemblers (Re: Issue with null association)
Posted by Niclas Hedhman <ni...@hedhman.org>.
On Tue, May 23, 2017 at 8:51 PM, Tibor Mlynarik <ti...@gmail.com>
wrote:
>
> Let me know if it will be still useful to try with branch
> pm/bootstrap/support-layer .
The more I think about it, the more I dislike the idea of a "floor layer"
permeating everywhere. We would need to make it read-only as well, since
otherwise people might be tempted to toss all kinds of less thought-out
stuff in there, and degrading their own work.
Unless someone can convince me otherwise, I think we won't go forward with
it.
Cheers
--
Niclas Hedhman, Software Developer
http://polygene.apache.org - New Energy for Java
Re: Default assemblers (Re: Issue with null association)
Posted by Tibor Mlynarik <ti...@gmail.com>.
Paul,
I did workaround with commenting out
//ModuleAssemblyImpl:127
//defaultAssemblers.put( IdentityGenerator.class, new DefaultIdentityGeneratorAssembler() );
that one line change helped me, so I am continuing with porting to 3.0 ( currently on Identity+HasIdentity changes )
I am very interested with Modules/Layer visibility topic so thanks for detailed explanation.
But I have to take deeper look into it yet.
Let me know if it will be still useful to try with branch pm/bootstrap/support-layer .
cheers,
- Tibor
> On May 22, 2017, at 5:43 PM, Paul Merlin <pa...@apache.org> wrote:
>
> Addendum:
>
> - s/polygene-runtime/polygene-support
> - Tibor, you could try the branch with your application, I hope it'll fix your IdentityGenerator issue
>
>
> Le 2017-05-22 17:38, Paul Merlin a écrit :
>> 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/9hJJnHL4h.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?
>
Re: Default assemblers (Re: Issue with null association)
Posted by Paul Merlin <pa...@apache.org>.
Addendum:
- s/polygene-runtime/polygene-support
- Tibor, you could try the branch with your application, I hope it'll
fix your IdentityGenerator issue
Le 2017-05-22 17:38, Paul Merlin a écrit :
> 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/9hJJnHL4h.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?
Re: Default assemblers (Re: Issue with null association)
Posted by Niclas Hedhman <ni...@hedhman.org>.
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 <pa...@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