icatproject / icat.server

The ICAT server offering both SOAP and "RESTlike" interfaces to a metadata catalog.
Other
1 stars 5 forks source link

Allow multiple field selection in SOAP interface #246

Closed louise-davies closed 3 years ago

louise-davies commented 3 years ago

This PR fixes the issue where an error would occur if a user attempts to use the SOAP interface for a query such as SELECT i.name, i.title FROM Investigation i. It creates a FieldSet entity that enables to marshaller to convert the results into XML. We have to perform some type introspection in order to be able to cast the results to FieldSet objects so they can be marshalled.

RKrahl commented 3 years ago

Tested this on the client side, see icatproject/python-icat#78. Works fine.

RKrahl commented 3 years ago

This implementation fails if multiple fields are selected and any of them is a related entity object (as opposed to an attribute or an attribute of a related object).

E.g., SELECT ds.investigation, ds.name FROM Dataset ds fails, while SELECT ds.investigation.name, ds.name FROM Dataset ds and SELECT ds.investigation FROM Dataset ds work.

The first case will trigger an uncaught exception in icat.server:

[2021-02-19T15:22:27.388+0100] [Payara 4.1] [WARNING] [] [javax.enterprise.webservices] [tid: _ThreadID=36 _ThreadName=http-thread-pool::http-listener-2(1)] [timeMillis: 1613744547388] [lev
elValue: 900] [[
  invocation error on ejb endpoint ICAT at /ICATService/ICAT : javax.xml.bind.MarshalException
 - with linked exception:
[com.sun.istack.SAXException2: A cycle is detected in the object graph. This will cause infinitely deep XML: Datafile:357 -> DatafileFormat:162 -> Datafile:357].
javax.xml.ws.WebServiceException: javax.xml.bind.MarshalException
 - with linked exception:
[com.sun.istack.SAXException2: A cycle is detected in the object graph. This will cause infinitely deep XML: Datafile:357 -> DatafileFormat:162 -> Datafile:357]
        at com.sun.xml.ws.message.jaxb.JAXBMessage.writePayloadTo(JAXBMessage.java:424)
        at com.sun.xml.ws.message.AbstractMessageImpl.writeTo(AbstractMessageImpl.java:192)
        at com.sun.xml.ws.api.message.MessageWrapper.writeTo(MessageWrapper.java:226)
        at com.sun.xml.ws.encoding.StreamSOAPCodec.encode(StreamSOAPCodec.java:144)
        at com.sun.xml.ws.encoding.SOAPBindingCodec.encode(SOAPBindingCodec.java:242)
        at com.sun.xml.ws.transport.http.HttpAdapter.encodePacket(HttpAdapter.java:636)
        at com.sun.xml.ws.transport.http.HttpAdapter.access$100(HttpAdapter.java:108)
        at com.sun.xml.ws.transport.http.HttpAdapter$HttpToolkit.handle(HttpAdapter.java:878)
        at com.sun.xml.ws.transport.http.HttpAdapter.handle(HttpAdapter.java:422)
        at com.sun.xml.ws.transport.http.servlet.ServletAdapter.handle(ServletAdapter.java:169)
        at org.glassfish.webservices.Ejb3MessageDispatcher.handlePost(Ejb3MessageDispatcher.java:110)
        at org.glassfish.webservices.Ejb3MessageDispatcher.invoke(Ejb3MessageDispatcher.java:80)
        at org.glassfish.webservices.EjbWebServiceServlet.dispatchToEjbEndpoint(EjbWebServiceServlet.java:239)
        at org.glassfish.webservices.EjbWebServiceServlet.service(EjbWebServiceServlet.java:162)
        at javax.servlet.http.HttpServlet.service(HttpServlet.java:790)
        at org.glassfish.grizzly.servlet.ServletHandler.doServletService(ServletHandler.java:228)
        at org.glassfish.grizzly.servlet.ServletHandler.service(ServletHandler.java:178)
        at com.sun.enterprise.v3.services.impl.ContainerMapper$HttpHandlerCallable.call(ContainerMapper.java:483)
        at com.sun.enterprise.v3.services.impl.ContainerMapper.service(ContainerMapper.java:180)
        at org.glassfish.grizzly.http.server.HttpHandler.runService(HttpHandler.java:206)
        at org.glassfish.grizzly.http.server.HttpHandler.doHandle(HttpHandler.java:180)
        at org.glassfish.grizzly.http.server.HttpServerFilter.handleRead(HttpServerFilter.java:235)
        at org.glassfish.grizzly.filterchain.ExecutorResolver$9.execute(ExecutorResolver.java:119)
        at org.glassfish.grizzly.filterchain.DefaultFilterChain.executeFilter(DefaultFilterChain.java:284)
        at org.glassfish.grizzly.filterchain.DefaultFilterChain.executeChainPart(DefaultFilterChain.java:201)
        at org.glassfish.grizzly.filterchain.DefaultFilterChain.execute(DefaultFilterChain.java:133)
        at org.glassfish.grizzly.filterchain.DefaultFilterChain.process(DefaultFilterChain.java:112)
        at org.glassfish.grizzly.ProcessorExecutor.execute(ProcessorExecutor.java:77)
        at org.glassfish.grizzly.nio.transport.TCPNIOTransport.fireIOEvent(TCPNIOTransport.java:539)
        at org.glassfish.grizzly.strategies.AbstractIOStrategy.fireIOEvent(AbstractIOStrategy.java:112)
        at org.glassfish.grizzly.strategies.WorkerThreadIOStrategy.run0(WorkerThreadIOStrategy.java:117)
        at org.glassfish.grizzly.strategies.WorkerThreadIOStrategy.access$100(WorkerThreadIOStrategy.java:56)
        at org.glassfish.grizzly.strategies.WorkerThreadIOStrategy$WorkerThreadRunnable.run(WorkerThreadIOStrategy.java:137)
        at org.glassfish.grizzly.threadpool.AbstractThreadPool$Worker.doWork(AbstractThreadPool.java:593)
        at org.glassfish.grizzly.threadpool.AbstractThreadPool$Worker.run(AbstractThreadPool.java:573)
        at java.lang.Thread.run(Thread.java:748)
Caused by: javax.xml.bind.MarshalException
 - with linked exception:
[com.sun.istack.SAXException2: A cycle is detected in the object graph. This will cause infinitely deep XML: Datafile:357 -> DatafileFormat:162 -> Datafile:357]
        at com.sun.xml.bind.v2.runtime.MarshallerImpl.write(MarshallerImpl.java:276)
        at com.sun.xml.bind.v2.runtime.BridgeImpl.marshal(BridgeImpl.java:104)
        at com.sun.xml.bind.api.Bridge.marshal(Bridge.java:145)
        at com.sun.xml.ws.db.glassfish.BridgeWrapper.marshal(BridgeWrapper.java:176)
        at com.sun.xml.ws.message.jaxb.JAXBMessage.writePayloadTo(JAXBMessage.java:415)
        ... 35 more
Caused by: com.sun.istack.SAXException2: A cycle is detected in the object graph. This will cause infinitely deep XML: Datafile:357 -> DatafileFormat:162 -> Datafile:357
        at com.sun.xml.bind.v2.runtime.XMLSerializer.reportError(XMLSerializer.java:249)
        at com.sun.xml.bind.v2.runtime.XMLSerializer.pushObject(XMLSerializer.java:537)
        at com.sun.xml.bind.v2.runtime.XMLSerializer.childAsXsiType(XMLSerializer.java:631)
        at com.sun.xml.bind.v2.runtime.property.ArrayElementNodeProperty.serializeItem(ArrayElementNodeProperty.java:69)
        at com.sun.xml.bind.v2.runtime.property.ArrayElementProperty.serializeListBody(ArrayElementProperty.java:172)
        at com.sun.xml.bind.v2.runtime.property.ArrayERProperty.serializeBody(ArrayERProperty.java:159)
        at com.sun.xml.bind.v2.runtime.ClassBeanInfoImpl.serializeBody(ClassBeanInfoImpl.java:360)
        at com.sun.xml.bind.v2.runtime.XMLSerializer.childAsXsiType(XMLSerializer.java:696)
        at com.sun.xml.bind.v2.runtime.property.SingleElementNodeProperty.serializeBody(SingleElementNodeProperty.java:158)
        at com.sun.xml.bind.v2.runtime.ClassBeanInfoImpl.serializeBody(ClassBeanInfoImpl.java:360)
        at com.sun.xml.bind.v2.runtime.XMLSerializer.childAsXsiType(XMLSerializer.java:696)
        at com.sun.xml.bind.v2.runtime.property.ArrayElementNodeProperty.serializeItem(ArrayElementNodeProperty.java:69)
        at com.sun.xml.bind.v2.runtime.property.ArrayElementProperty.serializeListBody(ArrayElementProperty.java:172)
        at com.sun.xml.bind.v2.runtime.property.ArrayERProperty.serializeBody(ArrayERProperty.java:159)
        at com.sun.xml.bind.v2.runtime.ClassBeanInfoImpl.serializeBody(ClassBeanInfoImpl.java:360)
        at com.sun.xml.bind.v2.runtime.XMLSerializer.childAsXsiType(XMLSerializer.java:696)
        at com.sun.xml.bind.v2.runtime.property.ArrayElementNodeProperty.serializeItem(ArrayElementNodeProperty.java:69)
        at com.sun.xml.bind.v2.runtime.property.ArrayElementProperty.serializeListBody(ArrayElementProperty.java:172)
        at com.sun.xml.bind.v2.runtime.property.ArrayERProperty.serializeBody(ArrayERProperty.java:159)
        at com.sun.xml.bind.v2.runtime.ClassBeanInfoImpl.serializeBody(ClassBeanInfoImpl.java:360)
        at com.sun.xml.bind.v2.runtime.XMLSerializer.childAsXsiType(XMLSerializer.java:696)
        at com.sun.xml.bind.v2.runtime.property.ArrayElementNodeProperty.serializeItem(ArrayElementNodeProperty.java:69)
        at com.sun.xml.bind.v2.runtime.property.ArrayElementProperty.serializeListBody(ArrayElementProperty.java:172)
        at com.sun.xml.bind.v2.runtime.property.ArrayERProperty.serializeBody(ArrayERProperty.java:159)
        at com.sun.xml.bind.v2.runtime.ClassBeanInfoImpl.serializeBody(ClassBeanInfoImpl.java:360)
        at com.sun.xml.bind.v2.runtime.XMLSerializer.childAsXsiType(XMLSerializer.java:696)
        at com.sun.xml.bind.v2.runtime.property.ArrayElementNodeProperty.serializeItem(ArrayElementNodeProperty.java:69)
        at com.sun.xml.bind.v2.runtime.property.ArrayElementProperty.serializeListBody(ArrayElementProperty.java:172)
        at com.sun.xml.bind.v2.runtime.property.ArrayERProperty.serializeBody(ArrayERProperty.java:159)
        at com.sun.xml.bind.v2.runtime.ClassBeanInfoImpl.serializeBody(ClassBeanInfoImpl.java:360)
        at com.sun.xml.bind.v2.runtime.XMLSerializer.childAsXsiType(XMLSerializer.java:696)
        at com.sun.xml.bind.v2.runtime.MarshallerImpl.write(MarshallerImpl.java:271)
        ... 39 more
]]

and an apparently mutilated SOAP response (note that S:Body is not closed):

<?xml version=\'1.0\' encoding=\'UTF-8\'?>
<S:Envelope xmlns:S="http://schemas.xmlsoap.org/soap/envelope/">
  <S:Body>
    <SOAP-ENV:Envelope xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/">
      <SOAP-ENV:Header/>
      <SOAP-ENV:Body>
    <SOAP-ENV:Fault>
      <faultcode xmlns:env="http://schemas.xmlsoap.org/soap/envelope/">env:Server</faultcode>
      <faultstring>invocation error on ejb endpoint ICAT at /ICATService/ICAT : javax.xml.bind.MarshalException\n - with linked exception:\n[com.sun.istack.SAXException2: A cycle is detected in the object graph. This will cause infinitely deep XML: Datafile:357 -&gt; DatafileFormat:162 -&gt; Datafile:357].</faultstring>
    </SOAP-ENV:Fault>
      </SOAP-ENV:Body>
    </SOAP-ENV:Envelope>

I honestly don't know if there is any use case for related objects in the select clause and whether we intend to support this. I just noticed it while experimenting with edge cases during client side implementation.

If we do not intend to support this, it would be good to at least intercept it and raise ICATParameterError.

Update edit: I should have looked into my own test suite before writing this, there are sensible use cases for related objects in the select clause: it makes sense to search for all datafile formats used in an investigation as in SELECT DISTINCT(df.datafileFormat) FROM Datafile df JOIN df.dataset AS ds JOIN ds.investigation AS i WHERE i.id = 7605. This works and is supported in the icat.server release version. So I update my statement to: I don't know if there is any use case for related objects in the select clause with multiple fields.

louise-davies commented 3 years ago

@RKrahl I've been investigating the issue and I thought I'd try the example query via the REST API as well to see the result and realised that SELECT ds.investigation, ds.name FROM Dataset ds fails with a 500 error when querying from the REST API as well (and looking at the logs due to the same issue - there's a bunch of logging to do with jsonizing and eventually a stackoverflow exception, which is presumably the equivalent of the SOAP "infinitely deep XML" error).

So this is an issue on both APIs and so perhaps should be a separate issue? As currently this issue is about aligning behaviour between the two APIs re: multiple fields.

This also highlights that if there was a use case for related objects in the select clause with multiple fields, currently ICAT can't support it via the more "complete" REST API either - so presumably it's not being used by anyone currently.

RKrahl commented 3 years ago

I'm fine with leaving it out here. The core feature that we wanted to address is icatproject/python-icat#76 needed by the DataGateway API. This works now.

Nevertheless, it's clearly a bug in icat.server, apparently also on the REST API. So we should open an issue so that we remember to fix it eventually.