[Geoserver-devel] catalog add events

GeoServer GeoServer Developer
1

Hey all,

got a question about that catalog event handling.

Modification events seem to be deferred to the CatalogFacade, which in
turns sends a pre-modified event with the currently saved object plus
the modified properties, and after persisting the changes issues a
post-modified event with the final object.
So far so good.
But when it comes to issuing a CatalogAddEvent, the one in charge is
CatalogImpl, not the CatalogFacade.
Still all CatalogFacade.add( T ) also have return type T, but the
return value gets lost, since CatalogImpl issues the CatalogAddEvent
with its argument object, not with the one returned by the
CatalogFacade.

For instance:
CatalogImpl{

  public void add(LayerInfo info){
      ...
      facade.add(info);
      added(info);
  }
}

instead of
  public void add(LayerInfo info){
      ...
      LayerInfo added = facade.add(info);
      added(added);
  }

This happens to all add methods except add(LayerGroupInfo) which does
added(facade.add(layerGroup))

I'm under the impression all add methods should issue the add event
with the value returned by the facade, but fear to be missing
something.

Opinions?

Cheers,
Gabriel
--
Gabriel Roldan
OpenGeo - http://opengeo.org
Expert service straight from the developers.

2

Hey Gabriel,

Yeah, I think it makes sense to have the result returned by the facade be passed into the event. Looks like most of the code was not updated when the facade api was changed to have the add methods return a result, that wasn’t always the case and they used have a void return.

So yeah, +1 on having the rest of the add methods follow suit after LayerGroupInfo.

-Justin

On Thu, Dec 29, 2011 at 4:24 PM, Gabriel Roldan <groldan@anonymised.com.1501…> wrote:

Hey all,

got a question about that catalog event handling.

Modification events seem to be deferred to the CatalogFacade, which in
turns sends a pre-modified event with the currently saved object plus
the modified properties, and after persisting the changes issues a
post-modified event with the final object.
So far so good.
But when it comes to issuing a CatalogAddEvent, the one in charge is
CatalogImpl, not the CatalogFacade.
Still all CatalogFacade.add( T ) also have return type T, but the
return value gets lost, since CatalogImpl issues the CatalogAddEvent
with its argument object, not with the one returned by the
CatalogFacade.

For instance:
CatalogImpl{

public void add(LayerInfo info){
…
facade.add(info);
added(info);
}
}

instead of
public void add(LayerInfo info){
…
LayerInfo added = facade.add(info);
added(added);
}

This happens to all add methods except add(LayerGroupInfo) which does
added(facade.add(layerGroup))

I’m under the impression all add methods should issue the add event
with the value returned by the facade, but fear to be missing
something.

Opinions?

Cheers,
Gabriel

Gabriel Roldan
OpenGeo - http://opengeo.org
Expert service straight from the developers.

Ridiculously easy VDI. With Citrix VDI-in-a-Box, you don’t need a complex
infrastructure or vast IT resources to deliver seamless, secure access to
virtual desktops. With this all-in-one solution, easily deploy virtual
desktops for less than the cost of PCs and save 60% on VDI infrastructure
costs. Try it free! http://p.sf.net/sfu/Citrix-VDIinabox

Geoserver-devel mailing list
Geoserver-devel@anonymised.comsts.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

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

3

On Fri, Dec 30, 2011 at 12:49 PM, Justin Deoliveira
<jdeolive@anonymised.com> wrote:

Hey Gabriel,

Yeah, I think it makes sense to have the result returned by the facade be
passed into the event. Looks like most of the code was not updated when the
facade api was changed to have the add methods return a result, that
wasn't always the case and they used have a void return.

So yeah, +1 on having the rest of the add methods follow suit after
LayerGroupInfo.

Okay, thanks for the prompt reply. I just committed the change to
trunk. Let me know if you think we should do so to 2.1.x too, although
my personal opinion is to refrain "just in case".

BTW, I took the liberty to add a hibernate user type for KeywordInfo
in the dbconfig module as it was missing. Let me know if you have any
objection or want something changed.

Cheers,
Gabriel

-Justin

On Thu, Dec 29, 2011 at 4:24 PM, Gabriel Roldan <groldan@anonymised.com> wrote:

Hey all,

got a question about that catalog event handling.

Modification events seem to be deferred to the CatalogFacade, which in
turns sends a pre-modified event with the currently saved object plus
the modified properties, and after persisting the changes issues a
post-modified event with the final object.
So far so good.
But when it comes to issuing a CatalogAddEvent, the one in charge is
CatalogImpl, not the CatalogFacade.
Still all CatalogFacade.add( T ) also have return type T, but the
return value gets lost, since CatalogImpl issues the CatalogAddEvent
with its argument object, not with the one returned by the
CatalogFacade.

For instance:
CatalogImpl{

public void add(LayerInfo info){
...
facade.add(info);
added(info);
}
}

instead of
public void add(LayerInfo info){
...
LayerInfo added = facade.add(info);
added(added);
}

This happens to all add methods except add(LayerGroupInfo) which does
added(facade.add(layerGroup))

I'm under the impression all add methods should issue the add event
with the value returned by the facade, but fear to be missing
something.

Opinions?

Cheers,
Gabriel
--
Gabriel Roldan
OpenGeo - http://opengeo.org
Expert service straight from the developers.

------------------------------------------------------------------------------
Ridiculously easy VDI. With Citrix VDI-in-a-Box, you don't need a complex
infrastructure or vast IT resources to deliver seamless, secure access to
virtual desktops. With this all-in-one solution, easily deploy virtual
desktops for less than the cost of PCs and save 60% on VDI infrastructure
costs. Try it free! http://p.sf.net/sfu/Citrix-VDIinabox
_______________________________________________
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.

--
Gabriel Roldan
OpenGeo - http://opengeo.org
Expert service straight from the developers.

4

On Sun, Jan 1, 2012 at 8:01 PM, Gabriel Roldan <groldan@…1501…> wrote:

On Fri, Dec 30, 2011 at 12:49 PM, Justin Deoliveira
<jdeolive@anonymised.com> wrote:

Hey Gabriel,

Yeah, I think it makes sense to have the result returned by the facade be
passed into the event. Looks like most of the code was not updated when the
facade api was changed to have the add methods return a result, that
wasn’t always the case and they used have a void return.

So yeah, +1 on having the rest of the add methods follow suit after
LayerGroupInfo.

Okay, thanks for the prompt reply. I just committed the change to
trunk. Let me know if you think we should do so to 2.1.x too, although
my personal opinion is to refrain “just in case”.

Agreed.

BTW, I took the liberty to add a hibernate user type for KeywordInfo
in the dbconfig module as it was missing. Let me know if you have any
objection or want something changed.

Thanks! Definitely no objection.

Cheers,
Gabriel

-Justin

On Thu, Dec 29, 2011 at 4:24 PM, Gabriel Roldan <groldan@anonymised.com> wrote:

Hey all,

got a question about that catalog event handling.

Modification events seem to be deferred to the CatalogFacade, which in
turns sends a pre-modified event with the currently saved object plus
the modified properties, and after persisting the changes issues a
post-modified event with the final object.
So far so good.
But when it comes to issuing a CatalogAddEvent, the one in charge is
CatalogImpl, not the CatalogFacade.
Still all CatalogFacade.add( T ) also have return type T, but the
return value gets lost, since CatalogImpl issues the CatalogAddEvent
with its argument object, not with the one returned by the
CatalogFacade.

For instance:
CatalogImpl{

public void add(LayerInfo info){
…
facade.add(info);
added(info);
}
}

instead of
public void add(LayerInfo info){
…
LayerInfo added = facade.add(info);
added(added);
}

This happens to all add methods except add(LayerGroupInfo) which does
added(facade.add(layerGroup))

I’m under the impression all add methods should issue the add event
with the value returned by the facade, but fear to be missing
something.

Opinions?

Cheers,
Gabriel

Gabriel Roldan
OpenGeo - http://opengeo.org
Expert service straight from the developers.

Ridiculously easy VDI. With Citrix VDI-in-a-Box, you don’t need a complex
infrastructure or vast IT resources to deliver seamless, secure access to
virtual desktops. With this all-in-one solution, easily deploy virtual
desktops for less than the cost of PCs and save 60% on VDI infrastructure
costs. Try it free! http://p.sf.net/sfu/Citrix-VDIinabox

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.

–
Gabriel Roldan
OpenGeo - http://opengeo.org
Expert service straight from the developers.

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