laravel/framework — pull request #3864 by dcarrith (2014-03-12 06:52:09+00:00)
Title: Unit Tests for SQS - Push support for SQS utilizing SNS - Overhaul of SubscribeCommand - Addition of UnsubscribeCommand - Move setContainer to Job base class and more

This PR has grown out of a few other proposals/PRs from myself and @SammyK (#3759, #3778, #3784, #3789) so I'd like to get some more eyes on the additions/deletions/changes that are included in the branch.

First, a little background on this. I started out by writing unit tests for SqsQueue and SqsJob since there were none and also made a couple other small changes. @taylorotwell wanted me to submit just 1 PR to cover those few things. Then, @SammyK proposed (and implemented) adding push support for SQS, so I wanted to get that included since there were going to be merge conflicts, so I made him a contributor to my fork and he then created a branch and pushed his changes into my fork. To squash my 23 commits that could be seen in the original PR #3860, I just manually copied over the 19 or so files and committed to the branch @SammyK had created (this branch).

I'd like to make sure this is all worthy of being included in the core of the framework and make sure any issues that people see are addressed so I can get everything ready to be merged.

It's hard to tell if I broke anything with my SubscribeCommand refactor since there weren't any unit tests and I was only using it for testing the IronMQ and SQS/SNS push queue functionality using the default queue config. I didn't really like how it was using IronMQ's updateQueue function (rather than the addSubscriber) for subscribing (along with all the functions for gathering meta data for the queue) since that seems better suited for a new command, something like php artisan queue:update -whatever (via a new UpdateCommand). However, I did make sure to call updateQueue after call to addSubscriber just in case it actually is needed (minus the subscribers part of the options parameter). I also didn't like the checks for if($queue instanceof IronMQ) since it seems like (and I think I've shown) it can be better handled through polymorphism. Additionally, I felt like the setContainer functions that were in every derived class of Job should be removed and just defined in the base class.

There are also a bunch of unit tests I wrote, so I'd love to get some feedback from a phpunit expert. Anyway, there are a lot of other changes to discuss and get feedback on, so please have a look. Thanks for taking the time.