Proposing removal of subscription record upon unsubscribe #3295

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

quocvu commented Dec 27, 2012

Currently both subscribe and unsubscribe operations to a feed execute the same code resulting into the same behavior. As a result subscription records created in the database have the same state and we can't tell which ones were unsubscribed and which ones are still subscribed.

I am proposing to keep the subscribe operation as currently implemented but tweak to unsubscribe operation as the following:

  1. when the unsubscribe request is sent to the hub, update the subscription record STATE column as to_delete
  2. once the hub sends the callback and it's been verified, delete the subscription record

Please let me know if this fit the original design ideas.

Owner

weierophinney commented Jan 3, 2013

Looks reasonable. @quocvu can you please provide some unit tests?

Contributor

quocvu commented Jan 4, 2013

Both changes that I suggested manifest only inside the database. Furthermore it happens on a DB table that is unknown to Zend as it injected by the developer.

@weierophinney could you please advise on how we can unit test such case with your current setup?
Thanks

Owner

weierophinney commented Jan 7, 2013

@quocvu Mock the storage, and test that the deleteSubscription() method is called once with the appropriate data; this way, no DB is necessary.

Contributor

quocvu commented Jan 9, 2013

@weierophinney I modified the existing test to reflect my changes but ran into these 2 problems:

  1. w/ the current setup tests in ZendTest/Feed/PubSubHubbub/SubscriberHttpTest.php are executed since they are dynamic

  2. while trying test the delete of record, I realized mock up storage object aren't deleted. How can this simulated in PHP Unit?

You can look at my repo. It has the changes checked in

@weierophinney weierophinney added a commit that referenced this pull request Jan 14, 2013

@weierophinney weierophinney [#3295] Fixed tests and logic flow
- Fixed the unit tests for the unsubscribe functionality. The table
  gateway was expecting the "delete" method now (not "update"), with
  only the "id" member in the array argument; additionally, we needed to
  test the return value, so I added a "will" clause.
- Altered the workflow of Callback to use a switch() statement for the
  "hub_mode"; this will make adding additional cases in the future
  easier, and allows us to raise an exception on an invalid mode.
ccb9df6

@weierophinney weierophinney added a commit that referenced this pull request Jan 14, 2013

@weierophinney weierophinney Merge branch 'hotfix/3295' into develop
Forward port #3295
5669636
Owner

weierophinney commented Jan 14, 2013

I identified the issue with the mocking, and fixed it on merge. Thanks for the patch!

@weierophinney weierophinney added a commit to zendframework/zend-feed that referenced this pull request May 15, 2015

@weierophinney weierophinney [zendframework/zendframework#3295] Fixed tests and logic flow
- Fixed the unit tests for the unsubscribe functionality. The table
  gateway was expecting the "delete" method now (not "update"), with
  only the "id" member in the array argument; additionally, we needed to
  test the return value, so I added a "will" clause.
- Altered the workflow of Callback to use a switch() statement for the
  "hub_mode"; this will make adding additional cases in the future
  easier, and allows us to raise an exception on an invalid mode.
a7ef12f

@weierophinney weierophinney added a commit to zendframework/zend-feed that referenced this pull request May 15, 2015

@weierophinney weierophinney Merge branch 'hotfix/3295' 6a72de0

@weierophinney weierophinney added a commit to zendframework/zend-feed that referenced this pull request May 15, 2015

@weierophinney weierophinney Merge branch 'hotfix/3295' into develop 8abd6e8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment