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

Bug fixes, GetFeatureInfo tests, RESTful tests #78

Merged
merged 10 commits into from
Jan 29, 2024

Conversation

cmorriscmps
Copy link

Contains bug fixes as well as options to execute tests for the GetFeatureInfo operation and the REST encoding.

Summary of Changes:

wmtsFunctions.xml

  • Fixed many broken tests by globally replacing "$wmts.capabilities.doc/" with "$wmts.capabilities.doc//" in wmtsFunctions.xml.
  • Fixed layer selection XPaths to make sure that the layer tested does not contain a dimension without a default.
  • Changed the GetTile.Valid.Tile.Transparency test so that setting up a transparent layer is not required. Even if no layer contains the keyword "transparent" in its metadata, the test will still check a layer for transparency and simply return a warning if no transparent pixels are found.
  • Removed redundant tests GetCapabilities.Response.Layer.LegendURL.Correct.Format and GetCapabilities.Response.Layer.LegendURL.Correct.Size since they are fully covered by GetCapabilities.Response.Layer.LegendURL.Correct.Resource.
  • Fixed legend URL checks to not require height, width, minScaleDenominator, and maxScaleDenominator attributes on legend URLs.
  • Fixed bugs in GetFeatureInfo and REST tests.

WMTS_ETS.xml

  • Moved starting form from the wmts:main test to the suite element to simplify adding options.
  • Added option to fail early on validation errors.
  • Added options to enable GetFeatureInfo and REST tests.

owsFunctions.xml

  • Fixed bugs in owsFunctions:validateExceptionReport function.

bootstrap.xml

  • Fixed test prerequisites as necessary

Chuck Morris added 4 commits September 13, 2023 15:05
@dstenger dstenger self-requested a review November 9, 2023 09:43
Copy link
Contributor

@dstenger dstenger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for providing the pull request.
However, when testing the pull request, the current reference implementation does not pass all mandatory tests anymore: https://cite.deegree.org/deegree-webservices-3.5.0/services/wmts100?service=WMTS&request=GetCapabilities
This is because group GetTile.Implemented.KVP.GET is now executed which was not the case before. One of the test fails making the complete conformance class GetTile.main fail.
Before merging the PR it must be clarified if this behavior is correct.

@dstenger
Copy link
Contributor

dstenger commented Nov 13, 2023

This pull request will be on hold until opengeospatial/teamengine#593 is merged and installed on Beta.

@cmorriscmps
Copy link
Author

The reference implementation @dstenger provided was failing due to an issue with style selection in the Server.KVP.GET.GetTile.Valid.Tile.Transparency test. I've fixed it and the RI now passes.

@dstenger dstenger self-requested a review November 14, 2023 17:22
Copy link
Contributor

@dstenger dstenger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cmorriscmps Thank you for the fix. I can confirm that the reference implementation passes all tests again, now.

@dstenger dstenger modified the milestone: 1.6 Dec 11, 2023
@dstenger dstenger self-requested a review December 15, 2023 21:28
Copy link
Contributor

@dstenger dstenger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that this pull request breaks the REST interface.

When requesting http://localhost:8081/teamengine/rest/suites/, following stack trace is returned:

Type Exception Report

Message java.util.ServiceConfigurationError: com.occamlab.te.spi.jaxrs.TestSuiteController: Provider org.opengis.cite.wmts10.CtlController could not be instantiated

Description The server encountered an unexpected condition that prevented it from fulfilling the request.

Exception

javax.servlet.ServletException: java.util.ServiceConfigurationError: com.occamlab.te.spi.jaxrs.TestSuiteController: Provider org.opengis.cite.wmts10.CtlController could not be instantiated
	com.sun.jersey.spi.container.servlet.WebComponent.service(WebComponent.java:420)
	com.sun.jersey.spi.container.servlet.ServletContainer.service(ServletContainer.java:558)
	com.sun.jersey.spi.container.servlet.ServletContainer.service(ServletContainer.java:733)
	javax.servlet.http.HttpServlet.service(HttpServlet.java:731)
	org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:52)
	com.occamlab.te.realm.RestAuthenticationFilter.doFilter(RestAuthenticationFilter.java:72)

Root Cause

java.util.ServiceConfigurationError: com.occamlab.te.spi.jaxrs.TestSuiteController: Provider org.opengis.cite.wmts10.CtlController could not be instantiated
	java.util.ServiceLoader.fail(ServiceLoader.java:232)
	java.util.ServiceLoader.access$100(ServiceLoader.java:185)
	java.util.ServiceLoader$LazyIterator.nextService(ServiceLoader.java:384)
	java.util.ServiceLoader$LazyIterator.next(ServiceLoader.java:404)
	java.util.ServiceLoader$1.next(ServiceLoader.java:480)
	com.occamlab.te.spi.jaxrs.TestSuiteRegistry.loadControllers(TestSuiteRegistry.java:96)
	com.occamlab.te.spi.jaxrs.TestSuiteRegistry.<init>(TestSuiteRegistry.java:89)
	com.occamlab.te.spi.jaxrs.TestSuiteRegistry.getInstance(TestSuiteRegistry.java:53)
	com.occamlab.te.spi.jaxrs.resources.TestSuiteSetResource.listTestSuites(TestSuiteSetResource.java:91)
	sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	java.lang.reflect.Method.invoke(Method.java:498)
	com.sun.jersey.spi.container.JavaMethodInvokerFactory$1.invoke(JavaMethodInvokerFactory.java:60)
	com.sun.jersey.server.impl.model.method.dispatch.AbstractResourceMethodDispatchProvider$TypeOutInvoker._dispatch(AbstractResourceMethodDispatchProvider.java:185)
	com.sun.jersey.server.impl.model.method.dispatch.ResourceJavaMethodDispatcher.dispatch(ResourceJavaMethodDispatcher.java:75)
	com.sun.jersey.server.impl.uri.rules.HttpMethodRule.accept(HttpMethodRule.java:302)
	com.sun.jersey.server.impl.uri.rules.ResourceClassRule.accept(ResourceClassRule.java:108)
	com.sun.jersey.server.impl.uri.rules.RightHandPathRule.accept(RightHandPathRule.java:147)
	com.sun.jersey.server.impl.uri.rules.RootResourceClassesRule.accept(RootResourceClassesRule.java:84)
	com.sun.jersey.server.impl.application.WebApplicationImpl._handleRequest(WebApplicationImpl.java:1542)
	com.sun.jersey.server.impl.application.WebApplicationImpl._handleRequest(WebApplicationImpl.java:1473)
	com.sun.jersey.server.impl.application.WebApplicationImpl.handleRequest(WebApplicationImpl.java:1419)
	com.sun.jersey.server.impl.application.WebApplicationImpl.handleRequest(WebApplicationImpl.java:1409)
	com.sun.jersey.spi.container.servlet.WebComponent.service(WebComponent.java:409)
	com.sun.jersey.spi.container.servlet.ServletContainer.service(ServletContainer.java:558)
	com.sun.jersey.spi.container.servlet.ServletContainer.service(ServletContainer.java:733)
	javax.servlet.http.HttpServlet.service(HttpServlet.java:731)
	org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:52)
	com.occamlab.te.realm.RestAuthenticationFilter.doFilter(RestAuthenticationFilter.java:72)

Root Cause

java.lang.NullPointerException
	org.opengis.cite.wmts10.CtlController.findScriptFile(CtlController.java:121)
	org.opengis.cite.wmts10.CtlController.<init>(CtlController.java:39)
	sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	java.lang.reflect.Constructor.newInstance(Constructor.java:423)
	java.lang.Class.newInstance(Class.java:442)
	java.util.ServiceLoader$LazyIterator.nextService(ServiceLoader.java:380)
	java.util.ServiceLoader$LazyIterator.next(ServiceLoader.java:404)
	java.util.ServiceLoader$1.next(ServiceLoader.java:480)
	com.occamlab.te.spi.jaxrs.TestSuiteRegistry.loadControllers(TestSuiteRegistry.java:96)
	com.occamlab.te.spi.jaxrs.TestSuiteRegistry.<init>(TestSuiteRegistry.java:89)
	com.occamlab.te.spi.jaxrs.TestSuiteRegistry.getInstance(TestSuiteRegistry.java:53)
	com.occamlab.te.spi.jaxrs.resources.TestSuiteSetResource.listTestSuites(TestSuiteSetResource.java:91)
	sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	java.lang.reflect.Method.invoke(Method.java:498)
	com.sun.jersey.spi.container.JavaMethodInvokerFactory$1.invoke(JavaMethodInvokerFactory.java:60)
	com.sun.jersey.server.impl.model.method.dispatch.AbstractResourceMethodDispatchProvider$TypeOutInvoker._dispatch(AbstractResourceMethodDispatchProvider.java:185)
	com.sun.jersey.server.impl.model.method.dispatch.ResourceJavaMethodDispatcher.dispatch(ResourceJavaMethodDispatcher.java:75)
	com.sun.jersey.server.impl.uri.rules.HttpMethodRule.accept(HttpMethodRule.java:302)
	com.sun.jersey.server.impl.uri.rules.ResourceClassRule.accept(ResourceClassRule.java:108)
	com.sun.jersey.server.impl.uri.rules.RightHandPathRule.accept(RightHandPathRule.java:147)
	com.sun.jersey.server.impl.uri.rules.RootResourceClassesRule.accept(RootResourceClassesRule.java:84)
	com.sun.jersey.server.impl.application.WebApplicationImpl._handleRequest(WebApplicationImpl.java:1542)
	com.sun.jersey.server.impl.application.WebApplicationImpl._handleRequest(WebApplicationImpl.java:1473)
	com.sun.jersey.server.impl.application.WebApplicationImpl.handleRequest(WebApplicationImpl.java:1419)
	com.sun.jersey.server.impl.application.WebApplicationImpl.handleRequest(WebApplicationImpl.java:1409)
	com.sun.jersey.spi.container.servlet.WebComponent.service(WebComponent.java:409)
	com.sun.jersey.spi.container.servlet.ServletContainer.service(ServletContainer.java:558)
	com.sun.jersey.spi.container.servlet.ServletContainer.service(ServletContainer.java:733)
	javax.servlet.http.HttpServlet.service(HttpServlet.java:731)
	org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:52)
	com.occamlab.te.realm.RestAuthenticationFilter.doFilter(RestAuthenticationFilter.java:72)

Copy link
Contributor

@dstenger dstenger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error with the REST interface is occurring because script src/main/scripts/ctl/wmts-auto.xml was deleted which is set as main-script in ets.properties file.

@cmorriscmps Can you recall why this script was deleted?

@cmorriscmps
Copy link
Author

I really have no idea what the purpose of wmts-auto.xml was supposed to be. It appeared to be a copy of WMTS_ETS.xml, except wmts-auto.xml includes other files with xinclude. It doesn't really make sense to maintain two copies of the file. The config.xml file references the ctl directory, which loads all of the ctl files. They are all loaded twice if wmts-auto.xml is included.

@dstenger
Copy link
Contributor

dstenger commented Jan 2, 2024

The wmts-auto.xml file is used by the CtlController which is executed by the already mentioned REST interface and command line interface.
In addition to the includes, graphics content (java.awt) is removed.

I just checked the file and it seems that all changes are required by the REST interface.

So, I propose to restore the file to make this pull requests merge able.

However, I agree that the implementation should reduce duplication. So, we could create a new issue with the aim to improve/simplify/merge the two main CTL scripts.

@dstenger
Copy link
Contributor

@cmorriscmps
I will restore wmts-auto.xmlwhen I merge the pull requests to make it workable with the REST interface again.

Also, I am currently doing a functional review of the pull request and come back to you if I have any questions.

@dstenger
Copy link
Contributor

I did a functional review of the pull request and these are my results.

Questions:

  • What is the use case of the new option Fail early on schema validation errors? Can it be used to speed up test runs?

This should be adjusted:

  • Upload button for WMTS capabilities document is missing in Web Browser Interface and should be added again.
  • Abstract Test Suite (ATS) should also be adjusted (https://github.com/opengeospatial/ets-wmts10/blob/master/src/site/resources/ats.html). E.g. replaced tests should be removed.
  • As WMTS_ETS.xml was improved, wmts-auto.xml must be added and adjusted accordingly. wmts-auto.xml is used by REST interface.
    • It should be made sure that following tests are executed by the REST interface: The added mandatory tests of GetCapabilities.main and GetTile.main conformance classes.
    • Unofficial tests for GetFeatureInfo and RESTful Encoding shall not be executed.
    • Optionally, the user can control via a parameter which conformance classes are executed.

Further comments:

  • Re-enabling and fixing tests is useful.
  • All added tests in GetCapabilities.main and GetTile.main conformance classes are marked as mandatory by ATS. Thus, adding them improves the test suite. Background: This pull request originally disabled them: Commented the test which has the type 'MandatoryIfImplemented' #28.
  • The reference implementation passes all added tests (or skips them).
  • Test Server.KVP.GET.GetCapabilities.Request.Sections.Contents (enabled by this pull request) is not listed in ATS. However, the test was not modified by this pull request. Thus, this is a separate issue.
  • Re-enabling unofficial tests for GetFeatureInfo and RESTful Encoding in Web Browser Interface might be useful. However, those cannot be used for certification. Thus, an additional hint might be added. To be discussed with @ghobona.

@dstenger
Copy link
Contributor

dstenger commented Jan 22, 2024

@cmorriscmps
Thank you for the good pull request again.
However, there are some things which need to be adjusted. Can you please check the list under This should be adjusted: in comment #78 (comment).
Also, the options to enable GetFeatureInfo and REST tests shall be renamed to Include GetFeatureInfo Tests (optional) and Include RESTful Encoding Tests (optional) in the UI.
If it is not possible for you to work on the changes, please contact me as I can support you here.

@cmorriscmps
Copy link
Author

I'll take a closer look and respond by the end of the week.

@cmorriscmps
Copy link
Author

To answer the question about the "Fail early on schema validation errors" option, If the option is selected then the root test will validate the capabilities document against the schema and fail without executing any subtests if validation fails. So yes, it will speed up test runs if validation fails.

I agree that the upload button for a capabilities document and that the wmts-auto.xml file should be restored. I can update the pull request with these changes.

I also agree that the ATS needs to be updated to reflect the current state of the ETS. However, it has been out of date long before this pull request was introduced, so I don't think that updating it falls under the scope of this pull request.

@dstenger
Copy link
Contributor

Thank you for updating the pull request.
Most problems are solved, now.
There are some small tasks which are still pending:

<soapui.tests.passed>39</soapui.tests.passed>
<soapui.tests.skipped>20</soapui.tests.skipped>

@cmorriscmps
Are you still working on updating the pull request? Can you please give me a short update if you need any assistance?

@cmorriscmps
Copy link
Author

@dstenger
I'm done with the pull request.

I've fixed the optional test labels.

The Upload capability has been restored. There is no Upload button on the initial form, but if you leave the Capabilities URL field blank, it will present another form asking for a file upload.

Feel free to update any other wording however you and/or Gobe think is appropriate. I'll also leave updating the ATS doc and the pom.xml up to you.

You can accept the pull request and make additional changes in the opengeospatial repository, or I've given you permission to edit the pull request if you would prefer to make changes there.

@dstenger
Copy link
Contributor

dstenger commented Jan 26, 2024

Thank you for updating the pull request.
I solved the last two open tasks:

  • Update soapui properties
  • Update ATS to mark redundant tests
    • I decided to mark tests GetCapabilities.Response.Layer.LegendURL.Correct.Format and GetCapabilities.Response.Layer.LegendURL.Correct.Size with an attention notice instead of removing them as the description of the abstract tests might be helpful. ATS must be refactored in a separate issue.

@dstenger dstenger self-requested a review January 26, 2024 12:07
@dstenger
Copy link
Contributor

Further, I created a new issue for an update of the ATS: #83

@dstenger dstenger merged commit 8ac9572 into opengeospatial:master Jan 29, 2024
@dstenger dstenger added this to the 1.7 milestone Jan 29, 2024
@ghobona
Copy link
Contributor

ghobona commented Feb 5, 2024

This PR has now been installed on the Beta validator.

https://cite.ogc.org/te2/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants