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

Twig filter/method for generating phpcr URLs that work even if document is moved. #178

Open
benglass opened this issue Sep 25, 2013 · 34 comments
Labels

Comments

@benglass
Copy link
Member

There should be a built in way to embed links/urls to other routes in the CMF into the body of a PHPCR document that can be resolved to the correct URL for the page whether the page has been moved or renamed. This is a feature that other CMS's have, for example in MODx you can have something like in your content

<a href="[~14~]">A link to page with id 14</a>

And the CMS will expand that to the correct URL for the page of that id. This allows the end user to change page URLs without having to update links throughout their site. This may be possible for any documents that use the referenceable mixin as they have a unique id.

Original discussion at symfony-cmf/simple-cms-bundle#77

@benglass
Copy link
Member Author

@dbu It would be be able to handle both regular symfony routes and phpcr routes but I feel the client use case is primary to insert links to other cms pages. Inserting dynamic routes with parameters would require a complicated user interface wherein you could select possible parameters. This part of the feature would be most used by developers and perhaps could be achieved by allowing some subset of twig (for example path() or url() calls could be evaluated). Perhaps this could be used as the approach for both kinds of url expansion (phpcr and symfony routes). Then the functionality @uwej711 mentioned in symfony-cmf/simple-cms-bundle#77 (comment) could perhaps be made more robust so it could handle uuid instead of a phpcr path which is volatile.

In terms of handling situations where the cms user delete a page and recreates it elsewhere im wondering whether this is should be wrapped in with this feature or considered a separate enhancement as I do not think this is something most cms' handle.

@dantleech
Copy link
Member

See also the RoutingAutoBundle for a different approach to this problem:

http://symfony.com/doc/current/cmf/bundles/routing_auto.html

Basically that bundle will automatically create routes based on given rules for a given Document (e.g. the Page). You can then, for instance, tell it to leave Redirect routes when the page is moved.

@dantleech
Copy link
Member

As an aside, I was thinking that it would be quite simple to create a "SimpleCMS" using the RoutingAutoBundle and the original routing system (i.e. not having a custom RouteProvider etc).

@benglass
Copy link
Member Author

Interesting, it seems the RoutingAutoBundle could potentially already address the requirement of automatically redirecting when a document is moved provided that the link pointed at the auto route and not the original document. Presumably AutoRoutes have uuids and could therefore be referenced in a link and parsed out in the way as any other phpcr document. We would just need perhaps a service that provides a list of pages from which the user can select when inserting a link in the wysiwyg. Whether this service loads autoroutes or documents would be up to the configuration, perhaps some kind of Interface could be provided for this and a few base implementations although it may be enough to simply allow the basepath to be configurable on a generic provider, which would load all of the routes found at that basepath.

@dantleech
Copy link
Member

Not sure I understand fully - in what scenario would you want to "parse
out" an auto routes UUID from a link?

On Wed, Sep 25, 2013 at 12:12:03PM -0700, Ben Glassman wrote:

Interesting, it seems the RoutingAutoBundle could potentially already
address the requirement of automatically redirecting when a document is
moved provided that the link pointed at the auto route and not the
original document. Presumably AutoRoutes have uuids and could therefore be
referenced in a link and parsed out in the way as any other phpcr
document. We would just need perhaps a service that provides a list of
pages from which the user can select when inserting a link in the wysiwyg.
Whether this service loads autoroutes or documents would be up to the
configuration, perhaps some kind of Interface could be provided for this
and a few base implementations although it may be enough to simply allow
the basepath to be configurable on a generic provider, which would load
all of the routes found at that basepath.


Reply to this email directly or [1]view it on GitHub.

References

Visible links

  1. Twig filter/method for generating phpcr URLs that work even if document is moved. #178 (comment)

@benglass
Copy link
Member Author

I havent used the auto routing so perhaps im misunderstanding but i was thinking that if a user was editing a page in the backend and inserts a link they would be able to choose from a list of pages/routes generated by the cmf. When selecting one, it would insert some special format

<a href="%%%path55%%%">Link to doc 55</a>

I was thinking 55 would be the uuid of the document to link to in phpcr. If you are using SimpleCms the uuid might be an instance of Page from /cms/simple. If you are using AutoRouting perhaps its the uuid of an AutoRoute. What options are presented to the user in terms of links they can insert to existing documents would be determined by a basepath configuration or perhaps a service that provides a Tree of documents or something similar would could be used to build a select dropdown.

@dantleech
Copy link
Member

Ah yes. Well, in that case if the original URL was "/blog/post-1" and it
was moved to "/blog/post-2" then the routing auto bundle would have just
dropped a redirect route at "/blog/post-1" (redirecting to
"blog/post-2"). So with that approach there would be no need to have a
subsitute.

But while thats great for URLs sent out by email etc. It is les good for
internal articles that we render, in which case some sort of thing like
you propose would indeed make sense.

On Wed, Sep 25, 2013 at 01:11:06PM -0700, Ben Glassman wrote:

I havent used the auto routing so perhaps im misunderstanding but i was
thinking that if a user was editing a page in the backend and inserts a
link they would be able to choose from a list of pages/routes generated by
the cmf. When selecting one, it would insert some special format

Link to doc 55

I was thinking 55 would be the uuid of the document to link to in phpcr.
If you are using SimpleCms the uuid might be an instance of Page from
/cms/simple. If you are using AutoRouting perhaps its the uuid of an
AutoRoute. What options are presented to the user in terms of links they
can insert to existing documents would be determined by a basepath
configuration or perhaps a service that provides a Tree of documents or
something similar would could be used to build a select dropdown.


Reply to this email directly or [1]view it on GitHub.

References

Visible links

  1. Twig filter/method for generating phpcr URLs that work even if document is moved. #178 (comment)

@dbu
Copy link
Member

dbu commented Sep 26, 2013

i would propose that instead (or in addition to) an internal link target selector, we should also have a doctrine listener that finds link to the site domain and resolves the target path of a normal <a href="... with the router to detect the route / content document and replaces the absolute link with the phpcr path and the uuid.

having the user put a uuid or something as link target will not work in frontend editng with CreateBundle, as the filter is then already applied and the link transformed into the real link. on the next save in frontend, the id would get overwritten.

@benglass
Copy link
Member Author

@dbu good point and i can think of other ways in which front end editing could cause issues like.

For example on our cms architecture we are not currently using the concept of a page being composed of multiple blocks (same with simple cms) so I implemented a twig filter which inserts dynamic calls to blocks based on a format.

<p>Some static content</p>
%%%sonata.block.service.foo &bar=`baz`%%%
<p>Some more static content

The %%%foo &bar=baz%%% would be replaced with the sonata.block.service.foo block service and called with the options array('bar' => 'baz').

This would be rendered by create as the final HTML output of the block and would be broken by front end editing. This approach is actually based on one that is written about in the docs of cmf block bundle at http://symfony.com/doc/current/cmf/cookbook/using_blockbundle_and_contentbundle.html#tutorial-block-embed

It makes me wonder about the approach of allowing users to editing filtered content through the front end as it could have a lot of potential issues similar to these which we cannot predict. Making it so users are editing the content that is stored in the phpcr (instead of the rendered content) may have other issues (we'd have to save the content and re-filter it and inject it into the space after editing).

  1. Can we somehow mark parts of the content as non front-end editable?
<span class="no-editable">
The dynamically generated html
</span>

Not even sure this would help unless we then matched up the no-editable sections with the original content which seems messy and error prone.

  1. Perhaps we can just allow the developers to disable front end editing on a page by page basis
  2. The doctrine listener for the create cmf that converted site URLs (such as /foo/bar) back into uuid's would solve the issue for these URLs but not for embedded blocks or other dynamic content.

Putting all your content into a collection of child blocks can sidestep the issues of the dynamic content a bit.

For me I think it would be an acceptable solution if I could just disable frontend editing of some pages and if the edit button just took users to sonata admin page edit for those pages but id love to hear other folks' thoughts.

@dbu
Copy link
Member

dbu commented Sep 27, 2013

i think the url is a very basic and thus special case where a back-and-forth conversion would make sense.

setting a part non-editable will not help, its still part of your wysiwyg blob that gets stored back into the db. what the embed block thing could do would be to have the filter render something around the magic tag that allows a listener to remove the rendered stuff and restore the call (unless the frontend editor goes destroying that something while editing, but then screw him). something like <div cmf-block-render="{eventual:parameters}">.... this concept could be applied to any filter thing somebody comes up with: if you provide a filter, you should provide a listener as well.
each filter you activate is going to cost you. listening on save operations should be comparably cheap as its not happening that often. and anyway is not more expensive than rendering the page.

deactivating frontend editing on a per page basis should become possible at some point. you definitely can do it on a per template basis by not rendering the createjs annotations. but once we have fine grained permission management, we need more. right now we just don't render the create.js inclusion if you are not allowed to edit (based on a role). anyway, this topic belongs into CreateBundle... for the OT i would vote for listeners to convert things back.

@dbu
Copy link
Member

dbu commented Sep 27, 2013

btw, as we already have the block twig filter, can you open an issue on BlockBundle explaining what was missing from the provided filter? could we extend the filter to be able to do what you want? or is it a very special case?

@benglass
Copy link
Member Author

@dbu perhaps this can go into block bundle but the reason I created a separate filter was that I am not embedding blocks that are stored in the database. These are service oriented blocks that are not about there being editable content stored in the cmf. They are basically just standard sonata blocks and I want to render them inside a cmf doc.

The main missing feature (which could certainly be added to block bundle) would be the ability to specify parameters in the embedded call. This is key for my use case where you are listing projects (or perhaps showing 3 featured products in which case we want the options of feature=1 and limit=3 in the embedded call). Ill move the discussion over there and see if there could be some enhancement to block bundle to suit this.

@benglass
Copy link
Member Author

@dbu I have moved the block discussion to a new issue symfony-cmf/block-bundle#143

@benglass
Copy link
Member Author

@dbu so at this point it looks like we can move forward with adding the following

  1. A twig filter that converts uuids to URLs

    What format should it be looking for? I like [uuid] as its very simple. It seems it might be worthwhile to come up with a standard format for dynamic content like this in documents which may be replaced. MODx uses brackets as does wordpress. Alternately it could be something that looks like like twig {{ path('', { phpcr_uuid: 55 }) }} and then we could actually parse it out and call the path function which could be made to support phpcr_uuids which would have other advantages.

  2. A doctrine preUpdate listener (or listener to a specific CreateBundle event that is fired before save would be better since its only for front end editing) that converts relative URLs or absolute URLs containing the current domain to the format we decide on.

I would say the other items we discussed such as parsing dynamic sections should be treated as a separate issue.

@dbu
Copy link
Member

dbu commented Sep 27, 2013

  1. sounds great. i would not go into sub-rendering twig, that is quite tricky business as it can lead to security issues. lets go with [uuid].
  2. i would do a doctrine listener. then the backend editor can also insert his links as absolute links without troubles. easier than writing plugins for all the wysiwyg editors and make source editing people figure out uuid's. i wonder if we should store the uuid of the Route or of the Content. when generating it does not matter as long as the content is still implementing RouteReferrersInterface. maybe lets go for route? as even if you move it and we lose it, there might still be a redirectroute at that place.

magnolia cms stores not only the uuid but also the path. this has the benefit that after migrating content or destroying and rebuilding a content, things still work. the uuid has the benefit that after moving, things still work. so ideally we would also store both path and uuid to have a way to survive as much as possible.

the filter for dynamic sections should go to BlockBundle, created symfony-cmf/block-bundle#144 for that.

@dbu
Copy link
Member

dbu commented Sep 27, 2013

so the magic thing could be [uuid:|path:] - but then we need to make sure to encode the path in a way that hides ~. could we use some other separator that is easily escepable in the path or not valid in a phpcr path in the first place? * might be a good path invalid character. [*uuid:<uuid>|path:<path>*]

@benglass
Copy link
Member Author

@dbu I am not grasping the magnoa cms/storing the path in addition to the uuid requirement, it seems like redundant data. Can you please explain further? You are talking about if the user deletes page uuid 1 at path /cms/about and then creates page uuid 2 at path /cms/about (the same path)? I suppose that would be a nice feature to support but it seems like a bit of an edge case that would add additional complexity. Perhaps handling paths as well to survive delete/recreate scenarios could be an extra enhancement.

I have no problem with [uuid:55] as a format. Perhaps we should consider using 2 brackets. My reasoning is that in MODx for example you can embed different kinds of things using these special tags, whether they are URLs or system settings. Which type of thing is loaded depends on the special character used in addition to the brackets so

[[55]]

Might be a url, whereas

[[#something.foo.bar#]]

Might be a container parameter that would be automatically injected.

@dbu
Copy link
Member

dbu commented Sep 27, 2013

ah, i see. well that sounds like the ultra dense wiki syntax that then are hard to read if you don't know them very well. i would prefer to be explicit. for the url thing i was assuming the base wrapper is that its href="<our thing>" that lets us detect it. oh and src="" for images i guess. but indeed thinking about this more generally might be a good idea.

the delete and recreate scenario is indeed an edge case. but dumping the repository and re-importing it is rather standard, e.g. to see the data on a stage or local dev env. re-importing can lead to uuid changing. depending on your settings when importing they either change for sure or only in case of conflicts. that scenario is imho less of an edge case.

@benglass
Copy link
Member Author

adding path support does sound like a good idea and it doesnt really seem particularly more complex from an implementation standpoint (just falling back to load by path if uuid is not found).

regarding the format i think this is a feature for developers more than non-technical users (clients would insert these links via some kind of wysiwyg editor function). Having clients understand the concept of uuids and paths is something we probably want to avoid regardless of the format of this.

I think it would be best not to assume these placeholders would appear in only certain places like href="" or src="" because users may want to use them to populate javascript variables or other things we may not think of.

Having a single format like

[uuid:55 path:55]

may be just fine and could be expanded to handle other things like parameters via

[parameter:key]

This would be more explicit

Perhaps the format should be more like

[filter_provider param1:value param2:value]

So we'd have

[url uuid:55 path:/cms/about]

and maybe later on

[parameter name:foo.param.name]

This meets the requirements of being explicit and also establishes a pattern that could be used for other things. We could potentially treat it like MenuProviders and register services via a tag that would be used by the filter to provide expansion of this snippet format based on the filter_provider. By default we could provide a phpcr_url_filter_provider

I am wondering if we should consider the name url vs path. In symfony those are used in templates to differentiate between absolute and relative.

@benglass
Copy link
Member Author

This could also dovetail with the way that the cmf_embed_block works. Instead of having a separate twig filter that bundle would just register a phpcr_block_filter_provider. That way the concept or parsing options from the string would be all handled in one place.

@dbu
Copy link
Member

dbu commented Sep 27, 2013

very interesting ideas. having one common format sounds like a good
idea. for the syntax we have to be careful, as phpcr paths can contain
spaces and colons...

for the reverse transform, a url is special, it can not have a tag
around itself as its part of a tag. where the blog can well have some
tagging around. so we still have somewhat two cases at least for the
storage filter. or the standard case of blocks and the special case of urls.

supporting both path and url makes sense. we can also detect that when
we see if href / src has a domain or not. btw, not sure how you would
parse out paths occuring at random locations. for urls it could actually
work.

@benglass
Copy link
Member Author

im thinking we may want to wrap the values in backticks to address them being able to contain anything, is backtick allowed in a phpcr path?

[*url uuid=`55` path=`/path:to/containing=equals`*]

In terms of parsing them out in random places I was envisioning that we are just going to be like the existing embedBlocks functionality https://github.com/symfony-cmf/BlockBundle/blob/master/Templating/Helper/CmfBlockHelper.php#L54

I am thinking perhaps a more unique delimter than [* may be required as if the content contained

<p>We offer everything* you can image [*well not everything] go look at <a href="[*url uuid=`55`*]">this</a></p>

It could create false positives. Seems like an unlikely situation but making the delimeter very unique would prevent it even as a possibility so perhaps having 2 brackets.

[[*url uuid=`55` path=`/path:to/containing=equals`*]]

I prefer the equals syntax to the colon as it seems more intuitive. We could also allow configuration of the delimeter just as the block bundle does.

@dbu
Copy link
Member

dbu commented Sep 28, 2013

a * is about the only thing that can really not occur in a phpcr path.
the alternative would be blank space and encode spaces with %20. well,
we could encode any delimiter we use with its character code that way.
making delimiters configurable sounds right, then people have a way out
for special cases. the uniqueness is why the block tag has this whole %
and a name business.

@uwej711
Copy link
Member

uwej711 commented Sep 28, 2013

Just an idea, why not storing that information in a data attribute of the a tag (data-target-uuid="...")? It should be easy to do that with ckeditor (it has some add link functionality) with the added benefit to have it for frontend and backend editing.

@benglass
Copy link
Member Author

@dbu im not seeing an easy solution to picking delimeters that wont run into possible encoding issues in terms of values and this should be able to support any string value so im voting for configurable delimeters that are escaped in any values that use them. This means we will probably have a Helper class which can do this encoding/decoding.

So i guess im saying a configurable delimeter (wondering if this should be configurable in the cmf_core because what im advocating is a general cmf twig filter that uses a provider concept to handle filtering the content based on the embed_filter_key. The filter providers would be tagged and gathered in a compiler pass and injected into the twig filter helper.

In the config

cmf_core:
    twig_embed_filter_delimiter: [[

In your document content

[[*url uuid=`55` path=`/path:to/containing=equals`*]]

This would be converted to a url by a service tagged as an embed_filter_provider, example service configuration

services:
    cmf_router.embed_filter_provider.url:
        class: Cmf\RouterBundle\Templating\Provider\EmbedFilterUrlProvider
        arguments:
            - @the.document.manager
        tags:
            -  { name: cmf_embed_filter_provider }

Pseudo code in the twig filter (see also https://github.com/symfony-cmf/BlockBundle/blob/master/Templating/Helper/CmfBlockHelper.php#L48)

    public function processedEmbedded($text)
    {
        // do a preg_replace_callback on the text looking for delimeter(embed_filter_name option1key=`option1value`)delimeter and calling embeddedRender
    }

    public function embededRender($embeddedMatches)
    {
        // Parse out the embedded_filter_name and the options into an array of key => value pairs
        // Loop through the providers from the cmf_embed_filter_provider tag and call $provider->has($embedded_filter_name)
        // If it returns true, then call $provider->get($embedded_filter_name, $options) which returns the replaced string
    }

@benglass
Copy link
Member Author

@dbu We are looking at this again for our CMS. We discussed possibly using a twig sandbox as it would provide a clear extension point. We would potentially provide config parameters for enabled methods/functions, etc. in the sandbox to make it easy to configure, something along the lines of:

    security_policy:
        tags: []
        filters: []
        methods: []
        properties: []
        functions: []
    sandbox:
        sandboxed_globally: false

@dbu
Copy link
Member

dbu commented Nov 29, 2013

so basically this would mean to evaluate embedded twig in the content, but evaluate it in a sandbox rather than the full application context? is that already done somewhere? it could make sense, just sounds like a complex undertaking to me :-)

and how will that help us with converting things back from the rendered html to the storage format, in case we used a wysiwyg editor in the frontend?

@benglass
Copy link
Member Author

@dbu Setting up the sandbox approach is actually pretty simple (ill post some code) and gains you the ability to embed any kind of content you might want into your document content which is a very powerful bonus. On the other hand it also introduces potential for abuse if you are too liberal with your sandbox policy. It also prevents having to do string parsing and introduce a new way to embed wiki-style tags in content and you can allow the path() twig function and take care of url generator. It does require the load_template_from_string twig extension/function as the usage is something like.

{% sandbox %}
    {% include load_template_from_string(cmfMainContent.content) %}
{% endsandbox %}

However I have discovered a few issues with using the existing path() function in this way (to insert links to cms pages).

  1. It throws an exception if the route isn't found, so if the target document moves it will break a page that links to it
  2. It is currently not working if you are trying to link to an absolute path that is not within the current basepath (the basepaths are dynamically modified based on the host matching a configured website so if you come in on foo.com the base path is mapped to /cms/foo and a path('/cms/bar/about') path will not be found even though its a valid path)
  3. I am still not able to find a way to link based on uuid (and im not sure if you can even get the uuid from a phpcr document easily via the doctrine models, I probably just need to map it to a property since i can see its being generated)

I think for my use case I will probably create a separate twig function for generating phpcr links that has the following additional behavior

  1. If an absolute path is passed in (starting with /) then just load the phpcr document from there and generate a URL if its a Route (otherwise throw an exception)
  2. If a relative path is passed in, take the currently configured base path and absolutizePath and proceed with step 1

This is similar to a PR I submitted to the menu bundle to allow absolute and relative paths to be passed to knp_menu_get/render so let me know if you think its something you'd like to have in the cmf or if you can see a way to integrate it into the existing path() function with causing breaking changes (i can't see how the relative path part would work beause it would mean you could never have a node in the phpcr with a name that is the same as a static route ie. a page at /foo/bar/about or /biz/baz/about would supercede a route named about). Also throwing an exception has to be caught/suppressed for a twig function that could easily fail like this (the document moved for example). This wouldn't be a problem if you have auto routing perhaps to set up 301 redirects but even then url generation code like this is somewhat volatile within a cms and can't throw an exception that could break the template.

@dbu
Copy link
Member

dbu commented Dec 15, 2013

for the question at hand, i think we want a specific thing that uses extra logic to figure out where to link to. in the sandbox, we could define a cmf_document_path funktion that accepts both path and uuid so that we can build the link from either repo path or uuid.

to get the uuid, we would either need to map the jcr:uuid field in Resources/config/doctrine-phpcr/Route.phpcr.xml and add a property and getter to the document class, or add something to phpcr-odm. the unit of work currently has getDocumentId, having getDocumentUuid would make sense to me.

so the relative paths would mean links inside the same domain? that would mean that the uuid could be wrong, if it is from a route of a different domain. for the routes outside of the domain, we would need an extra route generator i think, something like a "domain aware generator" that realizes if we create a crossdomain link and make sure the domain name is output as well. it would need to take care of configuring the right prefix as well, to make the route building successful. do you see what i mean? it looks to me like that is one, if not the bit we need to support multi-domain sites.
btw, it would be great if you could share how you set the basepath based on the domain. i would love to have a cookbook entry in the cmf doc explaining how to do multidomain with the cmf.

regarding hiding system routes, that depends on the order you configure your routers, but of course having multiple routers you always have the potential of one hiding the other.

by the way: here we talk about links in the content, right? a template loading routes stored in the repository by using their repository path would be flaky. you don't want to hardcode database paths in your templates, that would break as soon as an editor changes the path. if its something that must exist for sure, make it a static route. if it has to be editable, give it a special type so that you can find it dynamically.

regarding failing to create links, a thing that i think we should add to the corebundle twig helper is a function cmf_is_linkable

public function isLinkable($document)
{
    return $document instanceof RouteReferrersReadInterface && count($document->getRoutes()) > 0;
}

@benglass
Copy link
Member Author

@dbu

  1. I agree there should be a cmf_document_path function. Also perhaps we should have a cmf_document_url function for symmetry with symfony (path and url functions both exist). There was a discussion at some point about having these functions able to accept a path (/cms/main/about) AND a uuid at the same time and to use one as a fallback if the other isn't found to cover situations where either the path has changed OR the doc was deleted and re-created at the same path (different uuid).
  2. Regarding uuids I think it would be good to add this perhaps to simple cms default mappings for Pages as an example of how to use it since it seems to be the preferred way to link to a document as its much less volatile than a path.
  3. When I was referring to relative paths I meant a path like 'about' which would be within the current website. I meant that the cmf_document_path function should potentially support a relative path like this and automatically prepend the current website path.
  4. I agree creating a DomainAwareGenerator sounds like a good approach for handling generation of paths/urls for routes in other websites but I think the issue is actually broader than just url generation. The general problem is how to load routes from multiple basepaths. Currently the cmf classes that exist are set up to load from a single basepath. The approach we are using which is based on the PrestaCms approach is to use a request listener to figure out what website the user is insterested in and then modify the services that have to do with basepaths to have the right basepath for that website. I am thinking this is not a great approach and it would be better instead to allow those services to handle multiple basepaths (or provide versions of them that do).

A step in this direction would be identifying classes that handle basepaths, these are the ones we have been concerned with in order to handle multi-website routes

  • IdPrefixListener which sets the prefix on PrefixInterface items loaded from the phpcr so they know which part of their own path is a prefix
  • RouteProvider which needs to know what prefix to load routes from

Any insight into other services that fall into this category would be great.

Regarding the behavior of a DomainAwareGenerator it would basically determine if the route being generated was on the current domain/website and if not it would always return an absolute url.

I would be willing to post example code but our implementation is not substantially different from PrestaCMSCoreBundle

A request listener to detect the website based on host matching https://github.com/prestaconcept/PrestaCMSCoreBundle/blob/master/EventListener/WebsiteListener.php#L58

Which dispatches to a WebsiteManager to actually do the work of configuring the services that handle basepaths (and anything else that has to change based on the website). https://github.com/prestaconcept/PrestaCMSCoreBundle/blob/master/Model/WebsiteManager.php#L255
https://github.com/prestaconcept/PrestaCMSCoreBundle/blob/master/Model/WebsiteManager.php#L188

@benglass
Copy link
Member Author

@dbu just to elaborate on why i think this listener based approach doesnt work in my experience with our implementation here is a scenario

  • IdPrefixListener's prefix is set to /cms/main
  • You need to load a route outside of this prefix, for example /cms/other/about
  • You need to set the prefix on the IdPrefixListener to /cms/other before loading the about route from the ODM or you will get an exception that the prefix wasnt set properly
  • If you use the approach of changing the prefix to load a route in another base path then you are responsible for making sure it gets set back to the original value (/cms/main) after you load it or future routes could end up with that exception

This is the reason Im thinking having the prefix listener and other phpcr related services that know about a prefix be able to handle multiple prefixes would be better than trying to keep track of a single prefix which starts to feel like global state.

Another issue is when you have a cms admin area for managing multiple websites (sonata admin) and the user could be logged on any number of domains and switching the current website they are managing (tracked via the session since you cant determine it from the url). Then lets say they edit a page on website A then switch to website B then hit the back button twice to go back to the edit page. Now your session says they are editing B but they are trying to load a page from A from the phpcr which is going to throw that IdPrefixListener exception.

@dbu
Copy link
Member

dbu commented Dec 16, 2013

ah, i see. you have a point here yes.
sure, +1 for cmf_document_url as well. and for defining that if either path or uuid is found, that is used rather than complain if a path was specified but not found. i guess it would be path = $name and uuid a field in the options array.

local paths those make assumptions on how your routes are named. what if i want the route for about to be called about-us? i would either have a menu for such things (a secondary navigation having the about, impressum and a contact for example) - then you do not need to hardcode the route. if you need to link to about from various places, maybe link it from the "website" document and get it from there?

multiple base_paths: it seems we indeed could use them, but i think it still should only be used for paths that lie within your current site. if you tell the route provider routes can be at /routes/site-a or /routes/site-b you can't determine anymore if you need absolute urls or not. and even worse, you would serve urls from site-a under domain site-b which is not a good thing. so i think what we should have:

  • a solution to generate routes outside of the current domain (a generator with a different loader that knows about all domains and comes into play if the normal router for the current domain did not succeed). i guess for this we need to make sure that trying to generate a route outside of the current domain does not throw an exception.
  • a solution for the admin to handle multiple domains. (would that not be the same again, handling route generating? the admin will never try to match frontend routes, or does it?)

@benglass
Copy link
Member Author

@dbu regarding multiple base_paths I was assuming there would be a single basepath PER website or domain. The you would be able to tell if someone asks for /routes/site-a that this is in site a and sita a is the current site and if they ask for /routes/site-b you could tell thats not in the current site and always return an absolute URL

This does assume there is a service which knows what the current website is. There would have to be a way to map a host pattern to a single basepath (even a set of basepaths would be ok). This would allow us to always know whether the requested basepath was within the current website or not and for this I think the approach of a request listener is fine. It would match the current site based on a host pattern and update a service.

Our service is called WebsiteContext and it just has a setWebsite and getWebsite method.

If we wanted to generalize this could be a HostAwareBasepathProvider service which would perhaps have methods like all and getByHost.

@dbu
Copy link
Member

dbu commented Dec 17, 2013

not sure if i should reply here or in the symfony-cmf issue you opened... but i think having a router that looks at routes from other domains for route matching would be wrong. and building all the logic into the route generator we have seems to not well handle separation of concerns. imho we should create a separate route generator that knows about the multisite stuff and what is the current site and handle that - see my comment in symfony-cmf/symfony-cmf#182

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

No branches or pull requests

4 participants