Skip to content

fixes #700: Psr16Adapter::deleteMultiple converts $keys to an array.#703

Merged
Geolim4 merged 2 commits intoTruCopilot:masterfrom
bylexus:fix_issue_700_delete_multiple
Sep 4, 2019
Merged

fixes #700: Psr16Adapter::deleteMultiple converts $keys to an array.#703
Geolim4 merged 2 commits intoTruCopilot:masterfrom
bylexus:fix_issue_700_delete_multiple

Conversation

@bylexus
Copy link
Copy Markdown
Contributor

@bylexus bylexus commented Sep 3, 2019

The internal Psr\Cache\CacheItemPoolInterface::deleteItems requires an array as argument. Some popular tools like PhpSpreadsheet use a Traversable instead.

Proposed changes

deleteMultiple() should convert the $keys argument to an array, as the underlying CacheItemPoolInterface::deleteItems() method takes arrays only.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)

Agreement

I have read the CONTRIBUTING and CODING GUIDELINE docs

Further comments

This issue also appears in the v6 branch: I will commit another pull request to fix it in the 6.x version, too.

…an array.

The internal Psr\Cache\CacheItemPoolInterface::deleteItems requires an array as argument. Some popular tools like PhpSpreadsheet use a Traversable instead.
@Geolim4
Copy link
Copy Markdown
Member

Geolim4 commented Sep 3, 2019

Hello,
I began the development of this fix couple of day ago but you were faster.

I'm gonna review & merge this PR by the end of day, thanks you.

Cheers,
Georges

@Geolim4
Copy link
Copy Markdown
Member

Geolim4 commented Sep 3, 2019

Can you please check the PSr16 specs please, in my memories setMultiple is also concerned by this requirement, thanks you !

@bylexus
Copy link
Copy Markdown
Contributor Author

bylexus commented Sep 3, 2019

Hi,

setMultiple() in Psr16Adapter is using a foreach() loop to process each item in keysseparately: Therefore no array, just an iterable object is needed.

Maybe getMultiple() is a problem, too, as it uses array_map. I will check this.

…sable $keys in an array

The internal array_map function can only be used with native arrays.
@bylexus
Copy link
Copy Markdown
Contributor Author

bylexus commented Sep 3, 2019

Hi,

I just fixed the two branches (master, v6): I also fixed getMultiple(), which requires an array, too (using array_map).

@Geolim4 Geolim4 merged commit 2cb775c into TruCopilot:master Sep 4, 2019
@Geolim4
Copy link
Copy Markdown
Member

Geolim4 commented Sep 4, 2019

Thanks, I'll push a new release tomorrow.

@bylexus
Copy link
Copy Markdown
Contributor Author

bylexus commented Sep 4, 2019

Many Thanks @Geolim4, I appreciate your fast reaction, as the phpfastcache lib is a central building block of our application!

@Geolim4
Copy link
Copy Markdown
Member

Geolim4 commented Sep 4, 2019

You're welcome, thanks you too for your contribution !

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.

2 participants