Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BBOXTests#invalidGeometryOperand expects exception #147

Closed
lgoltz opened this issue Apr 15, 2019 · 12 comments · Fixed by #208
Closed

BBOXTests#invalidGeometryOperand expects exception #147

lgoltz opened this issue Apr 15, 2019 · 12 comments · Fixed by #208
Assignees
Labels
Projects
Milestone

Comments

@lgoltz
Copy link
Contributor

lgoltz commented Apr 15, 2019

The test org.opengis.cite.iso19142.basic.filter.spatial.BBOXTests#invalidGeometryOperand expects an exception with InvalidParameterValue code and status code 400.

Test:

/**
* [{@code Test}] Submits a GetFeature request where the BBOX predicate
* refers to a feature property (gml:description) that is not
* geometry-valued. An exception is expected in response with status code
* 400 and exception code {@code InvalidParameterValue}.
*
* @param featureType
* A QName representing the qualified name of a feature type for
* which instances exist.
*
* @see "ISO 19142:2010, 11.4: GetFeature - Exceptions"
* @see "ISO 19143:2010, 8.3: Exceptions"
*/
@Test(description = "See ISO 19142: 11.4; ISO 19143: 8.3", dataProvider = "instantiated-feature-types")
public void invalidGeometryOperand(QName featureType) {
XSElementDeclaration gmlDesc = this.model.getElementDeclaration("description", Namespaces.GML);
Element valueRef = WFSMessage.createValueReference(gmlDesc);
WFSMessage.appendSimpleQuery(this.reqEntity, featureType);
Envelope spatialExtent = featureInfo.get(featureType).getSpatialExtent();
Document gmlEnv = envelopeAsGML(spatialExtent);
addBBOXPredicate(this.reqEntity, gmlEnv.getDocumentElement(), valueRef);
ClientResponse rsp = wfsClient.submitRequest(reqEntity, ProtocolBinding.ANY);
this.rspEntity = rsp.getEntity(Document.class);
Assert.assertEquals(rsp.getStatus(), ClientResponse.Status.BAD_REQUEST.getStatusCode(),
ErrorMessage.get(ErrorMessageKeys.UNEXPECTED_STATUS));
String xpath = "//ows:Exception[@exceptionCode='InvalidParameterValue']";
ETSAssert.assertXPath(xpath, this.rspEntity, null);
}

Some services returns an empty FeatureCollection.

Specification should be rechecked.

@lgoltz
Copy link
Contributor Author

lgoltz commented Sep 10, 2019

I cannot find any hint in the specification about the correct behavior.

  • ATS: A.2.22.1.5.2 Invalid property name: "Test Method: Execute a query with an non-existent property name in the projection clause of a query and verify that the server generates an InvalidParameterValue exception."
  • referenced Chapter 11.4: GetFeature - Exceptions does not mention the InvalidParameterValue exception
  • referenced Chapter 8.3: Exceptions does not exist

I think the test should be removed.

@lgoltz lgoltz added this to the 1.33 milestone Sep 11, 2019
@dstenger
Copy link
Contributor

I agree regarding referenced chapter 11.4: GetFeature - Exceptions.

However, regarding Chapter 8.3 OWS Common (OGC 06-121r3) is referenced. Thus, the chapter exists.
The chapter says following:

exceptionCode value: InvalidParameterValue
Meaning of code: Operation request contains an invalid parameter value
“locator” value: Name of parameter with invalid value

Nevertheless, in my opinion the link to A.2.22.1.5.2 is not very clear.

@dstenger dstenger added this to To do in CITE via automation Dec 18, 2019
@dstenger dstenger modified the milestones: 1.33, 1.34 Apr 28, 2020
@dstenger dstenger moved this from To do to Needs discussion in CITE Apr 29, 2020
@dstenger dstenger assigned ghobona and unassigned lgoltz Apr 29, 2020
@dstenger dstenger removed this from the 1.34 milestone Jun 26, 2020
@dstenger dstenger moved this from Needs discussion to To do in CITE Sep 16, 2020
@dstenger
Copy link
Contributor

@ghobona What do you think of this issue? Shall the test be removed?

@ghobona
Copy link
Contributor

ghobona commented Sep 23, 2020

We should keep the test.

The standard is clear about the exception to be returned and the response code. Please see Section 7.5, 11.4 and the attached screenshot from Section D.3.

Screenshot 2020-09-23 at 10 48 00

CITE automation moved this from To do to Done Sep 28, 2020
@dstenger dstenger reopened this Sep 21, 2021
CITE automation moved this from Done to In progress Sep 21, 2021
@dstenger
Copy link
Contributor

Still, it is not clear for me whether this case (see description of issue) should lead to an exception (status code 400) or to an empty feature collection (status code 200).

The specification does not clearly define this special case.

The only abstract test coming close to this test was already referenced in #147 (comment). However, it does not clarify the described case.

@ghobona
Copy link
Contributor

ghobona commented Sep 27, 2021

@cportele May we please have the SWG's view on this issue?

@cportele
Copy link
Member

It is a general issue with WFS/FES that the requirements are sometimes not clear. But let's have a look at this case:

The test in the original comment references "ISO 19142:2010, 11.4: GetFeature - Exceptions" and "ISO 19143:2010, 8.3: Exceptions".

  • The tests should not reference the ISO standards, which are outdated (2.0.0 instead of 2.0.2).
  • Section 8.3 in FES is about exceptions for sorting, so it does not seem applicable.
  • Get Feature - Exceptions is section 11.3 in WFS. Based on the current text I would have excepted an OperationProcessingFailed exception, if there is an exception.
  • With respect to the question whether an exception or an empty feature collection should be returned, my reading of FES is that it should be an exception. A spatial operator as specified by FES (and ISO 19125-1) expects two geometries as operands. If the property is not a geometry (or NULL) then this is invalid input for the operator. The same applies, if a literal is not a geometry literal.

We can discuss this in the SWG meeting today.

cc @pvretano

@ghobona
Copy link
Contributor

ghobona commented Oct 4, 2021

@cportele Any further information from the SWG meeting?

@cportele
Copy link
Member

cportele commented Oct 4, 2021

We discussed it briefly. @pvretano agreed with my general interpretation and we decided that he will add another comment to this issue.

@pvretano
Copy link

pvretano commented Oct 4, 2021

@ghobona at its core the test is checking for an invalid Geometry operand and so if the operand does not resolve to a valid geometry (i.e. a property with a geometric value or a geometry literal) then an exception should be thrown as specified in the spec. So, @cportele's explanation is correct.

@dstenger
Copy link
Contributor

dstenger commented Oct 4, 2021

Thank you for your expertise.

@ghobona Thus, I propose to leave the test as it is and to also allow OperationProcessingFailed (Status Code 403) as valid exception type.

@ghobona
Copy link
Contributor

ghobona commented Oct 4, 2021

@dstenger Agreed.

bpross-52n added a commit that referenced this issue Oct 28, 2021
@dstenger dstenger moved this from In progress to To verify in CITE Nov 3, 2021
@dstenger dstenger assigned dstenger and unassigned bpross-52n Nov 3, 2021
@dstenger dstenger added bug and removed question labels Nov 3, 2021
@dstenger dstenger added this to the 1.37 milestone Nov 24, 2021
CITE automation moved this from To verify to Done Nov 24, 2021
dstenger added a commit that referenced this issue Nov 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
CITE
  
Done
Development

Successfully merging a pull request may close this issue.

7 participants