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

FB-115 Change App Url format #2635

Closed
wants to merge 36 commits into from
Closed

Conversation

zazabe
Copy link
Contributor

@zazabe zazabe commented May 11, 2017

  • Extract site / language / version from Url path parts
  • Use AppUrl in CM_Request
  • Map CM_Response with AppUrl

@zazabe zazabe changed the title Change App Url format FB-115 Change App Url format May 11, 2017
@zazabe
Copy link
Contributor Author

zazabe commented May 11, 2017

@{c7e8ab9c-90bf-4796-bb88-9aa7c8150044,Reto Kaiser} here we are... this is the challenging PR i told you, about https://youtrack.cargomedia.ch/issue/FB-115...

replace CM_Request::getPathPart($index) by CM_Request::getUrl()->getPathSegmentAt($index)
Used by log formatter, because getQuery() could raise an exception (eg. CM_Http_Request_Post)
@njam
Copy link
Member

njam commented May 12, 2017

There are some problems with setUrlFromString(), which needs to instantiate a specific type of URL. This is needed so that later the Request::getUrl() can be used to generate correct URLs.
Unfortunately there are way to many dependencies in this area of code (Request, Response, URL, etc), and it's uncertain if it makes sense / is possible to refactor it.

One idea would be to cut the dependencies at CM_Http_Response_Abstract, by ensuring it doesn't receive the Request.

@njam
Copy link
Member

njam commented May 15, 2017

Idea:

  • Keep single type of Url on Request (Url\Url)
  • Move parsing of URL to different place (out of Url) - will provide functionality like Request::popUrlEnvironment()
  • Move generating of URLs to different place (out of Url) - will provide functionality like Render::getUrlLayout()

@njam njam closed this May 15, 2017
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.

2 participants