Uploaded image for project: 'eZ Publish / Platform'
  1. eZ Publish / Platform
  2. EZP-26916

Invalid fieldSettings for existing ContentTypes in the default database

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Critical Critical
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      How to reproduce:
      1. export the definition of, f.e., the Folder ContentType from eZP 2014.11 (using the ez5 content API, not REST)
      2. try to create a new ContentType, with any random name, using the Field definitions taken from the Folder fields
      3. you will get an error: the fieldSettings do not validate!

      In particular:

      • for xmltext fields, the settings contain a tagPreset = '' value, whereas only 0 and 1 are valid values
      • for ezpage fields, the settings contain a defaultLayout = '' value, which is not valid

      (but there might be more, of course)

      What I expect: to be able to use the fieldSettings of existing ContentType fields to be able to create new ContentTypes.

        Issue Links

          Activity

          Hide
          Gaetano Giunta added a comment - - edited

          ps: checking what happens when the REST api is used:

          tagPreset = '' is converted into tagPreset = 'TAG_PRESET_DEFAULT' by the class eZ\Publish\Core\REST\Common\FieldTypeProcessor\XmlTextProcessor.

          The fact that that class accepts an empty string value and treats it as 0 is good, but tbh it seems more like a side effect of not doing strict-type comparisons rather than a conscious decision.

          Also, I do not think that it is the duty of the REST layer to paper over invalid values in the settings gotten from the API layer. It is the duty of the API layer itself to produce valid values.

          Show
          Gaetano Giunta added a comment - - edited ps: checking what happens when the REST api is used: tagPreset = '' is converted into tagPreset = 'TAG_PRESET_DEFAULT' by the class eZ\Publish\Core\REST\Common\FieldTypeProcessor\XmlTextProcessor. The fact that that class accepts an empty string value and treats it as 0 is good, but tbh it seems more like a side effect of not doing strict-type comparisons rather than a conscious decision. Also, I do not think that it is the duty of the REST layer to paper over invalid values in the settings gotten from the API layer. It is the duty of the API layer itself to produce valid values.
          Hide
          Gaetano Giunta added a comment -

          On the same vein: while using the Legacy Admin, I can change the selectionMethod setting of a RelationList field.
          I can then use export successfully the definition of the ContentType which contains that field, but can not import it back unless the selectionMethod is set to 2 values out of the 6 supported by legacy...

          Show
          Gaetano Giunta added a comment - On the same vein: while using the Legacy Admin, I can change the selectionMethod setting of a RelationList field. I can then use export successfully the definition of the ContentType which contains that field, but can not import it back unless the selectionMethod is set to 2 values out of the 6 supported by legacy...
          Hide
          Gaetano Giunta added a comment - - edited

          One more on the same vein (not related to settings though): while using the Legacy Admin, I can set a Field of type ezuser to be searchable.
          Again, the corresponding field definition will be exported successfully, but the system will refuse to re-import it.

          I guess the main question here is becoming: why allow the Legacy Admin interface to set up content classes in ways which the eZ5 kernel deems invalid? Should we not just prevent this (as long as the ez5 kernel is onboard and we are not in pure-legacy mode)?
          Or should we do the opposite, and "loosen" the ez5 kernel so that it accepts as valid all class definitions which can be set up by the legacy kernel?

          Show
          Gaetano Giunta added a comment - - edited One more on the same vein (not related to settings though): while using the Legacy Admin, I can set a Field of type ezuser to be searchable. Again, the corresponding field definition will be exported successfully, but the system will refuse to re-import it. I guess the main question here is becoming: why allow the Legacy Admin interface to set up content classes in ways which the eZ5 kernel deems invalid? Should we not just prevent this (as long as the ez5 kernel is onboard and we are not in pure-legacy mode)? Or should we do the opposite, and "loosen" the ez5 kernel so that it accepts as valid all class definitions which can be set up by the legacy kernel?
          Hide
          Gaetano Giunta added a comment -

          One more quirk: create a field of type Object Relation (single), set it to default selection root: none.

          When you get the FieldDefinition settings, you will then get a 'selectionRoot' setting with a value of '' (empty string).
          But if you try to create a new ContentType with a relation Field and pass in a 'selectionRoot' setting with a value of '', you get a db error (trying to save a string into a numeric field).

          I think that the error in this case is to define the selectionRoot setting as string in eZ\Publish\Core\FieldType\Relation\Type.php: it should only be allowed to be int or null. Then, the SPI converter eZ\Publish\Core\Persistence\Legacy\Content\FieldValue\Converter\Relation::toFieldDefinition() method should not convert 0 values from DB to '', but to null instead

          Show
          Gaetano Giunta added a comment - One more quirk: create a field of type Object Relation (single), set it to default selection root: none. When you get the FieldDefinition settings, you will then get a 'selectionRoot' setting with a value of '' (empty string). But if you try to create a new ContentType with a relation Field and pass in a 'selectionRoot' setting with a value of '', you get a db error (trying to save a string into a numeric field). I think that the error in this case is to define the selectionRoot setting as string in eZ\Publish\Core\FieldType\Relation\Type.php: it should only be allowed to be int or null. Then, the SPI converter eZ\Publish\Core\Persistence\Legacy\Content\FieldValue\Converter\Relation::toFieldDefinition() method should not convert 0 values from DB to '', but to null instead
          Hide
          Gaetano Giunta added a comment -

          More details about ezpage and an empty defaultLayout setting:

          • this seems to be a valid case in Legacy Admin UI: you can create a class with a layout attribute with no default layout, create content from it, etc...
          • the value for the defaultLayout field setting, if left unspecified by the developer in the fieldCreateStruct, is: empty string
          • otoh the empty string does not get validated as acceptable defaultLayout field setting by the very same Page FieldType

          This is highly inconsistent, and the fix seems easy: alter method eZ\Publish\Core\FieldType\Page\Type::validateFieldSettings() to accept an empty string as valid to mean 'no default layout chosen' (this might even have been patched already in ezplatform...)

          Show
          Gaetano Giunta added a comment - More details about ezpage and an empty defaultLayout setting: this seems to be a valid case in Legacy Admin UI: you can create a class with a layout attribute with no default layout, create content from it, etc... the value for the defaultLayout field setting, if left unspecified by the developer in the fieldCreateStruct, is: empty string otoh the empty string does not get validated as acceptable defaultLayout field setting by the very same Page FieldType This is highly inconsistent, and the fix seems easy: alter method eZ\Publish\Core\FieldType\Page\Type::validateFieldSettings() to accept an empty string as valid to mean 'no default layout chosen' (this might even have been patched already in ezplatform...)

            People

            • Assignee:
              Unassigned
              Reporter:
              Gaetano Giunta
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated: