You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@polygene.apache.org by "Niclas Hedhman (JIRA)" <ji...@apache.org> on 2017/05/22 23:25:05 UTC

[jira] [Comment Edited] (POLYGENE-257) Custom IdentityGenerator or Serialization hidden by default assemblers

[ https://issues.apache.org/jira/browse/POLYGENE-257?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16020385#comment-16020385 ]

Niclas Hedhman edited comment on POLYGENE-257 at 5/22/17 11:24 PM:
-------------------------------------------------------------------

Paul's analysis of the situation (from mail thread);

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

Addendum:

- s/polygene-runtime/polygene-support
- Tibor, you could try the branch with your application, I hope it'll
fix your IdentityGenerator issue

was (Author: niclas):
Paul's analysis of the situation (from mail thread);

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.

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.

And returning plain Objects where one would expect services feels weird
to me. Plus it makes getting these core
services injected in composites impossible.

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

> Custom IdentityGenerator or Serialization hidden by default assemblers
> ----------------------------------------------------------------------
>
> Key: POLYGENE-257
> URL: https://issues.apache.org/jira/browse/POLYGENE-257
> Project: Polygene
> Issue Type: Bug
> Reporter: Paul Merlin
> Assignee: Paul Merlin
> Fix For: 3.0.0
>
>
> See https://lists.apache.org/thread.html/9b28a67e75cb8952202d0b0b029de53fc19257b733907408ba20ccde@%3Cdev.polygene.apache.org%3E

--
This message was sent by Atlassian JIRA
(v6.3.15#6346)