Skip to content
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.

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

Fix updating path identification string #334

Merged
merged 1 commit into from
Apr 30, 2013

Conversation

pspanja
Copy link
Contributor

@pspanja pspanja commented Apr 30, 2013

This PR fixes a regression introduced with SPI Cache, where $updatePathIdentificationstring flag was not passed in Cache/UrlAliasHandler.

@andrerom
Copy link
Contributor

Can't find this parameter in the SPI, this is the root issue then, a handler + Core relying on a parameter not in the interface.

@pspanja
Copy link
Contributor Author

pspanja commented Apr 30, 2013

It was not added to the interface as path identification string is deprecated Legacy Storage feature, so no other storage needs it. Done in this way as isolating it to the storage would add quite some complexity.

Ref: #207

@andrerom
Copy link
Contributor

ok, but this is then a issue. Could you at least report it so we can clean this up afterwards?

We should basically never have to know about internal parameters from outside the layer, so both in case of API SignalSlot and SPI Cache, they should only have to implement the interface.

+1 for 5.1, but with 5.2 followup issue, @patrickallaert / @bdunogier ?

@bdunogier
Copy link
Member

Agree with you @andrerom, +1 with follow-up.

andrerom added a commit that referenced this pull request Apr 30, 2013
…ache-handler

Fix updating path identification string
@andrerom andrerom merged commit e1a00ab into master Apr 30, 2013
@andrerom andrerom deleted the fix-pathidentificationstring-cache-handler branch April 30, 2013 15:32
@pspanja
Copy link
Contributor Author

pspanja commented Apr 30, 2013

Follow up: https://jira.ez.no/browse/EZP-20795

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

Successfully merging this pull request may close these issues.

3 participants