Problem/Motivation
The preSave() method in PrimaryEntityReferenceFieldItemList uses the spaceship operator (<=>) to sort items by their 'primary' property. While Drupal's typed data system should enforce the correct boolean type, adding explicit type casting provides defense in depth and makes the code's intent clearer.
Location: src/Plugin/Field/PrimaryEntityReferenceFieldItemList.php, lines 32-34
Current Code
public function preSave() {
usort($this->list, function ($a, $b) {
return $b->get('primary')->getValue() <=> $a->get('primary')->getValue();
});
parent::preSave();
}
Potential Issues
- If the 'primary' property isn't properly validated, type juggling could cause unexpected sort behavior
- The wrong item could be marked as primary if values are strings like "1" vs integers
- PHP's loose typing could cause
"0"(string) and0(integer) to compare differently than expected
Likelihood
Low - Drupal's typed data system enforces types and the 'primary' property is defined as a boolean in the field schema. However, explicit type casting improves code robustness and clarity.
Proposed resolution
Add explicit type casting to ensure consistent integer comparison regardless of the actual data type stored.
Recommended Implementation
public function preSave() {
usort($this->list, function ($a, $b) {
// Explicit casting ensures consistent comparison
return (int) $b->get('primary')->getValue() <=> (int) $a->get('primary')->getValue();
});
parent::preSave();
}
Benefits
- Defense in depth: Protects against unexpected type issues
- Code clarity: Makes the intent explicit that we're comparing integers
- Consistency: Ensures predictable behavior regardless of data source
- Performance: No performance penalty as type casting is negligible
Remaining tasks
- Add explicit
(int)type casting to both sides of the spaceship operator - Verify existing unit tests still pass
- Add unit test cases for edge cases (null values, string values, etc.)
- Review other comparison operations in the module for similar issues
- Update code comments to document the type expectation
User interface changes
None
API changes
None. This is an internal implementation detail that doesn't affect the public API.
Behavioral Changes
The behavior should remain unchanged for correctly typed data. If any edge cases existed where incorrect types were being compared, those will now be handled consistently.
Data model changes
None
Issue fork primary_entity_reference-3570441
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #4
bluegeek9 commented