Code migration

[vidakovic]

Split original code in following modules:

• spring-data-orientdb-commons
• spring-data-orientdb-document
• spring-data-orientdb-object
• spring-data-orientdb-graph
• spring-boot-orientdb-autoconfigure
• spring-data-samples

Removed all object related classes from commons.

[vidakovic]

@trainings I am finally done with the migration. The commons module is now pretty much sanatized from all object API references. All sub-modules only contain dependencies that immediately concern them. There is of course room for improvement, but I suggest that we start the API improvement after the merge. I made already some notes, will put those in separate tickets.

I was also able to fix the the issues with the incompatible database factories (actually the pools) APIs (in document). There is now a common interface for the factories (instead of using the abstract parent of the database factories). The parent doesn't contain any reference to the pools, this is now completely delegated to the sub-classe... and therefor no API clashes there.

As we discussed: spring-boot-orientdb is included; test-commons is gone (domain classes are back in the object module tests);

We didn't talk about it, but I forgot the spring-data-web classes. These are now included in a separate module.

@pires excellent demo with Shrio and Hazelcast is also included now as a first sample. @pires could you have a look at it when you have time? Everything is running, but can't authenticate... I think you fixed that already... don't remember exactly ;-)

@sleroy the commons module is really small now in terms of dependencies. The only reference you normally don't have in a bare bones Spring project is spring-data-commons; but if you use only the minimum (which is the database factory in spring-data-orientdb-document) and just include the spring-data-xxx dependencies in your pom.xml then you should be fine (not tested though).

All available tests are passing.

@pires @trainings @lvca @sleroy when you have time please have a quick look at it and leave your comments here. But I'd suggest to finish this here as quick as possible so that the code lands in the official repository to tackle the next steps.

Quicklink: https://github.com/vidakovic/spring-data-orientdb

[vidakovic]

2 sample applications; added @trainings "hello" app.

[sleroy]

It is a great work. I just reviewed the spring-data-commons and document/graph part.
My comments :

• I still do not understand if pool.acquire() is not already performing the code present in the db() method.
  link
• As I previously discussed with @vidakovic, I am confused with the (automatic but remote) database creation inside the connection pool. My preference is to be able to disable/enable this behaviour.
• my repository tokens are in the master pom.xml (coveralls token, and a link to my bintray). I suggest to remove it before the code lands in the official repository.
• I did not run cobertura, on a first glance, the code coverage is low;
• One basic test required is a functional/performance tests with * many thread acquiring/releasing a connection * with the orientdb factory. That's the most basic behaviour used in web apps.

[trainings]

some suggestions from my side after a quick view :

1. Remove spring-data-orientdb-web. The appropriate code should be moved to spring-data-orientdb-commons
2. Rename spring-boot-orientdb to spring-boot-orientdb-autoconfigure
3. Rename samples to spring-data-orientdb-samples or spring-data-orientdb-examples

@sleroy please create corresponding issue for enable/disable automatic creation and update database schema

[pires]

Tests are failing for me.

select * from Person where firstName = :1
select * from Person where lastName = :1
select count(*) from Person where firstName = :1
select * from Person where active = :1
select * from Person where active = :1
select * from Person where address.city = :1
select * from Person where (firstName = :1 or lastName = :2)
select * from Person where firstName like :1
select * from Person where (firstName = :1 or lastName = :2)
select count(*) from Person where firstName = 'Dzmitry'
select * from Person where lastName = :1
select * from Person where lastName like :1
select * from Person where lastName = :1
Tests run: 33, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 3.916 sec
<<< FAILURE! - in TestSuite
countByFirstName(org.springframework.data.orient.object.repository.PersonRepositoryTest)
 Time elapsed: 0.012 sec  <<< FAILURE!
java.lang.AssertionError: expected [1] but found [2]
at org.testng.Assert.fail(Assert.java:94)
at org.testng.Assert.failNotEquals(Assert.java:494)
at org.testng.Assert.assertEquals(Assert.java:123)
at org.testng.Assert.assertEquals(Assert.java:165)
at
org.springframework.data.orient.object.repository.PersonRepositoryTest.countByFirstName(PersonRepositoryTest.java:98)

countPerson(org.springframework.data.orient.object.repository.PersonRepositoryTest)
 Time elapsed: 0.003 sec  <<< FAILURE!
java.lang.AssertionError: expected [7] but found [5]
at org.testng.Assert.fail(Assert.java:94)
at org.testng.Assert.failNotEquals(Assert.java:494)
at org.testng.Assert.assertEquals(Assert.java:123)
at org.testng.Assert.assertEquals(Assert.java:265)
at org.testng.Assert.assertEquals(Assert.java:275)
at
org.springframework.data.orient.object.repository.PersonRepositoryTest.countPerson(PersonRepositoryTest.java:93)


Results :

Failed tests:

PersonRepositoryTest>AbstractTestNGSpringContextTests.run:171->countByFirstName:98
expected [1] but found [2]

PersonRepositoryTest>AbstractTestNGSpringContextTests.run:171->countPerson:93
expected [7] but found [5]



Tests run: 33, Failures: 2, Errors: 0, Skipped: 0

[INFO]
------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO]
[INFO] Spring Data OrientDB ............................... SUCCESS [
 0.906 s]
[INFO] Spring Data OrientDB Commons ....................... SUCCESS [
 2.912 s]
[INFO] Spring Data OrientDB Document ...................... SUCCESS [
 0.331 s]
[INFO] Spring Data OrientDB Object ........................ FAILURE [
 4.795 s]
[INFO] Spring Data OrientDB Graph ......................... SKIPPED
[INFO] Spring Data OrientDB Web ........................... SKIPPED
[INFO] Spring Boot OrientDB ............................... SKIPPED
[INFO] Sample - Spring Boot OrientDB Hello ................ SKIPPED
[INFO] Sample - Spring Boot OrientDB Shiro ................ SKIPPED
[INFO]
------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO]
------------------------------------------------------------------------

Other than that, my app is working and I'm submitting a little patch.

On Thu, Dec 18, 2014 at 8:22 AM, trainings notifications@github.com wrote:

Suggestions from my side:

1. Remove spring-data-orientdb-web. The appropriate code should be moved
   to spring-data-orientdb-commons
2. Rename spring-boot-orientdb to spring-boot-orientdb-autoconfigure
3. Rename samples to spring-data-orientdb-samples or
   spring-data-orientdb-examples

@sleroy https://github.com/sleroy please create corresponding issue for
enable/disable automatic creation and update database schema

—
Reply to this email directly or view it on GitHub
#16 (comment)
.

Paulo Pires

[vidakovic]

@sleroy concerning the whole pool.acquire()/db() stuff I'd suggest that we put this in a separate ticket and discuss after the migration. Personally I think that there should be just one way to get the db (always with the pool). Not sure if I am missing here a requirement.

Could you open a ticket so that we don't forget? Thanks

[vidakovic]

@trainings I just did your point 2) and 3). Done. Concerning 1) putting the web related stuff in commons... I'm not so sure... this just drags the whole spring-webmvc dependencies with it that people with minimal requirements (who use just database factories with Spring wiring) really don't need.

But if more people think that it should move there I'll do.

Update: after a quick conversation with @trainings I understand now that the spring-webmvc dependencies are optional and anyway referenced in spring-data-commons. So, this is done. One module less. Thanks @trainings for the clarification.

[vidakovic]

@pires Hmm... strange that this test is failing... can someone else confirm? They just pass here.

[vidakovic]

@sleroy I created a ticket to not forget your point with the performance tests.

[vidakovic]

@pires helped setting up a quick test on drone.io:

https://drone.io/github.com/vidakovic/spring-data-orientdb/5

I tried a couple of things, but the 2 tests are still failing. To me it seems like a configuration issue (annotations?).

@trainings If you have time would be nice if you could have a look at it.

Overall I think it's not a show stopper for the PR

[corneil]

If I am not mistake OrientDB can be used in an embedded mode. I am not sure
how the pool fits in with that mode.

On 18 December 2014 at 15:19, Aleksandar Vidakovic <notifications@github.com

wrote:

@pires https://github.com/pires helped setting up a quick test on
drone.io:

https://drone.io/github.com/vidakovic/spring-data-orientdb/5

I tried a couple of things, but the 2 tests are still failing. To me it
seems like a configuration issue (annotations?).

@trainings https://github.com/trainings If you have time would be nice
if you could have a look at it.

Overall I think it's not a show stopper for the PR

—
Reply to this email directly or view it on GitHub
#16 (comment)
.

[vidakovic]

@pires @trainings @lvca @sleroy

Ok... to get things rolling:

+1 from me to finish up here, do the PR and ask @lvca to merge

Please have your say here.

And thanks again for all the help and feedback.

[sleroy]

@corneil My experiments show that for version 1.7, embedded database (aka memory database) is closed (and destroyed) with the connection. Using a pool results to the destruction of the database when the pool is releasing the connection.

Using a embedded database with a pool is troublesome when :

• you share this database btw Junit/TestNG test cases
• when you do not control the lifecycle of the ODatabaseDocumentTX (if you are manipulating it through a repository)

The turnaround in 1.7 is to use a Global system property of OrientDB to force embedded database memory to stay opened (KeepOpened)

-Dstorage.keepOpen=true or directly OGlobalConfiguration.STORAGE_KEEP_OPEN.setValue( false );

link1
link2

Please notes that this property has been marked deprecated in 2.x.

Sylvain.

[sleroy]

Let's PR when the build on drone.io is ok.

[vidakovic]

@sleroy ok... have a couple of cycles left here... I am trying now locally if I can reproduce the failing tests with other JDKs... I just realized that I used a Zulu JDK 8 from Azul, might be the source why I see something different. Will come back with results.

[vidakovic]

Just FYI: I fixed the tests

https://drone.io/github.com/vidakovic/spring-data-orientdb/12

I think we can proceed with the PR tomorrow if we don't find something horribly failing.

Everything is configured for the new 2.0-rc1 release.

[pires]

+1 for PR. Amazing job, @vidakovic

[sleroy]

+1 Real great work and involvement @vidakovic

[vidakovic]

Closed by #21