apigee-edge-drupal
apigee-edge-drupal copied to clipboard
Use FieldAttributeConverterInterface instead of concrete FieldAttributeConverter class in constructor injection
Is your feature request related to a problem? Please describe.
Yes. Currently, the \Drupal\apigee_edge\Form\DeveloperAttributesSettingsForm class injects the concrete class \Drupal\apigee_edge\FieldAttributeConverter in its constructor.
Since FieldAttributeConverter is declared final, it cannot be extended. This causes a TypeError when another module tries to decorate or override the service apigee_edge.converter.field_attribute, because the decorated service is not an instance of the final class.
Example error:
TypeError: Argument #4 ($field_attribute_converter) must be of type
Drupal\apigee_edge\FieldAttributeConverter
This limits extensibility and makes it difficult for other modules to customize or wrap the service.
Describe the solution you would like
Update DeveloperAttributesSettingsForm constructor type hints to use the interface FieldAttributeConverterInterface instead of the concrete class FieldAttributeConverter.
public function __construct(ConfigFactoryInterface $config_factory, EntityFieldManagerInterface $entity_field_manager, FieldStorageFormatManagerInterface $field_storage_format_manager, FieldAttributeConverterInterface $field_attribute_converter, TypedConfigManagerInterface $typed_config_manager) {
parent::__construct($config_factory, $typed_config_manager);
$this->fieldAttributeConverter = $field_attribute_converter;
$this->entityFieldManager = $entity_field_manager;
$this->fieldStorageFormatManager = $field_storage_format_manager;
}
This change will:
- Allow other modules to decorate or replace the service without type conflicts.
- Follow the dependency inversion principle.
- Improve extensibility and maintainability.
Describe alternatives you have considered
- Describe alternatives you have considered
- Using a service decorator — fails because FieldAttributeConverter is final
- Removing the type hint in the constructor — works but reduces type safety and is not ideal
- Replacing the service entirely — possible but risky, as it may break other parts of the module relying on the original implementation.
- Using the interface is the cleanest, most future-proof solution.
Additional context
Example use case: A custom module wants to extend FieldAttributeConverter (e.g., to add custom logic or logging). However, because the class is final and type-hinted directly, the decorated service cannot be injected into the form.
Updating the constructor to depend on FieldAttributeConverterInterface would fix this and allow safe decoration.
@berramou , Thank you for bringing this issue to our attention and sharing a corresponding fix. We will review your Pull Request and follow up shortly.
@berramou
Thank you for this contribution. I have reviewed PR #1199 and tested it locally. The code itself looks good.
However, I identified some phpcs-related issues that need to be addressed. Could you please fix these coding standards violations?
If you run into any difficulty, you can refer to this patch where I have resolved the issues.
Hello @shailesh-google Fixed here https://github.com/apigee/apigee-edge-drupal/pull/1199/commits/a21d7b8795ccd9fa02818ca2d6cb91f92cd9da28