search

orbeon / orbeon-forms

Orbeon Forms is an open source web forms solution. It includes an XForms engine, the Form Builder web-based form editor, and the Form Runner runtime.
http://www.orbeon.com/
GNU Lesser General Public License v2.1
514 stars 220 forks source link

Static image in section template doesn't use the correct form definition version #4919

Closed ebruchez closed 3 years ago

ebruchez commented 3 years ago

The reason is that fr|image-attachment has this:

<xf:header ref="fr:form-version()">
    <xf:name>Orbeon-Form-Definition-Version</xf:name>
    <xf:value value="."/>
</xf:header>

This means that the section template will pass the version of the enclosing form to retrieve the attachment, not the version of the section template.

We need to change this so that the version of the section template is used.

ebruchez commented 3 years ago

Not a regression: there was never a mechanism to get the correct version.

ebruchez commented 3 years ago

Two approaches:

  1. Add library version to section template namespace.
  2. upon publish, copy all attachments from all section templates to published form definition location.

Option 2 makes more sense and appears more correct. With option 1, if a change was made to the published library form, the attachment would change, and this is not consistent with how we handle section templates generally.

Also option 1 would require a way to query the section template version and modify the attachment XBL components. With option 2, no change to the XBL components are needed.

ebruchez commented 3 years ago

So upon publish in putWithAttachments() we need to recognize the URLs to read and write.

The current code is a little funny because it is not particularly designed for form definitions and iterates the entire form definition's elements and attributes to find the attachments. So it should find all of them if we provide the appropriate fromBasePaths. For those on the top-level form definition side, there is no version (Form Builder data). For those on the library side, though, we need pairs of (fromBasePath, version) as the data will be retrieved on the published forms side.

ebruchez commented 3 years ago

Copy of comment from duplicate #3170 from @avernet:

To reproduce, with Form Runner setup to use a RDB (say MySQL):

  1. In Form Builder, create a form a/library, add a static image, publish it. Create a form a/a, add the section template, publish it, load a/a/new ⇒ all good.
  2. Update a/library, change the image, publish it. Back to a/a, click on the green reload icon to update the section template ⇒ the image shows broken (404).

The 404 is because we try to retrieve the image with version 1, while it is stored as version 2 in the database.

Since we embed the section template into the form on publish, we should also copy attachments part of the section template where we store the published form (crud/a/a/form/123.bin), as the published form shouldn't have a dependency on the published section template (crud/a/library/form/123.bin).

I would suggest that, when in Form Builder the template is added or updated (not when publishing), we copy attachments part of the template along the form data. This way they will be automatically copied in the right place, and with the right version, when the form is published.

+1 from community

ebruchez commented 3 years ago

+1 from customer

ebruchez commented 3 years ago

Logic:

ebruchez commented 3 years ago

Now this solution works, except when the section template is in Form Builder:

ebruchez commented 3 years ago

As discussed initially, the Form Builder issue is due to the fact that we use fr:form-version(). By copying attachments upon publish we remove this problem for published form definitions, but not in Form Builder.

Indeed, if I hardcode version 2 in image.xbl the image loads.

So:

We could change fr:form-version() so that at design-time it returns something else. However this wouldn't solve the "test" mode unless that is tested too.

ebruchez commented 3 years ago

Changing the namespace URI of the section components in the form seems like heavy and error-prone.

However the top-level fr-form-metadata contains the library versions we need:

<xf:instance id="fr-form-metadata" ...>
    <metadata>
        ...
        <library-versions>
            <orbeon>3</orbeon>
            <app>2</app>
        </library-versions>
    </metadata>
</xf:instance>

This is used by Form Builder only.

So we need to:

Maybe we could have a new function, for example:

fr:attachment-form-version()
ebruchez commented 3 years ago
ebruchez commented 3 years ago

FormRunnerCompiler retrieves attachments without version information, therefore it fails to retrieve them in some cases. So we need to figure out version information.

FormRunnerCompiler works in two ways:

ebruchez commented 3 years ago

We can actually know based on the path:

/fr/service/persistence/crud

ebruchez commented 3 years ago

There is more. When we publish a form definition from Form Builder, and we read attachments from a versioned library, we do not pass any versioning header at all during the GET.

Say I am publishing the form as v3. The form includes a section template from acme/library v2. We need to read the attachment with v2, and then write it to the published form definition with v3.

However, we do this with fr-create-update-attachment-submission right now. This uses the application/octed-stream serialization, which reads the URL (here the section template attachment) but doesn't allow passing headers.

With FormRunnerCompiler we read the attachments directly so we don't have this problem.

ebruchez commented 3 years ago

There is even more. The publish API loads all the components including section templates, like Form Builder does, except that it doesn't pass the orbeon-library-version and app-library-version at all. So it always includes the latest version of the section templates when publishing.

ebruchez commented 3 years ago

All fixes on the offline branches for now.