Removing support for global "ofbiz-containers.xml"

[Mathieu Lirzin]

Hello,

To extend the containers loaded on startup, it is possible define a
container element either in the global “ofbiz-containers.xml” file or in
the “ofbiz-component.xml” file from each component.

This redundancy adds extra complexity in the startup process for no good
extensibility reason.  The component container loader is more flexible
since it allows developper to add new containers without touching the
framework so it is better to only rely on this option.

In order to simplify the startup process I am proposing to remove the
"ofbiz-containers.xml" config file and hard-code the required base
container which is loading the components which is currently defined in
this file.

What do people think?

I have opened OFBIZ-11100 [1] with some patches demonstrating what I am
exactly proposing.

Thanks.

[1] https://issues.apache.org/jira/browse/OFBIZ-11100

--
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37

[Michael Brohl-3]

Hi Mathieu,

thanks for your proposal. Just a few questions to get the whole picture:

1. do we really have a complexity problem here? The removed code seems
not too complicated or long.

2. what do we lose with the removal? I see one can not only configure
the ComponentContainer class but also set different loaders.

    @all: are there any implementations which make use of this?

Thanks and regards,

Michael


Am 10.06.19 um 19:33 schrieb Mathieu Lirzin:

> Hello,
>
> To extend the containers loaded on startup, it is possible define a
> container element either in the global “ofbiz-containers.xml” file or in
> the “ofbiz-component.xml” file from each component.
>
> This redundancy adds extra complexity in the startup process for no good
> extensibility reason.  The component container loader is more flexible
> since it allows developper to add new containers without touching the
> framework so it is better to only rely on this option.
>
> In order to simplify the startup process I am proposing to remove the
> "ofbiz-containers.xml" config file and hard-code the required base
> container which is loading the components which is currently defined in
> this file.
>
> What do people think?
>
> I have opened OFBIZ-11100 [1] with some patches demonstrating what I am
> exactly proposing.
>
> Thanks.
>
> [1] https://issues.apache.org/jira/browse/OFBIZ-11100
>

[Mathieu Lirzin]

Hello Michael,

Michael Brohl <[hidden email]> writes:

> thanks for your proposal. Just a few questions to get the whole picture:
>
> 1. do we really have a complexity problem here? The removed code seems
> not too complicated or long.

I would claim that we have a complexity problem in OFBiz in
general. Some is essential (the data model) but a lot of it is
accidental (ad-hoc semantics of the XML, Groovy and Freemarker DSLs,
data manipulation across programming layers, complex unused
abstractions...).

I agree that when considering the change I am proposing in isolation it
is not that big in term of complexity reduction. However in the long-run
incremental clean-ups that are reducing the number of unused/unnecessary
moving parts will allow significative improvements in term of
simplification.

It is similar to juggling, getting from 7 balls to 6 balls does not
reduce the difficulty, however when having only 2 or 3 balls it starts
to become manageable for most human.

My underlying goal is to empower most OFBiz integrators/contributors by
allowing them to put every OFBiz elements (containers, entity engine,
service engine, webapps, screen renderer, components) in their brain at
the same time and reason informally about the relations between those
elements without getting headaches.

> 2. what do we lose with the removal? I see one can not only configure
> the ComponentContainer class but also set different loaders.

I see 2 things we are losing:

- The ability to desactivate/replace the "component-container" container
  a.k.a the ‘ComponentContainer’ class which is augmenting the classpath
  to add the “src” directories from the components to be able to among
  other things to find the class implementing the containers defined
  inside components.

- The ability to define a container without associating it to a
  component.

I am not sure to understand what you mean by "set(ting) different
loaders" here? As I understand it, "loaders" are set in the ‘Config’
class from the "ofbiz.start.loaders" property which is defined in
“{load-data,rmi,start,test}.properties” file or by the arguments passed
to the JVM.  Am I missing something?

Thanks.

--
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37

[Michael Brohl-3]

Hi Mathieu,

sorry for the late reply. I meant the loaders configuration attribute
and corresponding loading code:

<container name="component-container" loaders="main,rmi,load-data,test"
class="org.apache.ofbiz.base.container.ComponentContainer"/>

Best regards,

Michael


Am 14.06.19 um 21:30 schrieb Mathieu Lirzin:

> Hello Michael,
>
> Michael Brohl <[hidden email]> writes:
>
>> thanks for your proposal. Just a few questions to get the whole picture:
>>
>> 1. do we really have a complexity problem here? The removed code seems
>> not too complicated or long.
> I would claim that we have a complexity problem in OFBiz in
> general. Some is essential (the data model) but a lot of it is
> accidental (ad-hoc semantics of the XML, Groovy and Freemarker DSLs,
> data manipulation across programming layers, complex unused
> abstractions...).
>
> I agree that when considering the change I am proposing in isolation it
> is not that big in term of complexity reduction. However in the long-run
> incremental clean-ups that are reducing the number of unused/unnecessary
> moving parts will allow significative improvements in term of
> simplification.
>
> It is similar to juggling, getting from 7 balls to 6 balls does not
> reduce the difficulty, however when having only 2 or 3 balls it starts
> to become manageable for most human.
>
> My underlying goal is to empower most OFBiz integrators/contributors by
> allowing them to put every OFBiz elements (containers, entity engine,
> service engine, webapps, screen renderer, components) in their brain at
> the same time and reason informally about the relations between those
> elements without getting headaches.
>
>> 2. what do we lose with the removal? I see one can not only configure
>> the ComponentContainer class but also set different loaders.
> I see 2 things we are losing:
>
> - The ability to desactivate/replace the "component-container" container
>    a.k.a the ‘ComponentContainer’ class which is augmenting the classpath
>    to add the “src” directories from the components to be able to among
>    other things to find the class implementing the containers defined
>    inside components.
>
> - The ability to define a container without associating it to a
>    component.
>
> I am not sure to understand what you mean by "set(ting) different
> loaders" here? As I understand it, "loaders" are set in the ‘Config’
> class from the "ofbiz.start.loaders" property which is defined in
> “{load-data,rmi,start,test}.properties” file or by the arguments passed
> to the JVM.  Am I missing something?
>
> Thanks.
>

[Mathieu Lirzin]

Hello Michael,

Michael Brohl <[hidden email]> writes:

> sorry for the late reply. I meant the loaders configuration attribute
> and corresponding loading code:
>
> <container name="component-container"
> loaders="main,rmi,load-data,test"
> class="org.apache.ofbiz.base.container.ComponentContainer"/>

Ah I see what you meant.  To my understanding, the ‘loaders’ attribute
tells OFBiz to load a container only if there are some elements in
common between actual loaders (defined at runtime in the
‘Config#loaders’ field) and the one declared by the containers, with the
special case where the loaders are empty.  You can take a look at the
‘Container#intersects’ method which implements this logic:

--8<---------------cut here---------------start------------->8---
    private static boolean intersects(Collection<?> a, Collection<?> b) {
        return UtilValidate.isEmpty(a) && UtilValidate.isEmpty(b)
                || !Collections.disjoint(a, b);
    }
--8<---------------cut here---------------end--------------->8---

Since the “loaders” attribute is not specific to the containers defined
in the “ofbiz-containers.xml” file, I don't think there will by any
feature loss in that regard.

I have proposed those changes about a month ago and nobody opposed to
them. As a consequence I would like to move forward by committing the
patches from OFBIZ-11100 [1] on ‘trunk’. Are you opposing that Michael?

Thanks.

[1] https://issues.apache.org/jira/browse/OFBIZ-11100

> Am 14.06.19 um 21:30 schrieb Mathieu Lirzin:
>> Hello Michael,
>>
>> Michael Brohl <[hidden email]> writes:
>>
>>> thanks for your proposal. Just a few questions to get the whole picture:
>>>
>>> 1. do we really have a complexity problem here? The removed code seems
>>> not too complicated or long.
>> I would claim that we have a complexity problem in OFBiz in
>> general. Some is essential (the data model) but a lot of it is
>> accidental (ad-hoc semantics of the XML, Groovy and Freemarker DSLs,
>> data manipulation across programming layers, complex unused
>> abstractions...).
>>
>> I agree that when considering the change I am proposing in isolation it
>> is not that big in term of complexity reduction. However in the long-run
>> incremental clean-ups that are reducing the number of unused/unnecessary
>> moving parts will allow significative improvements in term of
>> simplification.
>>
>> It is similar to juggling, getting from 7 balls to 6 balls does not
>> reduce the difficulty, however when having only 2 or 3 balls it starts
>> to become manageable for most human.
>>
>> My underlying goal is to empower most OFBiz integrators/contributors by
>> allowing them to put every OFBiz elements (containers, entity engine,
>> service engine, webapps, screen renderer, components) in their brain at
>> the same time and reason informally about the relations between those
>> elements without getting headaches.
>>
>>> 2. what do we lose with the removal? I see one can not only configure
>>> the ComponentContainer class but also set different loaders.
>> I see 2 things we are losing:
>>
>> - The ability to desactivate/replace the "component-container" container
>>    a.k.a the ‘ComponentContainer’ class which is augmenting the classpath
>>    to add the “src” directories from the components to be able to among
>>    other things to find the class implementing the containers defined
>>    inside components.
>>
>> - The ability to define a container without associating it to a
>>    component.
>>
>> I am not sure to understand what you mean by "set(ting) different
>> loaders" here? As I understand it, "loaders" are set in the ‘Config’
>> class from the "ofbiz.start.loaders" property which is defined in
>> “{load-data,rmi,start,test}.properties” file or by the arguments passed
>> to the JVM.  Am I missing something?
>>
>> Thanks.
>>
>

--
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37

[Mathieu Lirzin]

Since there was no opposition, I committed the proposed changes.

Mathieu Lirzin <[hidden email]> writes:

> Hello Michael,
>
> Michael Brohl <[hidden email]> writes:
>
>> sorry for the late reply. I meant the loaders configuration attribute
>> and corresponding loading code:
>>
>> <container name="component-container"
>> loaders="main,rmi,load-data,test"
>> class="org.apache.ofbiz.base.container.ComponentContainer"/>
>
> Ah I see what you meant.  To my understanding, the ‘loaders’ attribute
> tells OFBiz to load a container only if there are some elements in
> common between actual loaders (defined at runtime in the
> ‘Config#loaders’ field) and the one declared by the containers, with the
> special case where the loaders are empty.  You can take a look at the
> ‘Container#intersects’ method which implements this logic:
>
>     private static boolean intersects(Collection<?> a, Collection<?> b) {
>         return UtilValidate.isEmpty(a) && UtilValidate.isEmpty(b)
>                 || !Collections.disjoint(a, b);
>     }
>
> Since the “loaders” attribute is not specific to the containers defined
> in the “ofbiz-containers.xml” file, I don't think there will by any
> feature loss in that regard.
>
> I have proposed those changes about a month ago and nobody opposed to
> them. As a consequence I would like to move forward by committing the
> patches from OFBIZ-11100 [1] on ‘trunk’. Are you opposing that Michael?
>
> Thanks.
>
> [1] https://issues.apache.org/jira/browse/OFBIZ-11100
>
>> Am 14.06.19 um 21:30 schrieb Mathieu Lirzin:
>>> Hello Michael,
>>>
>>> Michael Brohl <[hidden email]> writes:
>>>
>>>> thanks for your proposal. Just a few questions to get the whole picture:
>>>>
>>>> 1. do we really have a complexity problem here? The removed code seems
>>>> not too complicated or long.
>>> I would claim that we have a complexity problem in OFBiz in
>>> general. Some is essential (the data model) but a lot of it is
>>> accidental (ad-hoc semantics of the XML, Groovy and Freemarker DSLs,
>>> data manipulation across programming layers, complex unused
>>> abstractions...).
>>>
>>> I agree that when considering the change I am proposing in isolation it
>>> is not that big in term of complexity reduction. However in the long-run
>>> incremental clean-ups that are reducing the number of unused/unnecessary
>>> moving parts will allow significative improvements in term of
>>> simplification.
>>>
>>> It is similar to juggling, getting from 7 balls to 6 balls does not
>>> reduce the difficulty, however when having only 2 or 3 balls it starts
>>> to become manageable for most human.
>>>
>>> My underlying goal is to empower most OFBiz integrators/contributors by
>>> allowing them to put every OFBiz elements (containers, entity engine,
>>> service engine, webapps, screen renderer, components) in their brain at
>>> the same time and reason informally about the relations between those
>>> elements without getting headaches.
>>>
>>>> 2. what do we lose with the removal? I see one can not only configure
>>>> the ComponentContainer class but also set different loaders.
>>> I see 2 things we are losing:
>>>
>>> - The ability to desactivate/replace the "component-container" container
>>>    a.k.a the ‘ComponentContainer’ class which is augmenting the classpath
>>>    to add the “src” directories from the components to be able to among
>>>    other things to find the class implementing the containers defined
>>>    inside components.
>>>
>>> - The ability to define a container without associating it to a
>>>    component.
>>>
>>> I am not sure to understand what you mean by "set(ting) different
>>> loaders" here? As I understand it, "loaders" are set in the ‘Config’
>>> class from the "ofbiz.start.loaders" property which is defined in
>>> “{load-data,rmi,start,test}.properties” file or by the arguments passed
>>> to the JVM.  Am I missing something?
>>>
>>> Thanks.
>>>
>>

--
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37