[Geoserver-devel] Catalog

[David_Winslow]

Hi Justin,

I've been looking into using XStream for RESTConfig and possibly working on a Catalog backend based on it as well. I've got a few questions from my catalog safari, hopefully you can help out.

1. It doesn't look like the POJO + Proxy implementation that's currently in trunk actually persists the catalog. In particular, I see the old SaveXMLAction class is still using the old XMLWriter. It also appears that Catalog.save() isn't being called anywhere, which seems to me to indicate that the catalog should not be changed ever. I guess what I am asking here is what we should expect when doing dumb stuff like:

Catalog c = ...
ResourceInfo a = c.getResource(someID);
alterSomeWay(a);
ResourceInfo b = c.getResource(someID);
// does b now reflect the changes made to a?
// I suspect it should not, since save() has not been called,
// but the current wrappers seem to rely on this behavior
// so I am a bit confused.

2. There are a couple of failing tests in trunk/main (which are masking a lot of failing tests in other modules, oops). It's not clear to me whether or not these tests are correct.

CatalogImplTest.testModifyWorkspace(): This seems to be testing that changing the name of a workspace does not change its ID; but I don't see this expectation documented anywhere. It seems like a reasonable one, so maybe we should just add a note about it to the Catalog interface.

GeoServerImplTest.testGlobalEvents(): This one seems less mysterious; from what I can see it changes 2 properties and then asserts that 3 have been changed. This is mostly an implementation detail of the current catalog implementation and not that important to my XStream work, but I thought I might as well check with you before changing tests for the current implementation.

3. It is not clear to me how much of the current catalog implementation is intended to be reusable, though if I had to guess I would say that possibly the event implementations are reusable due to their simplicity, but not much else. If you have anything to add here I'd be glad to find large chunks of reusable code (not holding my breath though )

4. It seems like the new Catalog API is both Important and Complicated. I wonder therefore whether we should start working on a test suite for Catalog implementations codifying all the weird expectations we have for it. A decent start would probably be to go over the current *ImplTest's and try to split off the ones that are testing implementation details from the ones that are testing the intended behavior; we could add tests as intended behavior proves ambiguous.

-David

[Justin_Deoliveira]

David Winslow wrote:

Hi Justin,

I've been looking into using XStream for RESTConfig and possibly working on a Catalog backend based on it as well. I've got a few questions from my catalog safari, hopefully you can help out.

I have a half implementation that persists and loads the catalog and config objects which i have not committed yet. I will try to commit soon as i finish the wfs 1.1 separation work.

1. It doesn't look like the POJO + Proxy implementation that's currently in trunk actually persists the catalog. In particular, I see the old SaveXMLAction class is still using the old XMLWriter.

Correct, currently on 1.7.x the only way to save the catalog is from the UI. My idea for the new UI was to add to GeoServerLoader.destroy() a routine that persists the catalog via xstream.

It also

appears that Catalog.save() isn't being called anywhere, which seems to me to indicate that the catalog should not be changed ever. I guess what I am asking here is what we should expect when doing dumb stuff like:

Correct, the old system works by replacing all application (global) objects with there ui (config) counterparts. So the way it works today is that the catalog is entirely thrown away and reloaded every time a chance is applied.

Catalog c = ...
ResourceInfo a = c.getResource(someID);
alterSomeWay(a);
ResourceInfo b = c.getResource(someID);
// does b now reflect the changes made to a?
// I suspect it should not, since save() has not been called,
// but the current wrappers seem to rely on this behavior
// so I am a bit confused.

You are correct, that is the intended use. Again, saving directly when an object is changed (in the current system) is not something that is supported so its not used. Ideally in the new ui every form that changes will jsut call save() directly, instead of reloading the entire catalog.

2. There are a couple of failing tests in trunk/main (which are masking a lot of failing tests in other modules, oops). It's not clear to me whether or not these tests are correct.

CatalogImplTest.testModifyWorkspace(): This seems to be testing that changing the name of a workspace does not change its ID; but I don't see this expectation documented anywhere. It seems like a reasonable one, so maybe we should just add a note about it to the Catalog interface.

GeoServerImplTest.testGlobalEvents(): This one seems less mysterious; from what I can see it changes 2 properties and then asserts that 3 have been changed. This is mostly an implementation detail of the current catalog implementation and not that important to my XStream work, but I thought I might as well check with you before changing tests for the current implementation.

Yeah, fixes for this are coming. I was about to commit them the last night of the sprint when svn gave out. I just have to sort the commits out from a bunch of other local mods in my checkout, the fix will come shortly.

3. It is not clear to me how much of the current catalog implementation is intended to be reusable, though if I had to guess I would say that possibly the event implementations are reusable due to their simplicity, but not much else. If you have anything to add here I'd be glad to find large chunks of reusable code (not holding my breath though )

Not sure exactly what you mean here by reusable. Do you mean by other subsystems like REST? Or do you mean reusable parts usable by other catalog implementations?

4. It seems like the new Catalog API is both Important and Complicated. I wonder therefore whether we should start working on a test suite for Catalog implementations codifying all the weird expectations we have for it. A decent start would probably be to go over the current *ImplTest's and try to split off the ones that are testing implementation details from the ones that are testing the intended behavior; we could add tests as intended behavior proves ambiguous.

I don't see it all that likely that another implementation will be coming along all that soon due to how non-trivial it would be. During the prototyping of this new api we tested two implementations. The first was the "in memory" implementation currently on trunk. The second was a catalog backed onto hibernate. So the interface and paradigm did hold up against two radically different implementations.

But I definitely agree that having tests against the interface itself and against the implementation should be separated. And if we do get another implementation i suspect it will be mandatory.

-David

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

!DSPAM:4007,486193de95793327367457!

--
Justin Deoliveira
The Open Planning Project
jdeolive@anonymised.com

[David_Winslow]

Justin Deoliveira wrote:

David Winslow wrote:

Hi Justin,

I've been looking into using XStream for RESTConfig and possibly working on a Catalog backend based on it as well. I've got a few questions from my catalog safari, hopefully you can help out.

I have a half implementation that persists and loads the catalog and config objects which i have not committed yet. I will try to commit soon as i finish the wfs 1.1 separation work.

1. It doesn't look like the POJO + Proxy implementation that's currently in trunk actually persists the catalog. In particular, I see the old SaveXMLAction class is still using the old XMLWriter.

Correct, currently on 1.7.x the only way to save the catalog is from the UI. My idea for the new UI was to add to GeoServerLoader.destroy() a routine that persists the catalog via xstream.

It also

appears that Catalog.save() isn't being called anywhere, which seems to me to indicate that the catalog should not be changed ever. I guess what I am asking here is what we should expect when doing dumb stuff like:

Correct, the old system works by replacing all application (global) objects with there ui (config) counterparts. So the way it works today is that the catalog is entirely thrown away and reloaded every time a chance is applied.

Catalog c = ...
ResourceInfo a = c.getResource(someID);
alterSomeWay(a);
ResourceInfo b = c.getResource(someID);
// does b now reflect the changes made to a?
// I suspect it should not, since save() has not been called,
// but the current wrappers seem to rely on this behavior
// so I am a bit confused.

You are correct, that is the intended use. Again, saving directly when an object is changed (in the current system) is not something that is supported so its not used. Ideally in the new ui every form that changes will jsut call save() directly, instead of reloading the entire catalog.

2. There are a couple of failing tests in trunk/main (which are masking a lot of failing tests in other modules, oops). It's not clear to me whether or not these tests are correct.

CatalogImplTest.testModifyWorkspace(): This seems to be testing that changing the name of a workspace does not change its ID; but I don't see this expectation documented anywhere. It seems like a reasonable one, so maybe we should just add a note about it to the Catalog interface.

GeoServerImplTest.testGlobalEvents(): This one seems less mysterious; from what I can see it changes 2 properties and then asserts that 3 have been changed. This is mostly an implementation detail of the current catalog implementation and not that important to my XStream work, but I thought I might as well check with you before changing tests for the current implementation.

Yeah, fixes for this are coming. I was about to commit them the last night of the sprint when svn gave out. I just have to sort the commits out from a bunch of other local mods in my checkout, the fix will come shortly.

3. It is not clear to me how much of the current catalog implementation is intended to be reusable, though if I had to guess I would say that possibly the event implementations are reusable due to their simplicity, but not much else. If you have anything to add here I'd be glad to find large chunks of reusable code (not holding my breath though )

Not sure exactly what you mean here by reusable. Do you mean by other subsystems like REST? Or do you mean reusable parts usable by other catalog implementations?

I was thinking of reusing as much as possible for the XStream backend. If the current backend is going to use XStream for persistence anyway then that doesn't mean much But yes, I had 'reusable for alternative implementations' in mind there, as opposed to 'reusable by other subsystems,' which I was taking for granted as a design goal. (I also wonder whether (some subset of) the Catalog API could be used for a Java client API for RESTConfig. If done right, this could allow a network of GeoServers to share configuration transparently (configure one to save to a real persistance mechanism, and the rest just sync with it via REST) and be Quite Nifty.)

4. It seems like the new Catalog API is both Important and Complicated. I wonder therefore whether we should start working on a test suite for Catalog implementations codifying all the weird expectations we have for it. A decent start would probably be to go over the current *ImplTest's and try to split off the ones that are testing implementation details from the ones that are testing the intended behavior; we could add tests as intended behavior proves ambiguous.

I don't see it all that likely that another implementation will be coming along all that soon due to how non-trivial it would be. During the prototyping of this new api we tested two implementations. The first was the "in memory" implementation currently on trunk. The second was a catalog backed onto hibernate. So the interface and paradigm did hold up against two radically different implementations.

But I definitely agree that having tests against the interface itself and against the implementation should be separated. And if we do get another implementation i suspect it will be mandatory.

Thanks for the quick response.

-David

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

Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

[Justin_Deoliveira]

I was thinking of reusing as much as possible for the XStream backend. If the current backend is going to use XStream for persistence anyway then that doesn't mean much But yes, I had 'reusable for alternative implementations' in mind there, as opposed to 'reusable by other subsystems,' which I was taking for granted as a design goal.

OK, still not sure I 100% understand but yes :). I do think that we should structure it in a way that we don't add any dependencies from the core model on xstream, so that we could swap out other persistence backends easily.

  (I also

wonder whether (some subset of) the Catalog API could be used for a Java client API for RESTConfig. If done right, this could allow a network of GeoServers to share configuration transparently (configure one to save to a real persistance mechanism, and the rest just sync with it via REST) and be Quite Nifty.)

Yeah I had the same thought. I think it could work. All the "client" really needs is the core catalog and config model which is pretty isolated from the rest of GeoServer.

Having a persistence backend based on this client and actually stores config on another GeoServer is a great idea. I think Chris brought this up a while ago.

4. It seems like the new Catalog API is both Important and Complicated. I wonder therefore whether we should start working on a test suite for Catalog implementations codifying all the weird expectations we have for it. A decent start would probably be to go over the current *ImplTest's and try to split off the ones that are testing implementation details from the ones that are testing the intended behavior; we could add tests as intended behavior proves ambiguous.

I don't see it all that likely that another implementation will be coming along all that soon due to how non-trivial it would be. During the prototyping of this new api we tested two implementations. The first was the "in memory" implementation currently on trunk. The second was a catalog backed onto hibernate. So the interface and paradigm did hold up against two radically different implementations.

But I definitely agree that having tests against the interface itself and against the implementation should be separated. And if we do get another implementation i suspect it will be mandatory.

Thanks for the quick response.

-David

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

Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

!DSPAM:4007,48619c55109111804284693!

--
Justin Deoliveira
The Open Planning Project
jdeolive@anonymised.com

[David_Winslow]

Justin Deoliveira wrote:

I was thinking of reusing as much as possible for the XStream backend. If the current backend is going to use XStream for persistence anyway then that doesn't mean much But yes, I had 'reusable for alternative implementations' in mind there, as opposed to 'reusable by other subsystems,' which I was taking for granted as a design goal.

OK, still not sure I 100% understand but yes :). I do think that we should structure it in a way that we don't add any dependencies from the core model on xstream, so that we could swap out other persistence backends easily.

Ah, got it now. I had been thinking persistence mechanisms would require reimplementing the entire catalog but it could probably be done in most cases with just loaders and change listeners, leaving the in-memory objects to be shared across implementations. I wonder, though, whether a 'store everything on shutdown' policy is necessary/useful if we are saving all changes that are meant to be saved along the way. I suppose it would be useful for 'export configuration' functionality as well.

-David

(I also

wonder whether (some subset of) the Catalog API could be used for a Java client API for RESTConfig. If done right, this could allow a network of GeoServers to share configuration transparently (configure one to save to a real persistance mechanism, and the rest just sync with it via REST) and be Quite Nifty.)

Yeah I had the same thought. I think it could work. All the "client" really needs is the core catalog and config model which is pretty isolated from the rest of GeoServer.

Having a persistence backend based on this client and actually stores config on another GeoServer is a great idea. I think Chris brought this up a while ago.

4. It seems like the new Catalog API is both Important and Complicated. I wonder therefore whether we should start working on a test suite for Catalog implementations codifying all the weird expectations we have for it. A decent start would probably be to go over the current *ImplTest's and try to split off the ones that are testing implementation details from the ones that are testing the intended behavior; we could add tests as intended behavior proves ambiguous.

I don't see it all that likely that another implementation will be coming along all that soon due to how non-trivial it would be. During the prototyping of this new api we tested two implementations. The first was the "in memory" implementation currently on trunk. The second was a catalog backed onto hibernate. So the interface and paradigm did hold up against two radically different implementations.

But I definitely agree that having tests against the interface itself and against the implementation should be separated. And if we do get another implementation i suspect it will be mandatory.

Thanks for the quick response.

-David

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

Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

[Andrea_Aime3]

David Winslow ha scritto:

Justin Deoliveira wrote:

I was thinking of reusing as much as possible for the XStream backend. If the current backend is going to use XStream for persistence anyway then that doesn't mean much But yes, I had 'reusable for alternative implementations' in mind there, as opposed to 'reusable by other subsystems,' which I was taking for granted as a design goal.

OK, still not sure I 100% understand but yes :). I do think that we should structure it in a way that we don't add any dependencies from the core model on xstream, so that we could swap out other persistence backends easily.

Ah, got it now. I had been thinking persistence mechanisms would require reimplementing the entire catalog but it could probably be done in most cases with just loaders and change listeners, leaving the in-memory objects to be shared across implementations. I wonder, though, whether a 'store everything on shutdown' policy is necessary/useful if we are saving all changes that are meant to be saved along the way.

I share the same concern. We do want to save along the way since we
cannot be sure there will be a clean shutdown (jvm segfaults anybody?),
so eventually on shutdown we'd just be saving whatever was already
saved the last time the config did change?

Cheers
Andrea

[Justin_Deoliveira]

I share the same concern. We do want to save along the way since we
cannot be sure there will be a clean shutdown (jvm segfaults anybody?),
so eventually on shutdown we'd just be saving whatever was already
saved the last time the config did change?

Yeah its a darned if you do darned if you don't sort of thing. On the one hand saving when the user decides to from the ui or at shutdown allows the ability to rollback or revert changes. Auto-saving everytime something gets modified removes the ability to do that but prevents loss of data when the system crashes.

I wonder if we could introduce the idea of a session (and i dont really mean page session here) to get best of both worlds. Basically instead of saving directly to persistence we save to a session. The session is persisted immediately to disk whenever a change occurs. If the user decides to save changes from the session are persisted. In the event of a jvm crash GeoServer could detect that the session was not saved and restore it so no work is lossed. Not sure if it will work or its worth the effort, just a crazy idea.

-Justin

Cheers
Andrea

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

!DSPAM:4007,4861e956213247082231907!

--
Justin Deoliveira
The Open Planning Project
jdeolive@anonymised.com

[Andrea_Aime3]

Justin Deoliveira ha scritto:

I share the same concern. We do want to save along the way since we
cannot be sure there will be a clean shutdown (jvm segfaults anybody?),
so eventually on shutdown we'd just be saving whatever was already
saved the last time the config did change?

Yeah its a darned if you do darned if you don't sort of thing. On the one hand saving when the user decides to from the ui or at shutdown allows the ability to rollback or revert changes. Auto-saving everytime something gets modified removes the ability to do that but prevents loss of data when the system crashes.

I wonder if we could introduce the idea of a session (and i dont really mean page session here) to get best of both worlds. Basically instead of saving directly to persistence we save to a session. The session is persisted immediately to disk whenever a change occurs. If the user decides to save changes from the session are persisted. In the event of a jvm crash GeoServer could detect that the session was not saved and restore it so no work is lossed. Not sure if it will work or its worth the effort, just a crazy idea.

So basically instead of having the DTO on top of catalog, we have their
cousins below it? (ok, I understand you can store stuff in whatever
way you want, not necessarily objects).

For what it's worth, I'd just go to the direct save route. People
are working directly against databases with a lot of software, and
none of it give you the "undo/redo" ability of a desktop app without
much complaints.

Wait... what did I just say? Undo/redo huh? We _could_ in fact store
on the disk a list of commands that morph the catalog and allow
people to actually undo their changes... that would be a first for
server side apps. And of course, it assumes a single admin model, or
else, we'd have to mark changes with the user that performed them
(with the new security subsystem any user can have the ROLE_ADMNISTRATOR
role).

Cheers
Andrea

[David_Winslow]

It does seem like having 'dump entire catalog' functionality would be useful (for migrating between different persistence mechanisms) so (for another compromises solution) maybe we could just have a button to download/snapshot the catalog and leave it up to the admin to handle backup policy.

-David

Andrea Aime wrote:

Justin Deoliveira ha scritto:

I share the same concern. We do want to save along the way since we
cannot be sure there will be a clean shutdown (jvm segfaults anybody?),
so eventually on shutdown we'd just be saving whatever was already
saved the last time the config did change?

Yeah its a darned if you do darned if you don't sort of thing. On the one hand saving when the user decides to from the ui or at shutdown allows the ability to rollback or revert changes. Auto-saving everytime something gets modified removes the ability to do that but prevents loss of data when the system crashes.

I wonder if we could introduce the idea of a session (and i dont really mean page session here) to get best of both worlds. Basically instead of saving directly to persistence we save to a session. The session is persisted immediately to disk whenever a change occurs. If the user decides to save changes from the session are persisted. In the event of a jvm crash GeoServer could detect that the session was not saved and restore it so no work is lossed. Not sure if it will work or its worth the effort, just a crazy idea.

So basically instead of having the DTO on top of catalog, we have their
cousins below it? (ok, I understand you can store stuff in whatever
way you want, not necessarily objects).

For what it's worth, I'd just go to the direct save route. People
are working directly against databases with a lot of software, and
none of it give you the "undo/redo" ability of a desktop app without
much complaints.

Wait... what did I just say? Undo/redo huh? We _could_ in fact store
on the disk a list of commands that morph the catalog and allow
people to actually undo their changes... that would be a first for
server side apps. And of course, it assumes a single admin model, or
else, we'd have to mark changes with the user that performed them
(with the new security subsystem any user can have the ROLE_ADMNISTRATOR
role).

Cheers
Andrea

!DSPAM:4040,486244e433951637810514!

[Justin_Deoliveira]

For what it's worth, I'd just go to the direct save route. People
are working directly against databases with a lot of software, and
none of it give you the "undo/redo" ability of a desktop app without
much complaints.

Good point.

Wait... what did I just say? Undo/redo huh? We _could_ in fact store
on the disk a list of commands that morph the catalog and allow
people to actually undo their changes... that would be a first for
server side apps. And of course, it assumes a single admin model, or
else, we'd have to mark changes with the user that performed them
(with the new security subsystem any user can have the ROLE_ADMNISTRATOR
role).

Hmmm... i like it... I like it a lot. Doing everything in a command not only gives us undo / redo it would also allow the rest stuff to use a lot of the same logic and avoid duplication. Go Andrea, you win a puppet :).

Cheers
Andrea

!DSPAM:4007,486244e433961012714783!

--
Justin Deoliveira
The Open Planning Project
jdeolive@anonymised.com

[Justin_Deoliveira]

David Winslow wrote:

It does seem like having 'dump entire catalog' functionality would be useful (for migrating between different persistence mechanisms) so (for another compromises solution) maybe we could just have a button to download/snapshot the catalog and leave it up to the admin to handle backup policy.

+1. I know Chris really wants an export utility as well so this would work well.

-David

--
Justin Deoliveira
The Open Planning Project
jdeolive@anonymised.com

[Chris_Holmes1]

Yeah, I think undo/redo is a cool idea, but in the desire to cut scope I'm inclined to go with David's suggestion in the short term - just have an 'export' or 'save as' button, so that an admin can save it at a point that he wants. In 2.1 or something we could try out the undo/redo, which I agree would be pretty slick.

C

David Winslow wrote:

It does seem like having 'dump entire catalog' functionality would be useful (for migrating between different persistence mechanisms) so (for another compromises solution) maybe we could just have a button to download/snapshot the catalog and leave it up to the admin to handle backup policy.

-David

Andrea Aime wrote:

Justin Deoliveira ha scritto:

I share the same concern. We do want to save along the way since we
cannot be sure there will be a clean shutdown (jvm segfaults anybody?),
so eventually on shutdown we'd just be saving whatever was already
saved the last time the config did change?

Yeah its a darned if you do darned if you don't sort of thing. On the one hand saving when the user decides to from the ui or at shutdown allows the ability to rollback or revert changes. Auto-saving everytime something gets modified removes the ability to do that but prevents loss of data when the system crashes.

I wonder if we could introduce the idea of a session (and i dont really mean page session here) to get best of both worlds. Basically instead of saving directly to persistence we save to a session. The session is persisted immediately to disk whenever a change occurs. If the user decides to save changes from the session are persisted. In the event of a jvm crash GeoServer could detect that the session was not saved and restore it so no work is lossed. Not sure if it will work or its worth the effort, just a crazy idea.

So basically instead of having the DTO on top of catalog, we have their
cousins below it? (ok, I understand you can store stuff in whatever
way you want, not necessarily objects).

For what it's worth, I'd just go to the direct save route. People
are working directly against databases with a lot of software, and
none of it give you the "undo/redo" ability of a desktop app without
much complaints.

Wait... what did I just say? Undo/redo huh? We _could_ in fact store
on the disk a list of commands that morph the catalog and allow
people to actually undo their changes... that would be a first for
server side apps. And of course, it assumes a single admin model, or
else, we'd have to mark changes with the user that performed them
(with the new security subsystem any user can have the ROLE_ADMNISTRATOR
role).

Cheers
Andrea

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

!DSPAM:4005,4862461838651096210785!