Skip to content

Conversation

@helios-ag
Copy link
Owner

@helios-ag helios-ag commented Nov 23, 2025

User description

Fixes deprecations for 7.4, closes #546


PR Type

Enhancement


Description

  • Migrate service configuration from XML to YAML format

  • Replace XmlFileLoader with YamlFileLoader in extension

  • Remove deprecated defaultValue from driver configuration

  • Convert three XML config files to equivalent YAML files


Diagram Walkthrough

flowchart LR
  XML["XML Config Files<br/>elfinder.xml<br/>form.xml<br/>command.xml"]
  YAML["YAML Config Files<br/>elfinder.yaml<br/>form.yaml<br/>command.yaml"]
  Loader["XmlFileLoader"]
  YamlLoader["YamlFileLoader"]
  XML -->|"Convert to"| YAML
  Loader -->|"Replace with"| YamlLoader
Loading

File Walkthrough

Relevant files
Configuration changes
FMElfinderExtension.php
Switch from XML to YAML file loader                                           

src/DependencyInjection/FMElfinderExtension.php

  • Changed loader from XmlFileLoader to YamlFileLoader
  • Updated all config file references from .xml to .yaml extensions
  • Maintains same service loading functionality with new format
+5/-5     
elfinder.yaml
New YAML configuration for elfinder services                         

config/elfinder.yaml

  • New YAML configuration file replacing elfinder.xml
  • Defines parameters for loader and configurator classes
  • Configures core services: configurator, loader, Twig extension, and
    ElFinder controller
  • Uses YAML syntax for service definitions and tags
+35/-0   
form.yaml
New YAML configuration for form services                                 

config/form.yaml

  • New YAML configuration file replacing form.xml
  • Defines parameter for form type class
  • Configures form type service with form.type tag
+8/-0     
command.yaml
New YAML configuration for command services                           

config/command.yaml

  • New YAML configuration file replacing command.xml
  • Configures ElFinder installer command service
  • Defines service dependencies and console.command tag
+8/-0     
elfinder.xml
Remove deprecated XML configuration file                                 

config/elfinder.xml

  • Deleted XML configuration file
  • Replaced by equivalent elfinder.yaml file
+0/-32   
form.xml
Remove deprecated XML configuration file                                 

config/form.xml

  • Deleted XML configuration file
  • Replaced by equivalent form.yaml file
+0/-15   
command.xml
Remove deprecated XML configuration file                                 

config/command.xml

  • Deleted XML configuration file
  • Replaced by equivalent command.yaml file
+0/-14   
Bug fix
Configuration.php
Remove deprecated driver default value                                     

src/DependencyInjection/Configuration.php

  • Removed deprecated defaultValue('LocalFileSystem') from driver scalar
    node
  • Driver configuration now requires explicit value without default
    fallback
+0/-1     

@qodo-merge-pro
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🟡
🎫 #546
🟢 Remove default value from required configuration node driver to address Symfony 7.4
deprecation "Setting a default value to a required node is deprecated" or make the node
optional.
Ensure bundle works without triggering the deprecation on Symfony 7.4 RC1 (no profiler
deprecations for this case).
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No auditing: The new service loading and configuration changes do not add or modify any logging of
critical actions, so it is unclear whether critical operations (e.g., controller actions
or command execution) are audited.

Referred Code
$loader = new YamlFileLoader($container, new FileLocator(__DIR__ . '/../../config'));
$loader->load('elfinder.yaml');
$loader->load('form.yaml');
$loader->load('command.yaml');
$container->setParameter('fm_elfinder', $config);

$repo = $container->getDefinition(ElFinderController::class);
$repo->replaceArgument(1, $config);

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Loader errors: The new YamlFileLoader calls lack explicit error handling for missing/invalid YAML files,
leaving uncertainty about how failures are reported or handled.

Referred Code
$loader = new YamlFileLoader($container, new FileLocator(__DIR__ . '/../../config'));
$loader->load('elfinder.yaml');
$loader->load('form.yaml');
$loader->load('command.yaml');

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Command errors: The new console command service registration adds an installer command without
demonstrating secure, user-facing error messages or separation of internal details on
failures.

Referred Code
services:
    fm_elfinder.command.installer:
        class: FM\ElfinderBundle\Command\ElFinderInstallerCommand
        arguments:
            - '@Symfony\Component\Filesystem\Filesystem'
            - '@Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface'
        tags:
            - { name: 'console.command', command: 'elfinder:install' }

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Config validation: The configuration now requires an explicit 'driver' value but the service
definitions do not show validation or sanitization of external configuration inputs passed
into the controller and loader.

Referred Code
parameters:
    fm_elfinder.loader: FM\ElfinderBundle\Loader\ElFinderLoader
    fm_elfinder.configurator: FM\ElfinderBundle\Configuration\ElFinderConfigurationReader

services:
    fm_elfinder.configurator.default:
        class: '%fm_elfinder.configurator%'
        arguments:
            - '%fm_elfinder%'
            - '@request_stack'
            - '@service_container'

    fm_elfinder.loader.default:
        class: '%fm_elfinder.loader%'
        autowire: true
        arguments:
            - '@fm_elfinder.configurator'

    FM\ElfinderBundle\Loader\ElFinderLoader:
        alias: fm_elfinder.loader.default



 ... (clipped 14 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@codecov
Copy link

codecov bot commented Nov 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 53.54%. Comparing base (27846f2) to head (0dd3678).

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #547      +/-   ##
============================================
- Coverage     53.74%   53.54%   -0.20%     
  Complexity      251      251              
============================================
  Files            17       17              
  Lines          1228     1212      -16     
============================================
- Hits            660      649      -11     
+ Misses          568      563       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@qodo-merge-pro
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Remove deprecated Twig extension alias

Remove the deprecated alias attribute from the twig.extension tag for the
twig.extension.fm_elfinder_init service.

config/elfinder.yaml [22-27]

 twig.extension.fm_elfinder_init:
     class: FM\ElfinderBundle\Twig\Extension\FMElfinderExtension
     arguments:
         - '@twig'
     tags:
-        - { name: 'twig.extension', alias: 'fm_elfinder_init' }
+        - { name: 'twig.extension' }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies a deprecated alias attribute in a twig.extension tag and proposes its removal, which is a good practice and improves the configuration file's quality.

Low
Avoid injecting the entire container

Avoid injecting the entire service container into the
fm_elfinder.configurator.default service and instead inject only the specific
services that are required.

config/elfinder.yaml [6-11]

 fm_elfinder.configurator.default:
     class: '%fm_elfinder.configurator%'
     arguments:
         - '%fm_elfinder%'
         - '@request_stack'
+        # TODO: Inject specific services instead of the whole container
         - '@service_container'
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies the use of an anti-pattern (injecting the service container), but the proposed change only adds a TODO comment instead of a concrete fix.

Low
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Since symfony/config 7.4: Setting a default value to a required node is deprecated

2 participants