Skip to content

Removed laminas/laminas-stdlib dependency and added native types to Storage module - #149

Merged
gsteel merged 7 commits into
laminas:3.0.xfrom
Jurj-Bogdan:update-storage-module
Jul 31, 2025
Merged

Removed laminas/laminas-stdlib dependency and added native types to Storage module#149
gsteel merged 7 commits into
laminas:3.0.xfrom
Jurj-Bogdan:update-storage-module

Conversation

@Jurj-Bogdan

Copy link
Copy Markdown
Contributor
Q A
BC Break yes

Description

Initial commit to add native method and return types to the src/Storage module, based on issue #113.

This PR might also be the place to further refactor the module as needed but I'm unsure how what the end goal of that refactoring might be.

Signed-off-by: Jurj-Bogdan <jurj@dotkernel.com>
@Jurj-Bogdan

Copy link
Copy Markdown
Contributor Author

Speaking of refactoring, I'd guess that SessionStorage and SessionArrayStorage could be marked as final as a start

@arhimede
arhimede requested review from gsteel and samsonasik June 19, 2025 09:48
Comment thread src/Storage/AbstractSessionArrayStorage.php Outdated

@gsteel gsteel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not convinced that it's possible for session keys to be integers, so the key type could be refined to string across the board. You can also type the interface to @extends ArrayAccess<string, mixed> etc.

It should be possible to eliminate usage of Laminas\Stdlib.

It's unfortunate that this implements ArrayAccess so we're stuck with offsetSet(mixed $key, mixed $value): void etc - ArrayAccess methods could have assertions for string keys perhaps because the doc blocks will not be inferred via SA tools AFAIK

Comment thread src/Storage/AbstractSessionArrayStorage.php Outdated
Comment thread src/Storage/AbstractSessionArrayStorage.php Outdated
Comment thread src/Storage/AbstractSessionArrayStorage.php Outdated
Comment thread src/Storage/AbstractSessionArrayStorage.php Outdated
Comment thread src/Storage/AbstractSessionArrayStorage.php Outdated
Comment thread src/Storage/AbstractSessionArrayStorage.php Outdated
Comment thread src/Storage/ArrayStorage.php Outdated
Comment thread src/Storage/Factory.php Outdated
Comment thread src/Storage/SessionArrayStorage.php Outdated
*
* @param array|null $input
* @param int $flags
* @param string $iteratorClass

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@param class-string<Iterator> $iteratorClass ??

…ode updates

Signed-off-by: Jurj-Bogdan <jurj@dotkernel.com>
@Jurj-Bogdan Jurj-Bogdan changed the title added native types to Storage module Removed laminas/laminas-stdlib dependency and added native types to Storage module Jul 10, 2025
Signed-off-by: Jurj-Bogdan <jurj@dotkernel.com>
@Jurj-Bogdan

Copy link
Copy Markdown
Contributor Author

Since the storage module was the main place laminas/laminas-stdlib was used (only 1 ArrayUtils use case in SessionManager outside of it), i've removed the dependency completely as per @gsteel's suggestion, replacing Stdlib\ArrayObjects with the native version.

@gsteel gsteel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @Jurj-Bogdan - Pretty sure that most additions of #[AllowDynamicProps] can be removed if all implementations define __set etc.

Comment thread src/AbstractContainer.php Outdated
Comment thread src/AbstractContainer.php Outdated
Comment thread src/AbstractContainer.php Outdated
Comment thread src/AbstractContainer.php
Comment thread src/AbstractContainer.php Outdated
Comment thread src/Storage/AbstractSessionArrayStorage.php
Comment thread src/Storage/ArrayStorage.php Outdated
Comment thread src/Storage/StorageInitializationInterface.php
Comment thread src/AbstractContainer.php Outdated
* This test case fails on laminas-session 2.8.0 with the php error below and works fine on 2.7.*.
* "Only variable references should be returned by reference"
*/
#[IgnoreDeprecations]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I assume this is because of __get and offsetGet implementations returning by ref.

Perhaps, in a different patch, return by ref should be removed across the board and all these IgnoreDeprecations attributes removed.

@gsteel gsteel added this to the 3.0.0 milestone Jul 14, 2025
Signed-off-by: Jurj-Bogdan <jurj@dotkernel.com>
Comment thread src/Storage/AbstractSessionArrayStorage.php Outdated
Signed-off-by: Jurj-Bogdan <jurj@dotkernel.com>
Signed-off-by: Jurj-Bogdan <jurj@dotkernel.com>
Comment thread src/AbstractContainer.php Outdated
Signed-off-by: Jurj-Bogdan <jurj@dotkernel.com>
@gsteel gsteel self-assigned this Jul 31, 2025

@gsteel gsteel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for sticking with me @Jurj-Bogdan - We got there in the end 😅

@gsteel
gsteel merged commit 257ce75 into laminas:3.0.x Jul 31, 2025
13 of 14 checks passed
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.

3 participants