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

Provide factories and a ConfigProvider #57

Conversation

weierophinney
Copy link
Member

@weierophinney weierophinney commented Mar 5, 2018

This patch adds factories for each of:

  • DispatchMiddleware
  • ImplicitHeadMiddleware
  • ImplicitOptionsMiddleware
  • RouteMiddleware

Whenever a response prototype is needed by the underlying middleware, the factory checks for either a response instance returned, or a callable; in the latter case, it invokes the callable to retrieve a new response instance to inject.

It also adds a ConfigProvider class mapping each of the above middleware, and exposes it to zend-component-installer via the package definition.

This patch builds on #54 and #55; merge only after those packages have been merged.

weierophinney added a commit to weierophinney/zend-expressive-router that referenced this pull request Mar 5, 2018
@weierophinney weierophinney force-pushed the feature/factories-and-provider branch from a98a7aa to 42dd304 Compare March 5, 2018 17:52
weierophinney added a commit to weierophinney/zend-expressive-router that referenced this pull request Mar 5, 2018
@weierophinney weierophinney force-pushed the feature/factories-and-provider branch from 42dd304 to 17735a9 Compare March 5, 2018 17:58
weierophinney added a commit to weierophinney/zend-expressive-router that referenced this pull request Mar 5, 2018
@weierophinney weierophinney force-pushed the feature/factories-and-provider branch from 17735a9 to e6c534d Compare March 5, 2018 19:45
weierophinney added a commit to weierophinney/zend-expressive-router that referenced this pull request Mar 5, 2018
Copy link
Member

@michalbundyra michalbundyra left a comment

Choose a reason for hiding this comment

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

Travis build is failing on PHP 5.6:

PHP Fatal error: Cannot use RuntimeException as RuntimeException because the name is already in use in /home/travis/build/zendframework/zend-expressive-router/src/Exception/MissingDependencyException.php on line 11

And in general it's fine, I like adding ConfigProvider but we need to check zend-component-installer if it will inject ConfigProvider to the config on package update.

@weierophinney weierophinney force-pushed the feature/factories-and-provider branch from e6c534d to 9670557 Compare March 6, 2018 14:12
weierophinney added a commit to weierophinney/zend-expressive-router that referenced this pull request Mar 6, 2018
@weierophinney weierophinney force-pushed the feature/factories-and-provider branch from 93ca715 to 0f06f57 Compare March 7, 2018 23:29
weierophinney added a commit to weierophinney/zend-expressive-router that referenced this pull request Mar 7, 2018
@weierophinney
Copy link
Member Author

I've rebased this now off my current work for #55; it should be ready for review.

@weierophinney weierophinney force-pushed the feature/factories-and-provider branch from 0f06f57 to e251008 Compare March 7, 2018 23:37
weierophinney added a commit to weierophinney/zend-expressive-router that referenced this pull request Mar 7, 2018
@weierophinney
Copy link
Member Author

This is also ready for review.

zend-component-installer does not operate on update; this is so that users do not get prompted for a package they've already indicated they do not want to install configuration for.

We will need to message to users to add this — however, all features in this package are opt-in by nature, so having the configuration immediately available is not as necessary.

Copy link
Member

@michalbundyra michalbundyra left a comment

Choose a reason for hiding this comment

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

@geerteltink
Copy link
Member

zend-component-installer does not operate on update; this is so that users do not get prompted for a package they've already indicated they do not want to install configuration for.

We will need to message to users to add this — however, all features in this package are opt-in by nature, so having the configuration immediately available is not as necessary.

Use composer suggest? Not that many read this :)

Imports the following middleware and their related tests, adapting them
to work under PHP 5.6:

- ImplicitHeadMiddleware
- ImplicitOptionsMiddleware
- MethodNotAllowedMiddleware
Since we cannot fix the major issues with implicit HEAD and OPTIONS
requests without a BC break, we need for the backported middleware to
support v2 conventions. In this case, we'll look for `implicitHead()`
support.
Similar to previous patch regarding ImplicitHeadMiddleware; this adapts
to follow the conventions of v2, and uses `implicitOptions()` to
determine whether or not to intervene.
- implicitHead
- implicitOptions
@weierophinney weierophinney force-pushed the feature/factories-and-provider branch from e251008 to 20135f5 Compare March 8, 2018 14:23
weierophinney added a commit to weierophinney/zend-expressive-router that referenced this pull request Mar 8, 2018
Improves the testInvokesHandlerWhenRouteImplicitlySupportsHeadAndSupportsGet
logic to properly exercise a case where a HEAD request is issued, the
route does not explicitly support it, but _does_ support a GET request.

In doing so, discovered that the final return statement was incorrectly
using the response prototype, instead of the response returned from the
GET request.
Adds factories for each of:

- DispatchMiddleware
- ImplicitHeadMiddleware
- ImplicitOptionsMiddleware
- RouteMiddleware

Whenever a response prototype is needed by the underlying middleware,
the factory checks for either a response instance returned, or a
callable; in the latter case, it invokes the callable to retrieve a new
response instance to inject.

Conflicts:
	composer.lock
Adds `Zend\Expressive\Router\ConfigProvider`, with dependencies mapped
for all new middleware shipped.

The package is exposed to zend-config-provider as well.

Conflicts:
	composer.lock
@weierophinney weierophinney force-pushed the feature/factories-and-provider branch from 20135f5 to c94430d Compare March 8, 2018 14:39
@weierophinney weierophinney merged commit c94430d into zendframework:develop Mar 8, 2018
@weierophinney weierophinney deleted the feature/factories-and-provider branch March 8, 2018 14:56
weierophinney added a commit that referenced this pull request Mar 8, 2018
Forward port #54
Forward port #55
Forward port #57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants