Design-Flaw in AbstractDBMediatorFactory and sub classes?

[Hubert, Eric-2]

Some parts of this message have been removed. Learn more about Nabble's security policy.

Hi Synapse-Devs,

 

While having a glance on the way Synapse creates its internal memory representation of the Synapse configuration I think I stumbled across a design flaw in the AbstractDBMediatorFactory and its sub classes.

 

Please correct me, if I’m wrong on the following assumptions:

-          the process of building the in-memory object representation of the Synapse configuration has to be separated from the initialisation of mediators

-          it is not the responsibility of mediator factories to initialize mediators, but to create the mediator (object representation) of the mediator configuration as part of the Synapse configuration

-          this allows one to create a configuration model from a synapse configuration (e.g. stored in an XML file) without the need to have an initialized environment running

-          this separation is also manifested in different execution phases within the startup process:
1) create Synapse configuration
2) create Synapse environment
3) initialize Synapse configuration with Synapse environment (calls init() on all config elements supporting some kind of lifecycle)

 

Unfortunately I noticed that my assumptions are wrong, although I honestly think they SHOULD be correct. It is currently not possible to create a Synapse Configuration from a synapse.xml which makes use of mediators which are created based on the AbstractDBMediatorFactory.

The reason is, mediator creation and initialization are mixed and the factories take responsibility of both. During the creation defined datasources are build (including datasource lookup which can only work, if the DataSourceHelper has been initialized).

I think the creation of the synapse configuration model itself should not depend on external dependencies. The initialization should rather be done in the init-Method of the AbstractDBMediator, instead of the create-method of the AbstractDBMediatorFactory.

 

What do others think?

 

Here is the current stacktrace, if someone tries to build a synapse configuration with standalone code like this:

 

SynapseConfiguration synapseConfiguration = scb.getConfiguration(synapseConfigFileName);

 

org.apache.synapse.commons.SynapseCommonsException: DataSourceHelper has not been initialized, it requires to be initialized at org.apache.synapse.commons.datasource.DataSourceHelper.assertInitialized(DataSourceHelper.java:108)      at org.apache.synapse.commons.datasource.DataSourceHelper.getRepositoryBasedDataSourceFinder(DataSourceHelper.java:122) at org.apache.synapse.config.xml.AbstractDBMediatorFactory.lookupDataSource(AbstractDBMediatorFactory.java:145) at org.apache.synapse.config.xml.AbstractDBMediatorFactory.buildDataSource(AbstractDBMediatorFactory.java:121)

...

 

Regards,

   Eric

 

[indika kumara]

> I think the creation of the synapse configuration model itself should not
> depend on external dependencies. The initialization should rather be done in
> the init-Method of the AbstractDBMediator, instead of the create-method of
> the AbstractDBMediatorFactory.

+1

Thanks
Indika

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

[Supun Kamburugamuva]

+1, this makes sense. It would be really great if we could enforce this behavior as well.

Supunn..

On Wed, Nov 4, 2009 at 1:52 PM, Hubert, Eric <Eric.Hubert@...> wrote:

Hi Synapse-Devs,

 

While having a glance on the way Synapse creates its internal memory representation of the Synapse configuration I think I stumbled across a design flaw in the AbstractDBMediatorFactory and its sub classes.

 

Please correct me, if I’m wrong on the following assumptions:

-          the process of building the in-memory object representation of the Synapse configuration has to be separated from the initialisation of mediators

-          it is not the responsibility of mediator factories to initialize mediators, but to create the mediator (object representation) of the mediator configuration as part of the Synapse configuration

-          this allows one to create a configuration model from a synapse configuration (e.g. stored in an XML file) without the need to have an initialized environment running

-          this separation is also manifested in different execution phases within the startup process:
1) create Synapse configuration
2) create Synapse environment
3) initialize Synapse configuration with Synapse environment (calls init() on all config elements supporting some kind of lifecycle)

 

Unfortunately I noticed that my assumptions are wrong, although I honestly think they SHOULD be correct. It is currently not possible to create a Synapse Configuration from a synapse.xml which makes use of mediators which are created based on the AbstractDBMediatorFactory.

The reason is, mediator creation and initialization are mixed and the factories take responsibility of both. During the creation defined datasources are build (including datasource lookup which can only work, if the DataSourceHelper has been initialized).

I think the creation of the synapse configuration model itself should not depend on external dependencies. The initialization should rather be done in the init-Method of the AbstractDBMediator, instead of the create-method of the AbstractDBMediatorFactory.

 

What do others think?

 

Here is the current stacktrace, if someone tries to build a synapse configuration with standalone code like this:

 

SynapseConfiguration synapseConfiguration = scb.getConfiguration(synapseConfigFileName);

 

org.apache.synapse.commons.SynapseCommonsException: DataSourceHelper has not been initialized, it requires to be initialized at org.apache.synapse.commons.datasource.DataSourceHelper.assertInitialized(DataSourceHelper.java:108)      at org.apache.synapse.commons.datasource.DataSourceHelper.getRepositoryBasedDataSourceFinder(DataSourceHelper.java:122) at org.apache.synapse.config.xml.AbstractDBMediatorFactory.lookupDataSource(AbstractDBMediatorFactory.java:145) at org.apache.synapse.config.xml.AbstractDBMediatorFactory.buildDataSource(AbstractDBMediatorFactory.java:121)

...

 

Regards,

   Eric

 

--
Software Engineer, WSO2 Inc
http://wso2.org
supunk.blogspot.com

[Hubert, Eric-2]

> > I think the creation of the synapse configuration model itself should
> not
> > depend on external dependencies. The initialization should rather be
> done in
> > the init-Method of the AbstractDBMediator, instead of the create-method
> of
> > the AbstractDBMediatorFactory.
>
> +1
Thanks for your feedback Indika. Will wait for more feedback and if there is consensus on this create a JIRA for the needed code changes. I also may find a bit of time at the weekend to come up with a patch.

Regards,
   Eric

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

[Hubert, Eric-2]

Some parts of this message have been removed. Learn more about Nabble's security policy.

Hi all,

 

I have created https://issues.apache.org/jira/browse/SYNAPSE-594 and attached a patch to move the logic to lookup/create data sources from the mediator creation to the init-phase of the db mediators.

I did not find time to test the changes intensively. Anyway existing tests are passing…

 

I tried to improve the existing code, although it is still not optimal.

 

Regards,

   Eric

---

From: Supun Kamburugamuva [mailto:supun06@...]
Sent: Thursday, November 05, 2009 9:14 AM
To: dev@...
Subject: Re: Design-Flaw in AbstractDBMediatorFactory and sub classes?

 

+1, this makes sense. It would be really great if we could enforce this behavior as well.

Supunn..

On Wed, Nov 4, 2009 at 1:52 PM, Hubert, Eric <Eric.Hubert@...> wrote:

Hi Synapse-Devs,

 

While having a glance on the way Synapse creates its internal memory representation of the Synapse configuration I think I stumbled across a design flaw in the AbstractDBMediatorFactory and its sub classes.

 

Please correct me, if I’m wrong on the following assumptions:

-          the process of building the in-memory object representation of the Synapse configuration has to be separated from the initialisation of mediators

-          it is not the responsibility of mediator factories to initialize mediators, but to create the mediator (object representation) of the mediator configuration as part of the Synapse configuration

-          this allows one to create a configuration model from a synapse configuration (e.g. stored in an XML file) without the need to have an initialized environment running

-          this separation is also manifested in different execution phases within the startup process:
1) create Synapse configuration
2) create Synapse environment
3) initialize Synapse configuration with Synapse environment (calls init() on all config elements supporting some kind of lifecycle)

 

Unfortunately I noticed that my assumptions are wrong, although I honestly think they SHOULD be correct. It is currently not possible to create a Synapse Configuration from a synapse.xml which makes use of mediators which are created based on the AbstractDBMediatorFactory.

The reason is, mediator creation and initialization are mixed and the factories take responsibility of both. During the creation defined datasources are build (including datasource lookup which can only work, if the DataSourceHelper has been initialized).

I think the creation of the synapse configuration model itself should not depend on external dependencies. The initialization should rather be done in the init-Method of the AbstractDBMediator, instead of the create-method of the AbstractDBMediatorFactory.

 

What do others think?

 

Here is the current stacktrace, if someone tries to build a synapse configuration with standalone code like this:

 

SynapseConfiguration synapseConfiguration = scb.getConfiguration(synapseConfigFileName);

 

org.apache.synapse.commons.SynapseCommonsException: DataSourceHelper has not been initialized, it requires to be initialized at org.apache.synapse.commons.datasource.DataSourceHelper.assertInitialized(DataSourceHelper.java:108)      at org.apache.synapse.commons.datasource.DataSourceHelper.getRepositoryBasedDataSourceFinder(DataSourceHelper.java:122) at org.apache.synapse.config.xml.AbstractDBMediatorFactory.lookupDataSource(AbstractDBMediatorFactory.java:145) at org.apache.synapse.config.xml.AbstractDBMediatorFactory.buildDataSource(AbstractDBMediatorFactory.java:121)

...

 

Regards,

   Eric

 

--
Software Engineer, WSO2 Inc
http://wso2.org
supunk.blogspot.com