Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Cache-Storage: make sure a '.post' event will be triggered #6259

Closed
wants to merge 4 commits into from
Closed

Cache-Storage: make sure a '.post' event will be triggered #6259

wants to merge 4 commits into from

Conversation

marc-mabe
Copy link
Member

... after the '.pre'-event even if the '.pre'-event was stopped

PS: '.exception'-events are specialized '.post'-events

…'.pre'-event even if the '.pre'-event was stopped (PS: '.exception'-events are specialized 'post'-events)
@Ocramius
Copy link
Member

@marc-mabe lots of changes in conditionals - can you cover these with tests?

@marc-mabe
Copy link
Member Author

@Ocramius You are right, tests are missing currently but I don't have time as I'm parental leave for 3 month starting this weekend :)

So sorry for the tests :( - If someone runs into this issue please feel free to use this work, add missing tests and create a new PR.

Changes explained:

Event flow on read/write items without a stopped event
on example of a serializer and crypting plugin:

  write "abc"
    trigger "setItem.pre"
      serialize -> 's:3:"abc";'
      crypt -> "??????"
    write "??????" to storage
    trigger "setItem.post"
      <no listener>

  read
    trigger "getItem.pre"
      <no listener>
    load "??????" from storage
    trigger "getItem.post"
      uncrypt -> 's:3:"abc;'
      unserialize -> "abc"
    return "abc"

Current event flow on read/write items with a stopped event
on example of a serializer, crypting and buffering plugin:

  write "abc"
    trigger "setItem.pre"
      serialize -> 's:3:"abc";'
      crypt -> "??????"
    write "??????" to storage
    trigger "setItem.post"
      write "??????" to buffer

  read
    trigger "getItem.pre"
      read "??????" from buffer -> stop propagation
    return "??????"

As you can see the getItem method returns immediately after the item was found in buffer and returns crypted data instead of uncrypted and unserialized data.

Updated/Fixed event flow on read/write items with a stopped event
on example of a serializer, crypting and buffering plugin:

  write "abc"
    trigger "setItem.pre"
      serialize -> 's:3:"abc";'
      crypt -> "??????"
    write "??????" to storage
    trigger "setItem.post"
      write "??????" to buffer

  read
    trigger "getItem.pre"
      read "??????" from buffer -> stop propagation
    trigger "getItem.post"
      uncrypt -> 's:3:"abc;'
      unserialize -> "abc"
    return "abc"

Now (after this PR) the getItem.post trigger will be called even if an getItem.pre-Event has propagation stopped and the crypted data from buffer will be uncrypted and unserialized as expected.

@Ocramius
Copy link
Member

@Ocramius You are right, tests are missing currently but I don't have time as I'm parental leave for 3 month starting this weekend :)

I guess I have to congratulate you :-)

@Ocramius
Copy link
Member

@marc-mabe I saw you lurking on php internals, are you back from your leave? Am I allowed to bug you here? :D

@marc-mabe
Copy link
Member Author

@Ocramius First thanks for your congratulation! My parental leave ends on end of august but I'm back from travel ;)

I hope I can add the missing tests until end of next week. (Coding is a bit different than reading mails with a baby)

@Ocramius
Copy link
Member

My parental leave ends on end of august but I'm back from travel ;)

Then let's wait till then, no? :)

@marc-mabe
Copy link
Member Author

@Ocramius unit tests are available now !!!

@Ocramius Ocramius added this to the 2.4.0 milestone Aug 13, 2014
@Ocramius Ocramius self-assigned this Aug 13, 2014
@Ocramius Ocramius added bug and removed enhancement labels Nov 14, 2014
Ocramius added a commit that referenced this pull request Nov 14, 2014
@Ocramius
Copy link
Member

Manually merged at 1e070d5

Thanks @marc-mabe!

Also note that I merged this only in 2.4, as it is a too risky change for 2.3.4

@Ocramius Ocramius closed this Nov 14, 2014
gianarb pushed a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants