4PointSolutions / FluentFormsAPI

Fluent API for Adobe AEM Forms
Apache License 2.0
6 stars 8 forks source link

Exporting Data from an empty form produces an error with an unhelpful error message #11

Closed rmcdouga closed 4 years ago

rmcdouga commented 4 years ago

I am executing a unit test using the other PDF available the sample forms directory (SampleForm.pdf) using the following code:

                @Test
        @DisplayName("Test exportData() NoData PDF.")
        void testExportDataNoData() throws Exception {

            Document pdforxdp = SimpleDocumentFactoryImpl.INSTANCE.create(SAMPLE_FORM_PDF.toFile());
            DataFormat dataformat = com.adobe.fd.forms.api.DataFormat.XmlData;
            Document pdfResult = underTest.exportData(pdforxdp, dataformat);

            org.w3c.dom.Document document1 = DocumentBuilderFactory.newInstance().newDocumentBuilder()
                                                                   .parse(new ByteArrayInputStream(pdfResult.getInlineData()));
            XPathExpression xpath = XPathFactory.newInstance().newXPath().compile("/form1");
            Node output1 = (Node) xpath.evaluate(document1, XPathConstants.NODE);

            assertEquals("form1", output1.getNodeName());
            assertEquals("abcd", output1.getTextContent().trim());

            if (SAVE_RESULTS) {
                TransformerFactory transformerFactory = TransformerFactory.newInstance();
                Transformer transformer = transformerFactory.newTransformer();
                DOMSource source = new DOMSource((Node) document1);

                StreamResult result = new StreamResult(new File(path + "/result.xml"));
                transformer.transform(source, result);
            }

            assertEquals(APPLICATION_XML, pdfResult.getContentType());

        }

When I invoke the code, I get the following error message and stack trace.

com._4point.aem.fluentforms.api.forms.FormsService$FormsServiceException: Call to server succeeded but server failed to return document.  This should never happen.
    at com._4point.aem.docservices.rest_services.client.forms.RestServicesFormsServiceAdapter.exportData(RestServicesFormsServiceAdapter.java:88)
    at com._4point.aem.fluentforms.impl.forms.SafeFormsServiceAdapterWrapper.exportData(SafeFormsServiceAdapterWrapper.java:25)
    at com._4point.aem.fluentforms.impl.forms.FormsServiceImpl.exportData(FormsServiceImpl.java:38)
    at com._4point.aem.docservices.rest_services.it_tests.client.forms.ExportDataTest.testExportDataNoData(ExportDataTest.java:124)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
    at java.lang.reflect.Method.invoke(Unknown Source)
    at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:686)
    at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
    at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:149)
    at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:140)
    at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:84)
    at org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
    at org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
    at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104)
    at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$6(TestMethodTestDescriptor.java:205)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:201)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:137)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:71)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:135)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
    at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
    at java.util.ArrayList.forEach(Unknown Source)
    at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
    at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
    at java.util.ArrayList.forEach(Unknown Source)
    at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
    at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
    at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:32)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:51)
    at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:248)
    at org.junit.platform.launcher.core.DefaultLauncher.lambda$execute$5(DefaultLauncher.java:211)
    at org.junit.platform.launcher.core.DefaultLauncher.withInterceptedStreams(DefaultLauncher.java:226)
    at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:199)
    at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:141)
    at org.eclipse.jdt.internal.junit5.runner.JUnit5TestReference.run(JUnit5TestReference.java:98)
    at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:41)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:542)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:770)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:464)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:210)

The message Call to server succeeded but server failed to return document. This should never happen. does not give me any indication of what the problem is or how to correct it. There are no error messages in the AEM log, so that is not helpful either.

This would be problematic if a library user encounters this issue.

rmcdouga commented 4 years ago

The reason this issue is happening is that the AEM API is returning an empty document to the fluentforms library and fluentforms is returning the empty document to the rest-services.server code. The rest-services.server code writes nothing into the response to the rest-services.client code, so no entity is created. The rest-services.client code detects that no entity was returned, which is not what was expected, so it throws an exception.

There's a couple of problems here: 1) By convention, a REST service that is not returning any output should return an HTTP status of 204 'No Content' to indicate the lack of content was intentional. The rest-services.server code should follow this convention. 2) The rest-services.client code should handle this and return something to the user indicating that there was nothing to export. One approach to this would be to return an empty Document (this is what the Adobe API does) however I don't believe this is the best approach. Using the empty Document approach means that the user of the API has to know that an empty Document is possible and check for that. This is easy to miss if they haven't read the documentation (assuming this behaviour is documented). A better approach would be to have the exportData API return an Optional<Document> instead of a Document. This makes it clear as part of the API that they are not guaranteed to get a result back.

I would propose that we make a change to the current fluentforms exportData API to return an Optional<Document>. This involves the following changes:

  1. changing fluentforms.core to have the exportData API return Optional<Document>
  2. changing rest-services.server return a 204 status of the Optional is empty.
  3. changing rest-services.client to translate a 204 response status into an Optional.empty() return value.

If anyone has additional thoughts or concerns, please leave them in the comments.

RainaRitikaPFG commented 4 years ago

Hi Rob,

The suggestion looks good to me. Currently, we are using export data API to export data from rendered pdf and saving the result in the document variable. From the xml document, we are extracting htmloutput through xpath. According to you, in the plugin we need to convert optional into document. I didn't get it clearly. My understanding is if we are not getting any document from API, it should just handle the error message , may be by putting the code into try catch or adding some logger. Let me know, if I am not getting it correctly.

Thanks, Ritika.

rmcdouga commented 4 years ago

According to you, in the plugin we need to convert optional into document. I didn't get it clearly. My understanding is if we are not getting any document from API, it should just handle the error message , may be by putting the code into try catch or adding some logger.

There are actually three cases: 1) PDF Form with Data in it - This returns a Document object with XML in it. 2) PDF without Form (or other server errors) - This throws an Exception from exportData() 3) PDF Form without Data in it - This returns an empty Document (note: this is not valid XML).

It's the last case that I am concerned about. I would rather we return an empty Optional than an empty Document as this follows the "principal of least surprise". Developers will see the API and realize that they have to allow for the case where no document is returned. Without that, they may not check for data in the Document before trying to do something with it.

rmcdouga commented 4 years ago

Associated PR was merged (I forgot to link the PR to this issue so it wasn't closed automatically). I'm closing this issue now.