I found an interesting behavior in the GeoServer REST API today. Details described here: here https://osgeo-org.atlassian.net/browse/GEOS-7852
While it does not impact functionality of core GeoServer, it could be construed as a violation of the CatalodFacade interface / api.
Unless there was a good reason for this implementation, I think it should be fixed.
PR here: https://github.com/geoserver/geoserver/pull/1966
Opinions welcome.
Hi Torben,
wondering, wouldn’t it be better to add that same code to the CatalogImpl instead?
In order to make the same issue impossible from other (future and unaware) calling points?
Cheers
Andrea
On Tue, Nov 15, 2016 at 1:45 AM, Torben Barsballe <tbarsballe@anonymised.com> wrote:
I found an interesting behavior in the GeoServer REST API today. Details described here: here https://osgeo-org.atlassian.net/browse/GEOS-7852
While it does not impact functionality of core GeoServer, it could be construed as a violation of the CatalodFacade interface / api.
Unless there was a good reason for this implementation, I think it should be fixed.
PR here: https://github.com/geoserver/geoserver/pull/1966
Opinions welcome.
Geoserver-devel mailing list
Geoserver-devel@anonymised.com.366…sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
–
Ing. Andrea Aime
@geowolf
Technical Lead
GeoSolutions S.A.S.
Via di Montramito 3/A
55054 Massarosa (LU)
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549
http://www.geo-solutions.it
http://twitter.com/geosolutions_it
AVVERTENZE AI SENSI DEL D.Lgs. 196/2003
Le informazioni contenute in questo messaggio di posta elettronica e/o nel/i file/s allegato/i sono da considerarsi strettamente riservate. Il loro utilizzo è consentito esclusivamente al destinatario del messaggio, per le finalità indicate nel messaggio stesso. Qualora riceviate questo messaggio senza esserne il destinatario, Vi preghiamo cortesemente di darcene notizia via e-mail e di procedere alla distruzione del messaggio stesso, cancellandolo dal Vostro sistema. Conservare il messaggio stesso, divulgarlo anche in parte, distribuirlo ad altri soggetti, copiarlo, od utilizzarlo per finalità diverse, costituisce comportamento contrario ai principi dettati dal D.Lgs. 196/2003.
The information in this message and/or attachments, is intended solely for the attention and use of the named addressee(s) and may be confidential or proprietary in nature or covered by the provisions of privacy act (Legislative Decree June, 30 2003, no.196 - Italy’s New Data Protection Code).Any use not in accord with its purpose, any disclosure, reproduction, copying, distribution, or either dissemination, either whole or partial, is strictly forbidden except previous formal approval of the named addressee(s). If you are not the intended recipient, please contact immediately the sender by telephone, fax or e-mail and delete the information in this message that has been received in error. The sender does not give any warranty or accept liability as the content, accuracy or completeness of sent messages and accepts no responsibility for changes made after they were sent or for other risks which arise as a result of e-mail transmission, viruses, etc.
I agree that would be a good idea, although that would technically be a reduction in functionality… perhaps some sort of warning in the logs for 1 version, then removal later?
Torben
On Tue, Nov 15, 2016 at 6:35 AM, Andrea Aime <andrea.aime@anonymised.com> wrote:
Hi Torben,
wondering, wouldn’t it be better to add that same code to the CatalogImpl instead?
In order to make the same issue impossible from other (future and unaware) calling points?Cheers
Andrea
On Tue, Nov 15, 2016 at 1:45 AM, Torben Barsballe <tbarsballe@anonymised.com> wrote:
I found an interesting behavior in the GeoServer REST API today. Details described here: here https://osgeo-org.atlassian.net/browse/GEOS-7852
While it does not impact functionality of core GeoServer, it could be construed as a violation of the CatalodFacade interface / api.
Unless there was a good reason for this implementation, I think it should be fixed.
PR here: https://github.com/geoserver/geoserver/pull/1966
Opinions welcome.
Geoserver-devel mailing list
Geoserver-devel@anonymised.comrge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
–
Ing. Andrea Aime
@geowolf
Technical Lead
GeoSolutions S.A.S.
Via di Montramito 3/A
55054 Massarosa (LU)
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549
http://www.geo-solutions.it
http://twitter.com/geosolutions_it
AVVERTENZE AI SENSI DEL D.Lgs. 196/2003
Le informazioni contenute in questo messaggio di posta elettronica e/o nel/i file/s allegato/i sono da considerarsi strettamente riservate. Il loro utilizzo è consentito esclusivamente al destinatario del messaggio, per le finalità indicate nel messaggio stesso. Qualora riceviate questo messaggio senza esserne il destinatario, Vi preghiamo cortesemente di darcene notizia via e-mail e di procedere alla distruzione del messaggio stesso, cancellandolo dal Vostro sistema. Conservare il messaggio stesso, divulgarlo anche in parte, distribuirlo ad altri soggetti, copiarlo, od utilizzarlo per finalità diverse, costituisce comportamento contrario ai principi dettati dal D.Lgs. 196/2003.
The information in this message and/or attachments, is intended solely for the attention and use of the named addressee(s) and may be confidential or proprietary in nature or covered by the provisions of privacy act (Legislative Decree June, 30 2003, no.196 - Italy’s New Data Protection Code).Any use not in accord with its purpose, any disclosure, reproduction, copying, distribution, or either dissemination, either whole or partial, is strictly forbidden except previous formal approval of the named addressee(s). If you are not the intended recipient, please contact immediately the sender by telephone, fax or e-mail and delete the information in this message that has been received in error. The sender does not give any warranty or accept liability as the content, accuracy or completeness of sent messages and accepts no responsibility for changes made after they were sent or for other risks which arise as a result of e-mail transmission, viruses, etc.
On Tue, Nov 15, 2016 at 4:41 PM, Torben Barsballe <
tbarsballe@anonymised.com> wrote:
I agree that would be a good idea, although that would technically be a
reduction in functionality... perhaps some sort of warning in the logs for
1 version, then removal later?
I lost you, why would it be a regression? Being able to set a workspace
that is not even in the catalog
as the default seems to be a bug... but wait... I see this in CatalogImpl:
public void setDefaultWorkspace(WorkspaceInfo workspace) {
if (workspace != null &&
facade.getWorkspaceByName(workspace.getName()) == null) {
facade.add(workspace);
}
facade.setDefaultWorkspace(workspace);
}
So what about changing it roughly this way:
public void setDefaultWorkspace(WorkspaceInfo workspace) {
if (workspace == null) {
facade.setDefaultWorkspace(workspace);
}
WorkspaceInfo ws = facade.getWorkspaceByName(workspace.getName());
if(ws == null) {
facade.add(ws);
}
facade.setDefaultWorkspace(ws);
}
Wouldn't this solve your problem and prevent similar potential issues in
other places?
Cheers
Andrea
Ing. Andrea Aime
@geowolf
Technical Lead
GeoSolutions S.A.S.
Via di Montramito 3/A
55054 Massarosa (LU)
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549
http://www.geo-solutions.it
http://twitter.com/geosolutions_it
*AVVERTENZE AI SENSI DEL D.Lgs. 196/2003*
Le informazioni contenute in questo messaggio di posta elettronica e/o
nel/i file/s allegato/i sono da considerarsi strettamente riservate. Il
loro utilizzo è consentito esclusivamente al destinatario del messaggio,
per le finalità indicate nel messaggio stesso. Qualora riceviate questo
messaggio senza esserne il destinatario, Vi preghiamo cortesemente di
darcene notizia via e-mail e di procedere alla distruzione del messaggio
stesso, cancellandolo dal Vostro sistema. Conservare il messaggio stesso,
divulgarlo anche in parte, distribuirlo ad altri soggetti, copiarlo, od
utilizzarlo per finalità diverse, costituisce comportamento contrario ai
principi dettati dal D.Lgs. 196/2003.
The information in this message and/or attachments, is intended solely for
the attention and use of the named addressee(s) and may be confidential or
proprietary in nature or covered by the provisions of privacy act
(Legislative Decree June, 30 2003, no.196 - Italy's New Data Protection
Code).Any use not in accord with its purpose, any disclosure, reproduction,
copying, distribution, or either dissemination, either whole or partial, is
strictly forbidden except previous formal approval of the named
addressee(s). If you are not the intended recipient, please contact
immediately the sender by telephone, fax or e-mail and delete the
information in this message that has been received in error. The sender
does not give any warranty or accept liability as the content, accuracy or
completeness of sent messages and accepts no responsibility for changes
made after they were sent or for other risks which arise as a result of
e-mail transmission, viruses, etc.
-------------------------------------------------------
I see what you mean. Sorry, I was mixing up CatalogImpl and CatalogFacade in my head there (I was thinking you meant to remove the current workaround from the facade).
Yes, that change does make sense.
Torben
On Tue, Nov 15, 2016 at 10:23 AM, Andrea Aime <andrea.aime@anonymised.com> wrote:
On Tue, Nov 15, 2016 at 4:41 PM, Torben Barsballe <tbarsballe@anonymised.com> wrote:
I agree that would be a good idea, although that would technically be a reduction in functionality… perhaps some sort of warning in the logs for 1 version, then removal later?
I lost you, why would it be a regression? Being able to set a workspace that is not even in the catalog
as the default seems to be a bug… but wait… I see this in CatalogImpl:
public void setDefaultWorkspace(WorkspaceInfo workspace) {
if (workspace != null && facade.getWorkspaceByName(workspace.getName()) == null) {
facade.add(workspace);
}
facade.setDefaultWorkspace(workspace);
}
So what about changing it roughly this way:
public void setDefaultWorkspace(WorkspaceInfo workspace) {
if (workspace == null) {
facade.setDefaultWorkspace(workspace);
}
WorkspaceInfo ws = facade.getWorkspaceByName(workspace.getName());
if(ws == null) {
facade.add(ws);
}
facade.setDefaultWorkspace(ws);
}
Wouldn’t this solve your problem and prevent similar potential issues in other places?
Cheers
Andrea
–
Ing. Andrea Aime
@geowolf
Technical Lead
GeoSolutions S.A.S.
Via di Montramito 3/A
55054 Massarosa (LU)
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549
http://www.geo-solutions.it
http://twitter.com/geosolutions_it
AVVERTENZE AI SENSI DEL D.Lgs. 196/2003
Le informazioni contenute in questo messaggio di posta elettronica e/o nel/i file/s allegato/i sono da considerarsi strettamente riservate. Il loro utilizzo è consentito esclusivamente al destinatario del messaggio, per le finalità indicate nel messaggio stesso. Qualora riceviate questo messaggio senza esserne il destinatario, Vi preghiamo cortesemente di darcene notizia via e-mail e di procedere alla distruzione del messaggio stesso, cancellandolo dal Vostro sistema. Conservare il messaggio stesso, divulgarlo anche in parte, distribuirlo ad altri soggetti, copiarlo, od utilizzarlo per finalità diverse, costituisce comportamento contrario ai principi dettati dal D.Lgs. 196/2003.
The information in this message and/or attachments, is intended solely for the attention and use of the named addressee(s) and may be confidential or proprietary in nature or covered by the provisions of privacy act (Legislative Decree June, 30 2003, no.196 - Italy’s New Data Protection Code).Any use not in accord with its purpose, any disclosure, reproduction, copying, distribution, or either dissemination, either whole or partial, is strictly forbidden except previous formal approval of the named addressee(s). If you are not the intended recipient, please contact immediately the sender by telephone, fax or e-mail and delete the information in this message that has been received in error. The sender does not give any warranty or accept liability as the content, accuracy or completeness of sent messages and accepts no responsibility for changes made after they were sent or for other risks which arise as a result of e-mail transmission, viruses, etc.
Looking at the CatalogImpl implementation of setDefaultNamespace(), we already do something like this there:
public void setDefaultNamespace(NamespaceInfo defaultNamespace) {
if(defaultNamespace != null) {
NamespaceInfo ns = getNamespaceByPrefix( defaultNamespace.getPrefix() );
if ( ns == null ) {
throw new IllegalArgumentException( “No such namespace: '” + defaultNamespace.getPrefix() + “'” );
}
}
facade.setDefaultNamespace(defaultNamespace);
}
However, rather than adding the namespace, we throw an exception. Do we want to do the same for workspace (throw an exception if it is not in the Catalog)?
Personally, I think throwing an exception makes a bit more sense than adding it to the catalog (Especially from the REST API perspective - consider trying to set the default workspace to some existing workspace but making a typo in the name - having a new workspace added to the catalog is not what I would expect in that situation)
Torben
On Tue, Nov 15, 2016 at 10:27 AM, Torben Barsballe <tbarsballe@anonymised.com3839…> wrote:
I see what you mean. Sorry, I was mixing up CatalogImpl and CatalogFacade in my head there (I was thinking you meant to remove the current workaround from the facade).
Yes, that change does make sense.
Torben
On Tue, Nov 15, 2016 at 10:23 AM, Andrea Aime <andrea.aime@anonymised.com1268…> wrote:
On Tue, Nov 15, 2016 at 4:41 PM, Torben Barsballe <tbarsballe@anonymised.com> wrote:
I agree that would be a good idea, although that would technically be a reduction in functionality… perhaps some sort of warning in the logs for 1 version, then removal later?
I lost you, why would it be a regression? Being able to set a workspace that is not even in the catalog
as the default seems to be a bug… but wait… I see this in CatalogImpl:
public void setDefaultWorkspace(WorkspaceInfo workspace) {
if (workspace != null && facade.getWorkspaceByName(workspace.getName()) == null) {
facade.add(workspace);
}
facade.setDefaultWorkspace(workspace);
}
So what about changing it roughly this way:
public void setDefaultWorkspace(WorkspaceInfo workspace) {
if (workspace == null) {
facade.setDefaultWorkspace(workspace);
}
WorkspaceInfo ws = facade.getWorkspaceByName(workspace.getName());
if(ws == null) {
facade.add(ws);
}
facade.setDefaultWorkspace(ws);
}
Wouldn’t this solve your problem and prevent similar potential issues in other places?
Cheers
Andrea
–
Ing. Andrea Aime
@geowolf
Technical Lead
GeoSolutions S.A.S.
Via di Montramito 3/A
55054 Massarosa (LU)
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549
http://www.geo-solutions.it
http://twitter.com/geosolutions_it
AVVERTENZE AI SENSI DEL D.Lgs. 196/2003
Le informazioni contenute in questo messaggio di posta elettronica e/o nel/i file/s allegato/i sono da considerarsi strettamente riservate. Il loro utilizzo è consentito esclusivamente al destinatario del messaggio, per le finalità indicate nel messaggio stesso. Qualora riceviate questo messaggio senza esserne il destinatario, Vi preghiamo cortesemente di darcene notizia via e-mail e di procedere alla distruzione del messaggio stesso, cancellandolo dal Vostro sistema. Conservare il messaggio stesso, divulgarlo anche in parte, distribuirlo ad altri soggetti, copiarlo, od utilizzarlo per finalità diverse, costituisce comportamento contrario ai principi dettati dal D.Lgs. 196/2003.
The information in this message and/or attachments, is intended solely for the attention and use of the named addressee(s) and may be confidential or proprietary in nature or covered by the provisions of privacy act (Legislative Decree June, 30 2003, no.196 - Italy’s New Data Protection Code).Any use not in accord with its purpose, any disclosure, reproduction, copying, distribution, or either dissemination, either whole or partial, is strictly forbidden except previous formal approval of the named addressee(s). If you are not the intended recipient, please contact immediately the sender by telephone, fax or e-mail and delete the information in this message that has been received in error. The sender does not give any warranty or accept liability as the content, accuracy or completeness of sent messages and accepts no responsibility for changes made after they were sent or for other risks which arise as a result of e-mail transmission, viruses, etc.
On Tue, Nov 15, 2016 at 7:59 PM, Torben Barsballe <
tbarsballe@anonymised.com> wrote:
Looking at the CatalogImpl implementation of setDefaultNamespace(), we
already do something like this there:public void setDefaultNamespace(NamespaceInfo defaultNamespace) {
if(defaultNamespace != null) {
NamespaceInfo ns = getNamespaceByPrefix(
defaultNamespace.getPrefix() );
if ( ns == null ) {
throw new IllegalArgumentException( "No such namespace: '"
+ defaultNamespace.getPrefix() + "'" );
}
}
facade.setDefaultNamespace(defaultNamespace);
}However, rather than adding the namespace, we throw an exception. Do we
want to do the same for workspace (throw an exception if it is not in the
Catalog)?Personally, I think throwing an exception makes a bit more sense than
adding it to the catalog (Especially from the REST API perspective -
consider trying to set the default workspace to some existing workspace but
making a typo in the name - having a new workspace added to the catalog is
not what I would expect in that situation)
I like throwing the exception more myself too
Cheers
Andrea
Ing. Andrea Aime
@geowolf
Technical Lead
GeoSolutions S.A.S.
Via di Montramito 3/A
55054 Massarosa (LU)
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549
http://www.geo-solutions.it
http://twitter.com/geosolutions_it
*AVVERTENZE AI SENSI DEL D.Lgs. 196/2003*
Le informazioni contenute in questo messaggio di posta elettronica e/o
nel/i file/s allegato/i sono da considerarsi strettamente riservate. Il
loro utilizzo è consentito esclusivamente al destinatario del messaggio,
per le finalità indicate nel messaggio stesso. Qualora riceviate questo
messaggio senza esserne il destinatario, Vi preghiamo cortesemente di
darcene notizia via e-mail e di procedere alla distruzione del messaggio
stesso, cancellandolo dal Vostro sistema. Conservare il messaggio stesso,
divulgarlo anche in parte, distribuirlo ad altri soggetti, copiarlo, od
utilizzarlo per finalità diverse, costituisce comportamento contrario ai
principi dettati dal D.Lgs. 196/2003.
The information in this message and/or attachments, is intended solely for
the attention and use of the named addressee(s) and may be confidential or
proprietary in nature or covered by the provisions of privacy act
(Legislative Decree June, 30 2003, no.196 - Italy's New Data Protection
Code).Any use not in accord with its purpose, any disclosure, reproduction,
copying, distribution, or either dissemination, either whole or partial, is
strictly forbidden except previous formal approval of the named
addressee(s). If you are not the intended recipient, please contact
immediately the sender by telephone, fax or e-mail and delete the
information in this message that has been received in error. The sender
does not give any warranty or accept liability as the content, accuracy or
completeness of sent messages and accepts no responsibility for changes
made after they were sent or for other risks which arise as a result of
e-mail transmission, viruses, etc.
-------------------------------------------------------