Problem/Motivation

Port:

  • CDN_SEO_* constants in cdn.constants.inc
  • cdn_boot() in cdn.module
  • _cdn_seo_should_redirect() in cdn.module
  • cdn_admin_other_settings_form() in cdn.admin.inc
  • cdn_ui/help/admin-other-seo.html
  • CDNSEOTestCase in tests/cdn.test

Proposed resolution

Should be implemented as a Stack middleware, because hook_boot() no longer exists in Drupal 8

Eventually should be published on http://stackphp.com/, because this is not in any way Drupal-specific. (It's basically redirecting all user agents except the whitelisted ones to the canonical domain for all HTML content.)

Remaining tasks

User interface changes

API changes

Data model changes

Yes.

CommentFileSizeAuthor
#19 interdiff.txt736 bytesWim Leers
#19 cdn_seo_port-2708771-19.patch32.06 KBWim Leers
#16 interdiff.txt2.25 KBWim Leers
#16 cdn_seo_port-2708771-15.patch32.06 KBWim Leers
#14 interdiff.txt1.04 KBWim Leers
#14 cdn_seo_port-2708771-14.patch32.29 KBWim Leers
#13 interdiff.txt6.54 KBWim Leers
#13 cdn_seo_port-2708771-13.patch31.99 KBWim Leers
#12 interdiff.txt4.77 KBWim Leers
#12 cdn_seo_port-2708771-12.patch27.29 KBWim Leers
#9 cdn_seo_port-2708771-7.patch23.7 KBWim Leers
#7 interdiff.txt3.15 KBWim Leers
#7 cdn_seo_port-2708771-7.patch23.7 KBWim Leers
#6 cdn_seo_port-2708771-6.patch26.63 KBWim Leers
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers created an issue. See original summary.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers as a volunteer commented
Issue summary: View changes
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers as a volunteer commented
Related issues: +#2294613: Port CDN to Drupal 8
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers as a volunteer commented
Issue summary: View changes
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers as a volunteer commented
Issue summary: View changes
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers as a volunteer commented
Assigned: Unassigned » Wim Leers
Issue summary: View changes
Status: Active » Needs review
FileSize
cdn_seo_port-2708771-6.patch26.63 KB

Port is mostly done.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers as a volunteer commented
FileSize
cdn_seo_port-2708771-7.patch23.7 KB
interdiff.txt3.15 KB

Reverting some out of scope changes.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers as a volunteer commented

I made one big mistake in this patch: this middleware runs always, even when the CDN module's status is set to disabled. We'd need it to work only when it's set to enabled… but doing so will make the middleware significantly slower, and hence also make Page Cache significantly slower. Because it'll mean initializing the config system and everything it depends on very early in the request.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers as a volunteer commented
FileSize
cdn_seo_port-2708771-7.patch23.7 KB

Not sure yet what to do, but reuploading #7 in the mean time so it is tested. I fixed the automated testing settings for this project.

Status: Needs review » Needs work

The last submitted patch, 9: cdn_seo_port-2708771-7.patch, failed testing.

The last submitted patch, 9: cdn_seo_port-2708771-7.patch, failed testing.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers as a volunteer commented
Issue summary: View changes
Status: Needs work » Needs review
FileSize
cdn_seo_port-2708771-12.patch27.29 KB
interdiff.txt4.77 KB
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers as a volunteer commented
Issue summary: View changes
Related issues: +#2424309: Expose more language container parameters, to allow services to be language-aware efficiently
FileSize
cdn_seo_port-2708771-13.patch31.99 KB
interdiff.txt6.54 KB

The problem described in #8 reminded me of #2424309: Expose more language container parameters, to allow services to be language-aware efficiently. I posted a new comment there, at #15:

This is basically choosing between:

  1. small cost on every request: instantiating the configuration factory and all of its dependencies on every request
  2. very big cost upon changing configuration: container rebuild

That's the trade-off we must make.

I face a similar choice over at #2708771-8: Port SEO (duplicate content prevention) to 8.x-3.x.

I think the conclusion is clear. You change that configuration rarely, so you want the common case to much faster. So we rebuild the container when necessary.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers as a volunteer commented
FileSize
cdn_seo_port-2708771-14.patch32.29 KB
interdiff.txt1.04 KB

Clean-up.

The last submitted patch, 13: cdn_seo_port-2708771-13.patch, failed testing.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers as a volunteer commented
Status: Needs review » Reviewed & tested by the community
FileSize
cdn_seo_port-2708771-15.patch32.06 KB
interdiff.txt2.25 KB

Nits.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers as a volunteer commented
Related issues: +#2678374: Document that SEO (duplicate content prevention) causes redirect loop in combination with reverse proxy between CDN and web server

Note that this already incorporated #2678374-16: Document that SEO (duplicate content prevention) causes redirect loop in combination with reverse proxy between CDN and web server.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers as a volunteer commented
Status: Reviewed & tested by the community » Needs work

Actually, all of the green patches above… they're not running my new unit test yet :/

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers as a volunteer commented
Status: Needs work » Needs review
FileSize
cdn_seo_port-2708771-19.patch32.06 KB
interdiff.txt736 bytes

Hah! Detail.

run-tests.sh is so frustrating…

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers as a volunteer commented
Status: Needs review » Reviewed & tested by the community

52 passes instead of 38! That's more like it :)

  • Wim Leers committed fec159b on 8.x-3.x 
    Issue #2708771 by Wim Leers: Port SEO (duplicate content prevention) to...
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers as a volunteer commented
Issue summary: View changes
Status: Reviewed & tested by the community » Fixed

Yay!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.