[Geoserver-devel] WFS-T request updates table twice

Hi,

I’ve got a case here where a WFS-T request ends up doing the UDPATE clause twice to the database table (using jdbc-oracle). This GeoServer that has a custom ResourceAccessManager that returns VectorAccessLimits. I’m using GeoServer 2.6.2, but I see no relevant updates to the code I’m referencing here in 2.6.x nor master.

Debugging this ended up me looking here: https://github.com/geoserver/geoserver/blob/master/src/main/src/main/java/org/geoserver/security/decorators/SecuredSimpleFeatureStore.java#L60

The case I’m looking at ends up going to storeDelegate.modifyFeatures() twice, on rows #65 and #77.

I’ve copied the relevant statements below. If the first if statement is true, then writeQuery.getPropertyNames() is a constant (=null) that is always equal to Query.ALL_NAMES (=null). What’s the reasoning behind this? Is there a case where it makes sense for this method to do two updates?

Query writeQuery = getWriteQuery(policy);
if (writeQuery == Query.ALL) {
((SimpleFeatureStore) storeDelegate).modifyFeatures(names, values, filter);
} else if (writeQuery.getFilter() == Filter.EXCLUDE

writeQuery.getPropertyNames() == Query.NO_NAMES) {
throw unsupportedOperation();
}

// get the mixed filter
final Query local = new Query(null, filter);
Query mixed = mixQueries(local, writeQuery);

if (writeQuery.getPropertyNames() == Query.ALL_NAMES) {
// it was just a matter of filtering.
((SimpleFeatureStore) storeDelegate).modifyFeatures(names, values, mixed.getFilter());
} else {

Sampo

···

Sampo Savolainen
R&D Director, Spatineo Oy
sampo.savolainen@anonymised.com
+358-407555649
Linnankoskenkatu 16 A 17, 00250 Helsinki, Finland
www.spatineo.com, twitter.com/#!/spatineo
www.linkedin.com/company/spatineo-inc

This message may contain privileged and/or confidential information. If you
have received this e-mail in error or are not the intended recipient, you
may not use, copy, disseminate, or distribute it; do not open any
attachments, delete it immediately from your system and notify the sender
promptly by e-mail that you have done so.

Hi,

A quick update. This double update issue happens when the ResourceAccessManager implementation returns a “new VectorAccessLimits(CatalogMode.HIDE, null, Filter.INCLUDE, null, Filter.INCLUDE);” (which means no access limits at all for this particlar resource)

For example https://github.com/geoserver/geoserver/blob/master/src/main/src/main/java/org/geoserver/security/DataAccessManagerAdapter.java never returns such objects. The buildLimits() method of this class detects if the user has unrestricted access to the resoruce in question and returns “null” the limits. This is probably the reason why this issue hasn’t come up before.

As a workaround, I’ve switched the ResourceAccessManager in question to return null for such cases as well. However I think this is a serious issue in GS that needs to be looked at by someone who knows this design better. Jody, any comments?

Sampo

···

On Mon, Jun 29, 2015 at 12:45 PM, Sampo Savolainen <sampo.savolainen@anonymised.com…> wrote:

Hi,

I’ve got a case here where a WFS-T request ends up doing the UDPATE clause twice to the database table (using jdbc-oracle). This GeoServer that has a custom ResourceAccessManager that returns VectorAccessLimits. I’m using GeoServer 2.6.2, but I see no relevant updates to the code I’m referencing here in 2.6.x nor master.

Debugging this ended up me looking here: https://github.com/geoserver/geoserver/blob/master/src/main/src/main/java/org/geoserver/security/decorators/SecuredSimpleFeatureStore.java#L60

The case I’m looking at ends up going to storeDelegate.modifyFeatures() twice, on rows #65 and #77.

I’ve copied the relevant statements below. If the first if statement is true, then writeQuery.getPropertyNames() is a constant (=null) that is always equal to Query.ALL_NAMES (=null). What’s the reasoning behind this? Is there a case where it makes sense for this method to do two updates?

Query writeQuery = getWriteQuery(policy);
if (writeQuery == Query.ALL) {
((SimpleFeatureStore) storeDelegate).modifyFeatures(names, values, filter);
} else if (writeQuery.getFilter() == Filter.EXCLUDE
|| writeQuery.getPropertyNames() == Query.NO_NAMES) {
throw unsupportedOperation();
}

// get the mixed filter
final Query local = new Query(null, filter);
Query mixed = mixQueries(local, writeQuery);

if (writeQuery.getPropertyNames() == Query.ALL_NAMES) {
// it was just a matter of filtering.
((SimpleFeatureStore) storeDelegate).modifyFeatures(names, values, mixed.getFilter());
} else {

Sampo

Sampo Savolainen
R&D Director, Spatineo Oy
sampo.savolainen@anonymised.com
+358-407555649
Linnankoskenkatu 16 A 17, 00250 Helsinki, Finland
www.spatineo.com, twitter.com/#!/spatineo
www.linkedin.com/company/spatineo-inc

This message may contain privileged and/or confidential information. If you
have received this e-mail in error or are not the intended recipient, you
may not use, copy, disseminate, or distribute it; do not open any
attachments, delete it immediately from your system and notify the sender
promptly by e-mail that you have done so.

Sampo Savolainen
R&D Director, Spatineo Oy
sampo.savolainen@…3889…
+358-407555649
Linnankoskenkatu 16 A 17, 00250 Helsinki, Finland
www.spatineo.com, twitter.com/#!/spatineo
www.linkedin.com/company/spatineo-inc

This message may contain privileged and/or confidential information. If you
have received this e-mail in error or are not the intended recipient, you
may not use, copy, disseminate, or distribute it; do not open any
attachments, delete it immediately from your system and notify the sender
promptly by e-mail that you have done so.

On Mon, Jun 29, 2015 at 12:45 PM, Sampo Savolainen <
sampo.savolainen@anonymised.com> wrote:

Hi,

A quick update. This double update issue happens when the
ResourceAccessManager implementation returns a "new
VectorAccessLimits(CatalogMode.HIDE, null, Filter.INCLUDE, null,
Filter.INCLUDE);" (which means no access limits at all for this particlar
resource)

For example
https://github.com/geoserver/geoserver/blob/master/src/main/src/main/java/org/geoserver/security/DataAccessManagerAdapter.java
never returns such objects. The buildLimits() method of this class detects
if the user has unrestricted access to the resoruce in question and returns
"null" the limits. This is probably the reason why this issue hasn't come
up before.

As a workaround, I've switched the ResourceAccessManager in question to
return null for such cases as well. However I think this is a serious issue
in GS that needs to be looked at by someone who knows this design better.
Jody, any comments?

I'd suggest you open a ticket on the bug tracker. Not sure if Jody has any
familiarity with those classes, I'm the one that wrote them (but then
again, maybe he has studied them recently).

Cheers
Andrea

--

GeoServer Professional Services from the experts! Visit
http://goo.gl/it488V for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
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.

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

Done: GEOS-7092

I mentioned Jody as I saw his name on the git logs for these components. I guess that’s just an artifact from switching over to github.

Sampo

···

On Mon, Jun 29, 2015 at 2:03 PM, Andrea Aime <andrea.aime@anonymised.com> wrote:

On Mon, Jun 29, 2015 at 12:45 PM, Sampo Savolainen <sampo.savolainen@anonymised.com> wrote:

Hi,

A quick update. This double update issue happens when the ResourceAccessManager implementation returns a “new VectorAccessLimits(CatalogMode.HIDE, null, Filter.INCLUDE, null, Filter.INCLUDE);” (which means no access limits at all for this particlar resource)

For example https://github.com/geoserver/geoserver/blob/master/src/main/src/main/java/org/geoserver/security/DataAccessManagerAdapter.java never returns such objects. The buildLimits() method of this class detects if the user has unrestricted access to the resoruce in question and returns “null” the limits. This is probably the reason why this issue hasn’t come up before.

As a workaround, I’ve switched the ResourceAccessManager in question to return null for such cases as well. However I think this is a serious issue in GS that needs to be looked at by someone who knows this design better. Jody, any comments?

I’d suggest you open a ticket on the bug tracker. Not sure if Jody has any familiarity with those classes, I’m the one that wrote them (but then again, maybe he has studied them recently).

Cheers
Andrea

==
GeoServer Professional Services from the experts! Visit
http://goo.gl/it488V for more information.

Ing. Andrea Aime

@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
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.


Sampo Savolainen
R&D Director, Spatineo Oy
sampo.savolainen@…3889…
+358-407555649
Linnankoskenkatu 16 A 17, 00250 Helsinki, Finland
www.spatineo.com, twitter.com/#!/spatineo
www.linkedin.com/company/spatineo-inc

This message may contain privileged and/or confidential information. If you
have received this e-mail in error or are not the intended recipient, you
may not use, copy, disseminate, or distribute it; do not open any
attachments, delete it immediately from your system and notify the sender
promptly by e-mail that you have done so.