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

Added the any() method. #116

Closed
wants to merge 5 commits into from
Closed

Added the any() method. #116

wants to merge 5 commits into from

Conversation

spiffyjr
Copy link
Contributor

This method is useful for defining a single middleware that will respond to any of the $httpRouteMethods. For example, a Restful controller that has logic to route to the appropriate method internally:

$app->any('/foo', Api\RestfulController::class);

This method is useful for defining a single middleware that will respond
to any of the $httpRouteMethods. For example, a Restful controller that
has logic to route to the appropriate method internally:

$app->any('/foo', Api\RestfulController::class);

Remove any check
$this->assertEquals('/foo', $route->getPath());
$this->assertSame($this->noopMiddleware, $route->getMiddleware());

$reflClass = new \ReflectionClass($this->getApp());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move ReflectionClass to use statement

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spiffyjr I agree with @samsonasik - but have a different suggestion:

  • Import ReflectionProperty into the file.
  • Use the following:
    php $app = $this->getApp(); $r = new ReflectionProperty($app, 'httpRouteMethods'); $r->setAccessible(true); $routeMethods = $r->getValue($app);

Since you're interested in a specific property, do reflection directly on it, instead of pulling it from class reflection. 😄

@weierophinney
Copy link
Member

That's what the route() method already does if no HTTP method are provided.
It an I missing something about your intent?
On Aug 29, 2015 4:05 PM, "Kyle Spraggs" [email protected] wrote:

This method is useful for defining a single middleware that will respond
to any of the $httpRouteMethods. For example, a Restful controller that
has logic to route to the appropriate method internally:

$app->any('/foo', Api\RestfulController::class);

You can view, comment on, or merge this pull request online at:

#116
Commit Summary

  • Added the any() method.

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#116.

@spiffyjr
Copy link
Contributor Author

spiffyjr commented Sep 1, 2015

Oh, honestly, I just assumed that route() required methods. I should RTFM?

@spiffyjr spiffyjr closed this Sep 1, 2015
@spiffyjr spiffyjr deleted the feature/any branch September 1, 2015 14:23
@weierophinney
Copy link
Member

@spiffyjr

Relevant docs are:

Essentially, if you only pass the route and middleware to route(), it assumes any HTTP method; you can pass null or Zend\Expressive\Router\Route::HTTP_METHOD_ANY as the third argument to specify matching all methods, too.

That said, it's more verbose than your proposed any() method for situations when you want to specify a name, which might lend weight to adding this. Thoughts?

@spiffyjr
Copy link
Contributor Author

spiffyjr commented Sep 2, 2015

I'd agree. It's also similar to other frameworks where route is typically used for custom mapping, e.g., POST and PUT but not GET, etc.

@spiffyjr spiffyjr restored the feature/any branch September 2, 2015 12:49
@spiffyjr spiffyjr reopened this Sep 2, 2015
@weierophinney
Copy link
Member

@spiffyjr Can you also update the doc/book/application.md file to indicate this new method, please?

@spiffyjr
Copy link
Contributor Author

spiffyjr commented Sep 2, 2015

I originally had an entire section on any() but removed in favor of this version. It's simple and the word "any" makes it obvious what it does.

*/
public function any($path, $middleware, $name = null)
{
return $this->route($path, $middleware, $this->httpRouteMethods, $name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pass a null or Router\Route::HTTP_METHOD_ANY for the third argument. $httpRouteMethods only really exists to determine what methods are valid overloading methods.

@spiffyjr
Copy link
Contributor Author

spiffyjr commented Sep 2, 2015

I used Router\Route::METHOD_ANY for brevity (and clarity!).

@weierophinney
Copy link
Member

for brevity

You mean "clarity," right? :) Looks good; will merge later today!

@spiffyjr
Copy link
Contributor Author

spiffyjr commented Sep 2, 2015

Dammit man, I'm a programmer not an English major. :D

@weierophinney
Copy link
Member

@spiffyjr Looks like you need to use null, and not the Route::HTTP_METHOD_ANY constant:

@spiffyjr
Copy link
Contributor Author

spiffyjr commented Sep 2, 2015

That's what I get for using the online editor. That's also what Travis is for! Hooray!

@weierophinney
Copy link
Member

Still failing:

Essentially, you should be asserting against Route::HTTP_METHOD_ANY instead of the array of methods. (In other words, you can totally forget the ReflectionProperty stuff I mentioned in a previous comment.)

@weierophinney weierophinney added this to the 0.2.0 milestone Sep 3, 2015
@weierophinney weierophinney self-assigned this Sep 3, 2015
weierophinney added a commit that referenced this pull request Sep 3, 2015
weierophinney added a commit that referenced this pull request Sep 3, 2015
- Assert against `Route::HTTP_METHOD_ANY`, not array of routes.
weierophinney added a commit that referenced this pull request Sep 3, 2015
weierophinney added a commit that referenced this pull request Sep 3, 2015
@weierophinney
Copy link
Member

Merged to develop for release with 0.2.0.

@spiffyjr spiffyjr deleted the feature/any branch September 3, 2015 22:11
@spiffyjr
Copy link
Contributor Author

spiffyjr commented Sep 3, 2015

Having a kid at the moment so thanks for updating this.

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.

3 participants