PiranhaCMS / piranha.core

Piranha CMS is the friendly editor-focused CMS for .NET that can be used both as an integrated CMS or as a headless API.
http://piranhacms.org
MIT License
1.99k stars 555 forks source link

New workflow for sites #461

Open steelspace opened 5 years ago

steelspace commented 5 years ago

With version 5.31 the generated links are fine for multi-site scenario (same domain name but different language): test.com/en/abc test.com/de/cde

Anyway, 404 is returned when accessing these links. Previous version generated links without proper host url part but route was resolved properly.

tidyui commented 5 years ago

Hi there! You will have to explain further as I don’t understand what you mean. The only thing that changed in 5.3.1 was how sites are resolved from hostname to prevent routing errors.

https://github.com/PiranhaCMS/piranha.core/commit/644288d18f98664360610ba0da4e6e2c6c1c93fd

These generated links that behave differently, how and where are they generated?

Best regards

steelspace commented 5 years ago

Hello, the problem is exactly the following. Create two sites with the same domain name but different part after slash, such as: Test.com/en (default) Test.com/de

Create 2 pages test1 and test2 on site 1.

All links are correct: /en/test1 /en/test2

But clicking n those links returns 404.

Thank you very much. Petr

tidyui commented 5 years ago

I have now tested this locally on the example project.

Default site

I've left the hostnames field of the default site empty, ie exactly like it was so it handles all incoming requests that are not intercepted by any other sites.

Swedish site

I've created a Swedish site and added localhost/sv into the hostnames field (see screenshot below)

skarmavbild 2018-12-11 kl 19 20 46

Result

GET /

Returns the start page of the default site as expected.

GET /sv

Returns the start page of the swedish site as expected.

GET /sv/testar

Returns a second page from the swedish site as expected.

Issues

The partial view rendering the menu in the templates does not take into account the /sv part of the hostname when generating the URL's, so the links in the menu are not rendered correctly.

Solution

One could argue that the sitemap could include both a relative and absolute url that you can choose from when rendering the menu. You can however handle this in the menu view right now by injecting an IApplicationService into it which has the current Site id and then get that Site. Given that you're using caching the site will be cached in memory and not generate a roundtrip to the database.

tidyui commented 5 years ago

I can however see that there's a big issue with internal linking when setting up the sites like this. As long as you use different domains everything will work fine, but when you start using suffixes, i.e site/de, site/en there will be issues.

When linking to an internal page/post/image/video/document in the HTML-editor the relative url (slug) will be stored in the database. This means the link will be rendered incorrectly as it needs a /en or /de to function correctly.

We really don't want to store the absolute URL in the database as this means running in a test environment will link to a resource on another domain, the same goes for suffixes. This means we would need to store something that's not a real link, like <a href="@{page:id}">...</a> and this means it has to be translated in runtime. There really aren't any good tools in place for at the moment and I think we need to specify exactly what is needed for all cases and make a real effort to make it work really well.

Regards

tidyui commented 5 years ago

The routing however works as expected, I'm closing this issue and we should open a separate issue for each issue that needs to be resolved to provide good multi-site support from the web application point-of-view.

steelspace commented 5 years ago

This is strange - I created new project, updated packages to 5.3.1, added new site as you did (even Swedish :), but routing doesn't work. When I have some time, I'll get the source and debug it.

tidyui commented 5 years ago

Hi again. There's so many things that just won't function properly setting up multiple sites and routing to the same domain with suffixes (mysite.com, mysite.com/de). The current implementation was taken from the legacy version of Piranha, unfortunately this version was very limited in its functionality so the faults we're experiencing now weren't obvious.

Solution

Instead we're proposing a different take on what you want to achieve, and I've built a working prototype on it. Instead of creating different sites for the same domain you create sub sites in one Site Tree, much like you would do in Umbraco for example. In the prototype it looks like this:

skarmavbild 2018-12-13 kl 09 49 22

To make this work I've created a module which

This means that when rendering the page, the sitemap only reflects the current portion of the site, see images below.

skarmavbild 2018-12-13 kl 09 49 39

skarmavbild 2018-12-13 kl 09 50 10

Limitations in the prototype

Since this is only a add-on to the existing version with no extra support in the core libraries there are some limitations.

Probably more details not noted here.

Going forward

If this feels like a good solution to this use-case we propose that we make a more proper implementation of this in the core packages where you can add more special handling for sub sites and not just handle them like regular pages.

I can also push the code to a feature branch so you can try setting up your content using this model to see how it feels.

Best regards

steelspace commented 5 years ago

Thank you. I am fine with this solution since it fully covers my use-case. If you don't mind, create the feature branch so I can check the middleware code and see how it works all together.

tidyui commented 5 years ago

Getting the code

The branch is available here:

https://github.com/PiranhaCMS/piranha.core/tree/features/subsites

You can just copy the project for the module to your solution. The module is located in ~/modules/Piranha.SubSites.

https://github.com/PiranhaCMS/piranha.core/tree/features/subsites/modules/Piranha.SubSites

Setting up the application

Please take note of the following changes you need to make in your Startup.cs.

Adding the Sub Site page type

var pageTypeBuilder = new Piranha.AttributeBuilder.PageTypeBuilder(api)
    ...
    .AddType(typeof(Piranha.SubSites.Models.SubSitePage));
pageTypeBuilder.Build()

Adding the middleware

You have to remove the line app.UsePiranha() as the middleware needs to be inserted at the right position in the application pipeline. Instead, manually add all middleware and the new one like so:

app.UseMiddleware<Piranha.AspNetCore.ApplicationMiddleware>()
    .UseMiddleware<Piranha.AspNetCore.AliasMiddleware>()
    .UseMiddleware<Piranha.SubSites.SubSiteMiddleware>()
    .UseMiddleware<Piranha.AspNetCore.PageMiddleware>()
    .UseMiddleware<Piranha.AspNetCore.PostMiddleware>()
    .UseMiddleware<Piranha.AspNetCore.ArchiveMiddleware>()
    .UseMiddleware<Piranha.AspNetCore.StartPageMiddleware>()
    .UseMiddleware<Piranha.AspNetCore.SitemapMiddleware>();

Best regards

tidyui commented 5 years ago

Remember that the slug of the Sub Site pages will be the "prefix" for your sub site. In my example the Swedish site had the slug se and the German de.

Also, if you don't specify a start page the middleware will choose the first child page as the start page of the sub site.

w0ns88 commented 5 years ago

Hi @tidyui, any news on this task for milestone 6.0?

tidyui commented 5 years ago

None other than that we believe the above proposed solution is the best way to go! The rewrite and accessibility updates for the manager interface has taken more time than we estimated so it unlikely that all of the issues in the 6.0 milestone will be finished in time, either we postpone the entire release or we will split it up into a number or releases (6.0, 6.1). However our goal is still to get the entire set of features done in Q1 2019

tidyui commented 5 years ago

Actually, since this functionality will more or less make the current Site concept redundant, I’m looking into the possibility to rename the current “Sites” to “Apps” and in the future elaborate more on this. For example you could specify which Content Types would be available for an app and so one. You could even have app-specific media files.

w0ns88 commented 5 years ago

Thanks for your answers @tidyui.

Just to comment on the original issue from @steelspace of getting 404 errors when adding sites and hostnames. The following are my findings using nuget packages version 5.3.x.

I have a seed method that creates three sites (danish, swedish and norwegian) in addition to the default site. I set the following hostname for the respective sites: localhost/dk, localhost/se and localhost/no. In the same seed method, I add a startpage (/home) and a blog archive (/blog) for each site.

When I run the program and seed for the FIRST time and move to the following url: https://localhost:5000/dk/home (or any other url from my three sites) I get a 404. BUT if I run the project, seed, stop it, re-run it and navigate to https://localhost:5000/dk/home (or any other url from my three sites) I get the desired data.

It is a very strange behavior that I have to stop my application and re-run it in order to get my data at the desired urls, but it is the only way I have made it work. I have tested this many times and this is the only way to get urls working on a multi-site scenario.

Would there be any reason for such behavior? Maybe this sheds a little light on the underlying issue or at least I hope so.

tidyui commented 5 years ago

Thanks for your input @w0ns88! There are a lot of things that are cached as they are needed for every request, and retrieving them from the database each time would be a big performance impact, hostname mappings are one of those things. The cache is purged and reloaded per entity when something is updated or deleted but it sounds like there's a bug in this behavior for the site/hostname mappings.

This would explain why it's not working until you restart, the cache is simply holding stale data for the sites.

mkarlsson commented 5 years ago

Hi @tidyui, I'm building a site with English, Spanish and Swedish. I tried setting Culture and hostname in the sites tab on localhost and it worked. However, on Azure websites it didn't work. 404... Reading this it looks like you are taking another turn on this anyway? What do you think about this solution. Create a start page with a language selector and add my real start pages below that? image

mkarlsson commented 5 years ago

And I'm not sure how to build the menus based on this solution.. I can see some pages have culture attributes when fetching from db but not really sure how culture is set?

tidyui commented 5 years ago

This issue is related to #806. Unfortunately this is a breaking change and has to be postponed to the next major version