Computed field should specify the cardinality of the field

Attached the patch setting up the cardinality. I hope it's okay, I'm not really expert with plugins.

Ideally, I suppose SingleValueTrait should override the default value for the annotation $cardinality, but I have no idea if that is possible and how do that, and I couldn't find an example on internet or other traits to do so.

I'd appreciate if someone could explain that (if it's actually possible) or fix the patch. Thanks

I didn't see that the field may be attached also inside hook_entity_bundle_field_info_alter for the "bundle" fields.
The patch add the cardinality check also in that hook.

Ah yes, I hadn't thought of that! Fields by default are single-valued, but the rendering system is lazy and just iterates anyway.

1. +++ b/computed_field.module
   @@ -121,6 +121,12 @@ function computed_field_entity_base_field_info_alter(&$fields, EntityTypeInterfa
   +    if ($plugin->fieldIsMultiple()) {
   

   same.

2. +++ b/computed_field.module
   @@ -177,6 +183,12 @@ function computed_field_entity_bundle_field_info_alter(&$fields, EntityTypeInter
   +    if ($plugin->fieldIsMultiple()) {
   

   We don't need this check - just pass on the cardinality, whatever it is.

3. +++ b/src/Annotation/ComputedField.php
   @@ -61,4 +62,12 @@ class ComputedField extends Plugin {
   +  public $cardinality = FieldStorageDefinitionInterface::CARDINALITY_UNLIMITED;
   

   I'm not sure we should default to unlimited, as BaseFieldDefinition defaults to 1. We should be consistent.

4. +++ b/src/Plugin/ComputedField/ComputedFieldBase.php
   @@ -43,6 +44,21 @@ abstract class ComputedFieldBase extends PluginBase implements ComputedFieldPlug
   +    return $this->pluginDefinition['cardinality'] ?: FieldStorageDefinitionInterface::CARDINALITY_UNLIMITED;
   

   You don't need the default here. It will come from the annotation property's default value if not specified in the plugin annotation.

5. +++ b/src/Plugin/ComputedField/ComputedFieldBase.php
   @@ -43,6 +44,21 @@ abstract class ComputedFieldBase extends PluginBase implements ComputedFieldPlug
   +    $cardinality = $this->getFieldCardinality();
   +    return ($cardinality === FieldStorageDefinitionInterface::CARDINALITY_UNLIMITED) || ($cardinality > 1);
   

   It's simpler to just say != 1 here I think?

6. +++ b/src/Plugin/ComputedField/ComputedFieldPluginInterface.php
   @@ -54,6 +54,21 @@ interface ComputedFieldPluginInterface extends PluginInspectionInterface, Deriva
   +   * Returns whether the field is multiple or not
   

   Missing full stop.

Changes regarding #4, not sure on point 3.
Thanks

3. I chose the unlimited instead of 1 because the module "default plugin logic" is a unlimited - while the "single" value needs to specify SingleValueTrait. But this is a mine arbitrary idea, it seems sensible to choose "1" as default.

5. I actually initially wrote something like that, but after I copied what BaseFieldDefinition::isMultiple() does:

  public function isMultiple() {
    $cardinality = $this->getCardinality();
    return ($cardinality == static::CARDINALITY_UNLIMITED) || ($cardinality > 1);
  }

But I've no idea if this is some overkill code or there's some obscure valid reason behind it.

Not sure if this is related, but I get errors importing nodes in feeds if they would return multiple values when calculated.

"Keywords (computed_keywords): Keywords: this field cannot hold more than 1 values."

This field doe hold more than one value just fine, and displays as multiple throughout the site, but, it breaks my feeds from importing...

@roxflame I think cardinality was supposed to go in your plugin's getFieldCardinality(), not in getFieldDefinitionSettings

Could this be related to 🐛 Allow computed function callbacks to determine cardinality dynamically Active ?

> 3. I chose the unlimited instead of 1 because the module "default plugin logic" is a unlimited value field.
> The "single" value needs to specify SingleValueTrait. But this is a mine arbitrary idea, it seems sensible to choose "1" as default.

The plugin logic was a mistake on my part -- I hadn't checked what base fields and config fields do.

We're still in alpha releases, so it's fine to change our API.

> Could this be related to #3437020: Allow computed function callbacks to determine cardinality dynamically?

Yes, they seem similar. Though I don't really understand the patch over there. Let's get this one in first.

Made a few changes, added a test.

Committed the MR.

Automatically closed - issue fixed for 2 weeks with no activity.

Hi, may suggest that the changelog of alpha9 explain that this MR introduced a breaking change, changing the default cardinality of the plugin from unlimited to single?

I don't know if there are others can be affected too, and if this can happen also with the rendering system, but when I switched from alpha8 + patch to alpha9 I got problems due this MR - namely, via JSON:API empty fields returned "false" instead of an empty array.

Considering I was puzzled a bit before finding the cause, and I'm the one opening the issue, others could be have even more difficulties to find the problem.