Skip to content

[SoftDeleteable] Remove dollar sign from cache key #2978

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

[SoftDeleteable] Remove dollar sign from cache key #2978

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
2aa6ce4
Handle array parameters in QueryAnalyzer
adambalint-srg Jun 8, 2021
d1df20e
Merge branch 'doctrine-extensions:main' into main
adambalint-srg Aug 22, 2025
b9f4b8f
refactor: Remove dollar sign from cache key
adambalint-srg Aug 22, 2025
0683d13
Cache key dollar sign - Add changelog entry
adambalint-srg Sep 22, 2025
0ed5521
Merge remote-tracking branch 'upstream/main'
adambalint-srg Sep 22, 2025
f8bc8a8
Merge branch 'main' into cache-key-fix
adambalint-srg Sep 22, 2025
fdae25a
fix: Remove not related change
adambalint-srg Sep 22, 2025
78de563
fix: Remove not related change
adambalint-srg Sep 22, 2025
74d0e9f
Bump hadolint/hadolint-action from 3.2.0 to 3.3.0
dependabot[bot] Sep 22, 2025
5f63ac6
Add a deprecations baseline to ignore selected deprecations in tests
mbabker Jun 2, 2025
7f32378
3.21.0
phansys Sep 20, 2025
6226afd
Cache key dollar sign - Add changelog entry
adambalint-srg Sep 22, 2025
3fceb08
Merge branch 'main' into cache-key-fix
adambalint-srg Sep 23, 2025
1acc141
docs: Add psr6 url to changelog
adambalint-srg Sep 23, 2025
dd26d78
Update CHANGELOG.md
adambalint-srg Sep 23, 2025
9f12943
docs: Changelog update (All extensions are affected)
adambalint-srg Sep 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ a release.
---

## [Unreleased]
### Fixed
- SoftDeleteable: Remove dollar sign ($) from the default cache key of soft deleteable metadata - to be compatible with stricter cache key naming conventions (see https://www.php-fig.org/psr/psr-6/#definitions) (#2978)
Copy link
Collaborator

@phansys phansys Sep 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading the provided description and the referenced definition, I can't see this change as a fix.
The PSR-6 spec about cache keys states the following:

Key - A string of at least one character that uniquely identifies a cached item. Implementing libraries MUST support keys consisting of the characters A-Z, a-z, 0-9, _, and . in any order in UTF-8 encoding and a length of up to 64 characters. Implementing libraries MAY support additional characters and encodings or longer lengths, but must support at least that minimum.

This does not mean that characters other than A-Z, a-z, 0-9, _, and . are not allowed or discouraged. It confirms that at least these must be supported as minimum.

However, even if the current choice of characters in this library complies with the PSR-6 spec but causes a problem, the Fixed section MUST explicitly and clearly state what problem this change is fixing.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@phansys
Should I link the Laminas File cache's pattern here?
https://github.com/laminas/laminas-cache-storage-adapter-filesystem/blob/3.2.x/src/FilesystemOptions.php#L26
This was the reason of the whole PR.

In my opinion if PSR6 says about what is the most strict standard, and this library follows this, it can be a fix, but it's your decision of course.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's my take on the changelog entry. It's not a fix per se as it's not directly fixing a bug, so let's call it a change. Also note it doesn't affect only SoftDeleteable as this is changing the base EMF class all extensions use.

### Changed
- All: Removed the dollar sign from the generated cache ID for extension metadata to ensure only characters mandated by PSR-6 are used, improving compatibility with caching implementations with strict character requirements (#2978)

FWIW, doctrine/persistence originally used $CLASSMETADATA as its cache salt in the class metadata factory when the doctrine/cache library was used. It changed to the current __CLASSMETADATA__ value in doctrine/persistence#144 when support for PSR-6 was added and support for the Doctrine Cache library was deprecated. There isn't any explanation in that PR why the salt changed.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with your proposal @mbabker.

@adambalint-srg, could you please rebase the PR and update the changelog according this suggestion?


## [3.21.0] - 2025-09-22
### Added
Expand Down
2 changes: 1 addition & 1 deletion src/Mapping/ExtensionMetadataFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ public function getExtensionMetadata($meta)
*/
public static function getCacheId($className, $extensionNamespace)
{
return str_replace('\\', '_', $className).'_$'.strtoupper(str_replace('\\', '_', $extensionNamespace)).'_CLASSMETADATA';
return str_replace('\\', '_', $className).'__'.strtoupper(str_replace('\\', '_', $extensionNamespace)).'_CLASSMETADATA';
}

/**
Expand Down