Skip to content

Conversation

markusmoosbrugger
Copy link
Contributor

@markusmoosbrugger markusmoosbrugger commented Jul 24, 2019

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets
Related issues/PRs sulu/SuluFormBundle#192
License MIT
Documentation PR

What's in this PR?

This PR adds the support for multiple form metadata loaders. The different FormMetadataLoader classes can be tagged in the associated services.xml files as sulu_admin.form_metadata_loader in order to get injected automatically in the FormMetadataProvider. The different FormMetadataLoader classes must implement the Interface FormMetadataLoaderInterface.

Furthermore this PR changes the loading and caching mechanism of form metadata. The FormXmlLoader returns now an instance of LocalizedFormMetadataCollection, which contains a FormMetadata object for each locale. Instead of the old metadata objects, the newer ones are used now. The old metadata classes are now marked as deprecated.

The XmlFormMetadataLoader implements the FormMetadataLoaderInterface and provides the functionality to load and cache the form metadata. This was done previously in the FormMetadataProvider.
The StructureFormMetadataLoader is added as well in this PR and also implements the FormMetadataLoaderInterface. The StructureLoader is responsible for caching the structure metadata.

The class FormMetadataMapper handles the conversation between old and new metadata objects and is used in the XmlFormMetadataLoader, StructureFormMetadataLoader and FormXmlLoader.

Why?

Enable to use a custom metadata loader provided from the form bundle (see PR sulu/SuluFormBundle#192) . The functionality can also be useful for other bundles that want to provide their own metadata loader.

The modification to use the new metadata classes in the FormXmlLoader is necessary to avoid the usage of the old metadata objects in new loaders.

Example Usage

Add a service that implements the FormMetadataLoaderInterface. Afterwards tag this service in the services.xml file as sulu_admin.form_metadata_loader.
Example:

 <service
            id="sulu_admin.xml_form_metadata_loader"
            class="Sulu\Bundle\AdminBundle\Metadata\FormMetadata\XmlFormMetadataLoader"
>
            <argument type="service" id="sulu_admin.form_metadata.form_xml_loader"/>
            <argument>%sulu_admin.forms.directories%</argument>
            <argument>%sulu.cache_dir%/forms</argument>
            <argument>%kernel.debug%</argument>
            <tag name="sulu_admin.form_metadata_loader"/>
            <tag name="kernel.cache_warmer" />
</service>

BC Breaks/Deprecations

Describe BC breaks/deprecations here. (remove this section if not needed)

To Do

  • Create a documentation PR
  • Add breaking changes to UPGRADE.md

@markusmoosbrugger markusmoosbrugger marked this pull request as ready for review July 24, 2019 13:17
Copy link
Member

@alexander-schranz alexander-schranz left a comment

Choose a reason for hiding this comment

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

@markusmoosbrugger the PR Example Usage is missleading it should be a example with sulu_admin.metadata_loader like: https://github.com/sulu/SuluFormBundle/pull/192/files#diff-1c64574ff1f633f0df3aac6fa7a25c7b

@markusmoosbrugger markusmoosbrugger force-pushed the enhancement/metadata-provider branch from 6d5ef17 to f9f6251 Compare July 30, 2019 06:49
'first' => [
'name' => 'first',
'const' => 1,
'allOf' => [
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure these changes are ok? Seems like it behaves differently as before...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The merging of schema information happened previously in the FormMetadataProvider and now it's already done in the FormXmlLoader and so the FormXmlLoader returns the merged schema information.
As the properties first and third are mandatory it seems correct to me that they are contained in the schema.

@danrot
Copy link
Contributor

danrot commented Aug 7, 2019

The XML file for the forms are loaded everytime in dev mode, instead of being taken from the cache.

Markus Moosbrugger added 10 commits August 8, 2019 14:35
…and form xml loader

tests are not adapted yet
StructureLoader and FormMetadataXmlLoader now responsible for caching;
FormXmlLoader returns collection of FormMetadata;
Marked old Metadata classes as deprecated
Removed local variable which is not needed anymore
@markusmoosbrugger markusmoosbrugger force-pushed the enhancement/metadata-provider branch from 0dbe383 to 04a514d Compare August 8, 2019 12:38
@danrot danrot merged commit 9183921 into sulu:develop Aug 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants