Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The $uri and $method params are now required when setting up Request. #68

Merged
merged 2 commits into from
Oct 13, 2016

Conversation

evert
Copy link
Member

@evert evert commented Oct 13, 2016

This ensures that it's not possible to create a Request object that's
not valid.

This ensures that it's not possible to create a Request object that's
not valid.
@evert evert added this to the 5.0.0 milestone Oct 13, 2016
@evert evert merged commit f9354d5 into master Oct 13, 2016
@evert evert deleted the require-method-uri branch October 13, 2016 01:50
@@ -117,6 +120,13 @@ function setAbsoluteUrl(string $url) {
*/
function getAbsoluteUrl() : string {

if (!$this->absoluteUrl) {
// Guessing we're a http endpoint.
$this->absoluteUrl = 'http://' .
Copy link
Member

Choose a reason for hiding this comment

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

Shouldnt we guess the protocoll based on e.g. The request uri, port or something?

Or maybe define the default protocol somewhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

So we kind of do both already. Usually the absolute url comes from the Sapi. It actually populates absoluteUrl using the information from $_SERVER and tends to do this accurately.

In cases where you are using the object in a client, you generally populate the information yourself. A request without an absolute url cannot be made really. This is really the ultimate fallback if all else fails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants