[Geoserver-devel] Patch for hibernate based catalog

Hi Justin and list,

Francesco has just uploaded a new patch in
http://jira.codehaus.org/browse/GEOS-3569 .

As already agreed on IRC, we planned to:
- make the 2.0.x branch work, with the compromise on having a minimal
code change to avoid breaking API;
- make the needed refactorization on trunk, in order to have a clean
design that allows both original catalog and an hibernate-driven one
to run.

This patch is the 2.0.x one.
With respect to the first proposed patch, we removed a lot of bean
classes, since most of them were just dup's, as Justin noticed;
The ImplementationMapper interface is still there, but we're going to
clean it in the trunk.

If the patch is ok for you, who's going to commit it? (I've only be
granted rights to commit in the community area.)

   Ciao,
   Emanuele

I am not sure we are understanding each other. My preference is not to workaround making api changes, my preference is to fix the things that should be fixed in order to allow things to be done cleanly.

I still have reservations about this patch because it introduces new api as a workaround. And it is api that we will be stuck with. We won't be able to just remove it later since it will break the hibernate catalog.

So I really think that we should do this properly now, and suffer a bit of pain now, rather than push this back and have to deal with it later to a time when it will be much more expensive to fix.

My preference is to allow the hibernate module to override the xstream persister via spring. Now I know that things written the way they are now does not allow this. This is what I would like to see fixed. Which involves two steps:

1) refactor the XStreamPersister class a bit to enable a subclass to override all the implementation class assumptions

2) Create a factory interface for XStreamPersister instances. This would be what is registered in the spring context, and would be what the hibernate module will override.

3) Update all code that instantiates XStreamPerister directly to instead go through the factory.

Now realizing that the two are in the end equivalent I think this i a bit cleaner of an approach, and it does not really change the default code path, so in mind it is actually a bit more stable of a change.

I also realize that this has been a frustrating issue for you guys. I am willing to put in time to helping with this change. If you agree that what I propose will be equivalent I will make it a priority to work up the patch so that a) you can continue to work and b) i can sleep easy knowing that we have now worked around the problem.

Thoughts?

Emanuele Tajariol wrote:

Hi Justin and list,

Francesco has just uploaded a new patch in
http://jira.codehaus.org/browse/GEOS-3569 .

As already agreed on IRC, we planned to:
- make the 2.0.x branch work, with the compromise on having a minimal
code change to avoid breaking API;
- make the needed refactorization on trunk, in order to have a clean
design that allows both original catalog and an hibernate-driven one
to run.

This patch is the 2.0.x one.
With respect to the first proposed patch, we removed a lot of bean
classes, since most of them were just dup's, as Justin noticed;
The ImplementationMapper interface is still there, but we're going to
clean it in the trunk.

If the patch is ok for you, who's going to commit it? (I've only be
granted rights to commit in the community area.)

   Ciao,
   Emanuele

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

Ciao Justin,
looking at the logs of the IRC chat we had last week, especially the
bits reported below here, it looks to me that we went ahead with the
agreed plan, make things work now for 2.0.x and then clean up and
continue on trunk:

[15:55] <etj> (it's only a matter "id" and "isDefault" fields)
[15:56] <jdeolive> cool, so etj to confirm
[15:56] <jdeolive> what commits are reqruired to make things compile?
[15:57] <etj> if I remeber right, it was a change in the XStreamPersister
[15:57] <etj> plus an Interface and a DefaultImplementation
[15:57] <jdeolive> sigh... so yeah, that is what i consider the thing
to be cleaned up on trunk
[15:58] <etj> for mapping the various gs *Info interfaces to the
proper implementations
[15:58] <etj> yep
[15:58] <etj> I was talking about making the whole thing stable
[15:58] <etj> but
[15:58] <etj> we may work on the XStreamPersister a bit
[15:59] <etj> in order to make generic List works
[15:59] <etj> avoiding dependencies to Hibernate classes
[16:00] <etj> (sorry for the latency, I'm using a web client)
[16:00] <jdeolive> np
[16:01] <etj> cleaning the hibenrate beans stuff, and writing generic
converters to avoid hibernate collections misinterpretation
[16:01] <jdeolive> ok
[16:02] <etj> the thing should be cleaned a bit
[16:03] <jdeolive> ok sounds good, well i will await your next steps
[16:03] <jdeolive> i have to run to another meeting now
[16:03] <jdeolive> thanks for the chat guys

Anyway, the goal here is to remove the blockers and move forward in a
way to kepps everybody happy.
You mention the fact that you may be able to spare some time for,
well, I think that at this stage the best thing is that you spare 1-2
h and lay down a reasonable plan that we can follows, in order to
avoid further issues.
What do you think?

Simone.
-------------------------------------------------------
Ing. Simone Giannecchini
GeoSolutions S.A.S.
Founder - Software Engineer
Via Carignoni 51
55041 Camaiore (LU)
Italy

phone: +39 0584983027
fax: +39 0584983027
mob: +39 333 8128928

http://www.geo-solutions.it
http://geo-solutions.blogspot.com/
http://simboss.blogspot.com/
http://www.linkedin.com/in/simonegiannecchini

-------------------------------------------------------

On Mon, Nov 16, 2009 at 5:01 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:

I am not sure we are understanding each other. My preference is not to
workaround making api changes, my preference is to fix the things that
should be fixed in order to allow things to be done cleanly.

I still have reservations about this patch because it introduces new api
as a workaround. And it is api that we will be stuck with. We won't be
able to just remove it later since it will break the hibernate catalog.

So I really think that we should do this properly now, and suffer a bit
of pain now, rather than push this back and have to deal with it later
to a time when it will be much more expensive to fix.

My preference is to allow the hibernate module to override the xstream
persister via spring. Now I know that things written the way they are
now does not allow this. This is what I would like to see fixed. Which
involves two steps:

1) refactor the XStreamPersister class a bit to enable a subclass to
override all the implementation class assumptions

2) Create a factory interface for XStreamPersister instances. This would
be what is registered in the spring context, and would be what the
hibernate module will override.

3) Update all code that instantiates XStreamPerister directly to instead
go through the factory.

Now realizing that the two are in the end equivalent I think this i a
bit cleaner of an approach, and it does not really change the default
code path, so in mind it is actually a bit more stable of a change.

I also realize that this has been a frustrating issue for you guys. I am
willing to put in time to helping with this change. If you agree that
what I propose will be equivalent I will make it a priority to work up
the patch so that a) you can continue to work and b) i can sleep easy
knowing that we have now worked around the problem.

Thoughts?

Emanuele Tajariol wrote:

Hi Justin and list,

Francesco has just uploaded a new patch in
http://jira.codehaus.org/browse/GEOS-3569 .

As already agreed on IRC, we planned to:
- make the 2.0.x branch work, with the compromise on having a minimal
code change to avoid breaking API;
- make the needed refactorization on trunk, in order to have a clean
design that allows both original catalog and an hibernate-driven one
to run.

This patch is the 2.0.x one.
With respect to the first proposed patch, we removed a lot of bean
classes, since most of them were just dup's, as Justin noticed;
The ImplementationMapper interface is still there, but we're going to
clean it in the trunk.

If the patch is ok for you, who's going to commit it? (I've only be
granted rights to commit in the community area.)

Ciao,
Emanuele

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel