Discussion: Persistence module without tests

[Christian Schneider <ch...@die-schneider.net>]

Hi all,

I am currently working on
https://issues.apache.org/jira/browse/SYNCOPE-241 .

I succeeded in moving the production classes for the persistence layer
to a separate module. One interesting last problem I solved there was
that spring did not auto detect the entities. I found that the
peristence.xml has to be in the same jar as the entities. Only then will
spring correctly detect the classes.

When trying to also move the persistence tests into the persistence
module I faced a lot of problems as our current build only works when
the tests are in one module. (See below).
I do not see a good way to move the persistence tests right now. With
some bigger changes in the way syncope builds and tests this may be
possible but it would be to big for me to tackle at the moment.

So I see two alternatives:

1. Keep persistence inside core but try to keep it as a separate
architectural layer. This would mean to avoid cycles between persistence
and the rest of core "by hand".
2. Only move the persistence classes to the persistence module but not
the tests. I created a patch for this in the issue above.

I personally prefer option 2. While it would be better to have the tests
in persistence too it is a good step in the right direction and already
has some benefits:
- It is impossible to accidently create new dependencies from
persistence classes to other parts of core. So persistence is guaranteed
to be a inner core of syncope that can be understood on its own
- The jpa enhancement happens in persistence. So when you work at core
you can leave the persistence project closed and this way eclipse will
not accidently overwrite the enhanced classes with the original classes
and make your tests fail

So what do you guys think about this?

Christian

----
The problems I faced:

- Persistence test as well as the other core tests have to support a
wide range of databases. So this results in a lot of duplication between
the poms of core and persistence
- The tests require a lot of test resources that are partly changed
during the build depending on the profile. So this could also lead to a
lot of duplication
- I tried to move the test resources into a separate test-resources
project. There these resources would then be in src/main/resources to go
into the jar to be available to core and persistence. This separate
project alone is already a bit ugly. Additionally we have some resources
like persistence.properties also in src/main/resources of core. The
current tests rely on the fact that src/test/resources override
src/main/resources. With a separate test-resources project this does not
work anymore.

RE: Discussion: Persistence module without tests

[Andrei Shakirin <as...@talend.com>]

Hi,

>From my perspective, we can stop activities related to SYNCOPE-241 with option (1) for now. I find refactoring already made by Christian very useful and important.

In perspective, let see if we will have some real requirements resulting to split core module, for example:
- alternative persistence implementation (Hybernate, EclipseLink, etc);
- clean OSGi design (but on the first step by OSGi support core can still stay monolithic);
- alternative remote access (no REST based);
- etc

As soon as such requirements are accepted, let resolve the problems mentioned by Christian and split core module including the tests.
I think it will be not artificial, but more requirements and features driven approach.

Regards,
Andrei.

> -----Original Message-----
> From: Christian Schneider [mailto:cschneider111@gmail.com] On Behalf Of
> Christian Schneider
> Sent: Dienstag, 15. Januar 2013 15:59
> To: dev@syncope.apache.org
> Subject: Discussion: Persistence module without tests
> 
> Hi all,
> 
> I am currently working on
> https://issues.apache.org/jira/browse/SYNCOPE-241 .
> 
> I succeeded in moving the production classes for the persistence layer to a
> separate module. One interesting last problem I solved there was that spring
> did not auto detect the entities. I found that the peristence.xml has to be in
> the same jar as the entities. Only then will spring correctly detect the classes.
> 
> When trying to also move the persistence tests into the persistence module I
> faced a lot of problems as our current build only works when the tests are in
> one module. (See below).
> I do not see a good way to move the persistence tests right now. With some
> bigger changes in the way syncope builds and tests this may be possible but it
> would be to big for me to tackle at the moment.
> 
> So I see two alternatives:
> 
> 1. Keep persistence inside core but try to keep it as a separate architectural
> layer. This would mean to avoid cycles between persistence and the rest of
> core "by hand".
> 2. Only move the persistence classes to the persistence module but not the
> tests. I created a patch for this in the issue above.
> 
> I personally prefer option 2. While it would be better to have the tests in
> persistence too it is a good step in the right direction and already has some
> benefits:
> - It is impossible to accidently create new dependencies from persistence
> classes to other parts of core. So persistence is guaranteed to be a inner core
> of syncope that can be understood on its own
> - The jpa enhancement happens in persistence. So when you work at core
> you can leave the persistence project closed and this way eclipse will not
> accidently overwrite the enhanced classes with the original classes and make
> your tests fail
> 
> So what do you guys think about this?
> 
> Christian
> 
> ----
> The problems I faced:
> 
> - Persistence test as well as the other core tests have to support a wide
> range of databases. So this results in a lot of duplication between the poms
> of core and persistence
> - The tests require a lot of test resources that are partly changed during the
> build depending on the profile. So this could also lead to a lot of duplication
> - I tried to move the test resources into a separate test-resources project.
> There these resources would then be in src/main/resources to go into the jar
> to be available to core and persistence. This separate project alone is already
> a bit ugly. Additionally we have some resources like persistence.properties
> also in src/main/resources of core. The current tests rely on the fact that
> src/test/resources override src/main/resources. With a separate test-
> resources project this does not work anymore.

Re: Discussion: Persistence module without tests

[Fabio Martelli <fa...@gmail.com>]

Il giorno 16/gen/2013, alle ore 09.59, Francesco Chicchiriccò ha scritto:

> On 15/01/2013 15:58, Christian Schneider wrote:
>> Hi all,
>> 
>> I am currently working on
>> https://issues.apache.org/jira/browse/SYNCOPE-241 .
>> 
>> I succeeded in moving the production classes for the persistence layer to a separate module. One interesting last problem I solved there was that spring did not auto detect the entities. I found that the peristence.xml has to be in the same jar as the entities. Only then will spring correctly detect the classes.
>> 
>> When trying to also move the persistence tests into the persistence module I faced a lot of problems as our current build only works when the tests are in one module. (See below).
>> I do not see a good way to move the persistence tests right now. With some bigger changes in the way syncope builds and tests this may be possible but it would be to big for me to tackle at the moment.
>> 
>> So I see two alternatives:
>> 
>> 1. Keep persistence inside core but try to keep it as a separate architectural layer. This would mean to avoid cycles between persistence and the rest of core "by hand".
>> 2. Only move the persistence classes to the persistence module but not the tests. I created a patch for this in the issue above.
>> 
>> I personally prefer option 2. While it would be better to have the tests in persistence too it is a good step in the right direction and already has some benefits:
>> - It is impossible to accidently create new dependencies from persistence classes to other parts of core. So persistence is guaranteed to be a inner core of syncope that can be understood on its own
>> - The jpa enhancement happens in persistence. So when you work at core you can leave the persistence project closed and this way eclipse will not accidently overwrite the enhanced classes with the original classes
>> and make your tests fail
>> 
>> So what do you guys think about this?
> 
> Christian,
> I don't find the option (2) acceptable: moving out persistence classes without associated tests is far to be a best-practice IMHO.
> 
> As already reported in some of my comments to SYNCOPE-241, while most refactoring is useful and has introduced more order and cleanings, I also find a portion of the latest modifications as artificial and with only purpose to force some additional layering. But excessive layering is bad as poor layering.
> 
> As I've reported since the beginning of this issue, I don't think that ease of working with a particular IDE (Eclipse) can drive the development.

I develop with Netbeans and I have no problem building Syncope.
It would be interesting to know how another IDE (IntelliJ IDEA, for example) works with the current configuration.
From my PPOV, if we have trouble just with Eclipse I have to agree with Francesco.

Is there someone that tried to build Syncope with an IDE different from Netbeans and Eclipse?

> My proposal here is then: let's leave things are they are currently committed, close SYNCOPE-241 and devote effort to the other open issues, especially the ones missing for 1.1.0.
> 
> WDYT?
> 
> (See below my inline comments to the problems you have faced).
> Regards.
> 
>> ----
>> The problems I faced:
>> 
>> - Persistence test as well as the other core tests have to support a wide range of databases. So this results in a lot of duplication between the poms of core and persistence
> 
> This can be handled somehow, not dramatic. But it needs quite some work, I recognize this.
> 
>> - The tests require a lot of test resources that are partly changed during the build depending on the profile. So this could also lead to a lot of duplication
> 
> This is part of the item above.
> 
>> - I tried to move the test resources into a separate test-resources project. There these resources would then be in src/main/resources to go into the jar to be available to core and persistence. This separate project alone is already a bit ugly. Additionally we have some resources like persistence.properties also in src/main/resources of core. The current tests rely on the fact that src/test/resources override src/main/resources. With a separate test-resources project this does not work anymore.
> 
> I agree with you, it's ugly :-)

Agree, really ugly.

> -- 
> Francesco Chicchiriccò
> 
> ASF Member, Apache Syncope PMC chair, Apache Cocoon PMC Member
> http://people.apache.org/~ilgrosso/
> 

Re: Discussion: Persistence module without tests

[Francesco Chicchiriccò <il...@apache.org>]

On 15/01/2013 15:58, Christian Schneider wrote:
> Hi all,
>
> I am currently working on
> https://issues.apache.org/jira/browse/SYNCOPE-241 .
>
> I succeeded in moving the production classes for the persistence layer to a separate module. One interesting last problem I solved there was that spring did not auto detect the entities. I found that the peristence.xml has to be in the same jar as the entities. Only then will spring correctly detect the classes.
>
> When trying to also move the persistence tests into the persistence module I faced a lot of problems as our current build only works when the tests are in one module. (See below).
> I do not see a good way to move the persistence tests right now. With some bigger changes in the way syncope builds and tests this may be possible but it would be to big for me to tackle at the moment.
>
> So I see two alternatives:
>
> 1. Keep persistence inside core but try to keep it as a separate architectural layer. This would mean to avoid cycles between persistence and the rest of core "by hand".
> 2. Only move the persistence classes to the persistence module but not the tests. I created a patch for this in the issue above.
>
> I personally prefer option 2. While it would be better to have the tests in persistence too it is a good step in the right direction and already has some benefits:
> - It is impossible to accidently create new dependencies from persistence classes to other parts of core. So persistence is guaranteed to be a inner core of syncope that can be understood on its own
> - The jpa enhancement happens in persistence. So when you work at core you can leave the persistence project closed and this way eclipse will not accidently overwrite the enhanced classes with the original classes
> and make your tests fail
>
> So what do you guys think about this?

Christian,
I don't find the option (2) acceptable: moving out persistence classes 
without associated tests is far to be a best-practice IMHO.

As already reported in some of my comments to SYNCOPE-241, while most 
refactoring is useful and has introduced more order and cleanings, I 
also find a portion of the latest modifications as artificial and with 
only purpose to force some additional layering. But excessive layering 
is bad as poor layering.

As I've reported since the beginning of this issue, I don't think that 
ease of working with a particular IDE (Eclipse) can drive the development.

My proposal here is then: let's leave things are they are currently 
committed, close SYNCOPE-241 and devote effort to the other open issues, 
especially the ones missing for 1.1.0.

WDYT?

(See below my inline comments to the problems you have faced).
Regards.

> ----
> The problems I faced:
>
> - Persistence test as well as the other core tests have to support a wide range of databases. So this results in a lot of duplication between the poms of core and persistence

This can be handled somehow, not dramatic. But it needs quite some work, 
I recognize this.

> - The tests require a lot of test resources that are partly changed during the build depending on the profile. So this could also lead to a lot of duplication

This is part of the item above.

> - I tried to move the test resources into a separate test-resources project. There these resources would then be in src/main/resources to go into the jar to be available to core and persistence. This separate project alone is already a bit ugly. Additionally we have some resources like persistence.properties also in src/main/resources of core. The current tests rely on the fact that src/test/resources override src/main/resources. With a separate test-resources project this does not work anymore.

I agree with you, it's ugly :-)

-- 
Francesco Chicchiriccò

ASF Member, Apache Syncope PMC chair, Apache Cocoon PMC Member
http://people.apache.org/~ilgrosso/

Re: Discussion: Persistence module without tests

[Christian Schneider <ch...@die-schneider.net>]

I think we have a consensus for alternative 1. So I will close the issue
and will not apply my last patch.

Christian

On 15.01.2013 15:58, Christian Schneider wrote:
> So I see two alternatives:
>
> 1. Keep persistence inside core but try to keep it as a separate
> architectural layer. This would mean to avoid cycles between persistence
> and the rest of core "by hand".
> 2. Only move the persistence classes to the persistence module but not
> the tests. I created a patch for this in the issue above.
>
>
>
> So what do you guys think about this?
>
> Christian
>
>