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

[CMSP-1459] Fixes URL normalization process #138

Merged
merged 40 commits into from
Jul 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
40f589c
add new helper function that parses and normalizes a url from parts
jazzsequence Jul 8, 2024
bd90d48
Update URL normalization function to refactor using __rebuild_url_fro…
jazzsequence Jul 8, 2024
efc0748
check if endpoint is defined already
jazzsequence Jul 8, 2024
d59ba4b
dont' use anonymous function
jazzsequence Jul 8, 2024
1489d34
add host if exists
jazzsequence Jul 8, 2024
5836606
replace graphql endpoint filter with an action hook to prepopulate
jazzsequence Jul 8, 2024
e477b51
move the prepopulate action and make it more robust
jazzsequence Jul 9, 2024
2607a16
add exclusions to site_url filter for graphql
jazzsequence Jul 9, 2024
0b5cda2
add playwright tests
jazzsequence Jul 9, 2024
0169f4b
change ci to install
jazzsequence Jul 9, 2024
9ea9a60
don't create the site if it already exists
jazzsequence Jul 9, 2024
468f5c1
add git config for the user
jazzsequence Jul 9, 2024
218da98
wait for the last action to finish before switching to sftp mode
jazzsequence Jul 9, 2024
c0bc9ea
set pretty permalinks for playwright test site
jazzsequence Jul 9, 2024
d1c0406
switch the existing site to git mode
jazzsequence Jul 9, 2024
f9a1180
force yes
jazzsequence Jul 9, 2024
25d1e44
check the h2 instead of the site title
jazzsequence Jul 9, 2024
d12ab4b
test that the resource urls have /wp/ in the path
jazzsequence Jul 9, 2024
707e2c9
missing "
jazzsequence Jul 9, 2024
9093ef7
expect the resource url to be truthy on a request
jazzsequence Jul 9, 2024
1f611f9
append the commit message from the PR to the commit to the test site
jazzsequence Jul 9, 2024
b702020
use github token so we can use gh
jazzsequence Jul 9, 2024
6aeb8c9
add package.json to export-ignore
jazzsequence Jul 9, 2024
dfef2ce
move the commit msg var up
jazzsequence Jul 9, 2024
312ba79
we don't need `page` at all
jazzsequence Jul 9, 2024
69e379c
this doesn't change anything other than seeing if we're getting the P…
jazzsequence Jul 9, 2024
d138206
fix the commit message
jazzsequence Jul 9, 2024
a9a1d10
Include the git commit message body
jazzsequence Jul 9, 2024
2aab87b
don't fail if there's nothing to commit
jazzsequence Jul 9, 2024
54dbbf9
add caching for dependencies
jazzsequence Jul 9, 2024
75292b8
fix hello world test
jazzsequence Jul 9, 2024
dec93c0
emdash problems
jazzsequence Jul 9, 2024
3c94b14
fix the locator on the welcome message test
jazzsequence Jul 9, 2024
1cc61ff
generate lock files so we can use the cache
jazzsequence Jul 9, 2024
4e9a2ff
pass default values
jazzsequence Jul 9, 2024
1db29d4
change test to toContainText
jazzsequence Jul 9, 2024
1d11125
only generate the npm lock
jazzsequence Jul 9, 2024
76ad38c
party pooper 💩
jazzsequence Jul 10, 2024
364083a
bail early
jazzsequence Jul 10, 2024
55c836b
Merge branch 'bugfix-fix-core-resource-url-handling' of github.com:pa…
jazzsequence Jul 10, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
/.editorconfig export-ignore
/.gitattributes export-ignore
/.github export-ignore
/package.json export-ignore
65 changes: 65 additions & 0 deletions .github/tests/wpcm.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { test, expect } from "@playwright/test";

const exampleArticle = "Hello world!";
const siteTitle = "WPCM Playwright Tests";
const siteUrl = process.env.SITE_URL || "https://dev-wpcm-playwright-tests.pantheonsite.io";

test("homepage loads and contains example content", async ({ page }) => {
await page.goto(siteUrl);
await expect(page).toHaveTitle(siteTitle);
await expect(page.getByText(exampleArticle)).toHaveText(exampleArticle);
});

test("WP REST API is accessible", async ({ request }) => {
const apiRoot = await request.get(`${siteUrl}/wp-json`);
expect(apiRoot.ok()).toBeTruthy();
});

test("Hello World post is accessible", async ({ page }) => {
await page.goto(`${siteUrl}/hello-world/'`);
await expect(page).toHaveTitle(`${exampleArticle}${siteTitle}`);
// Locate the element containing the desired text
const welcomeText = page.locator('text=Welcome to WordPress');
await expect(welcomeText).toContainText('Welcome to WordPress');
});

test("validate core resource URLs", async ({ request }) => {
const coreResources = [
'wp-includes/js/dist/interactivity.min.js',
'wp-includes/css/dist/editor.min.css',
];

for ( const resource of coreResources ) {
const resourceUrl = `${siteUrl}/wp/${resource}`;
const response = await request.get(resourceUrl);
await expect(response).toBeTruthy();
}
});

test("graphql is able to access hello world post", async ({ request }) => {
const query = `
query {
posts(where: { search: "${exampleArticle}" }) {
edges {
node {
title
}
}
}
}
`;

const response = await request.post(`${siteUrl}/wp/graphql`, {
data: {
query: query
},
headers: {
'Content-Type': 'application/json'
}
});

const responseBody = await response.json();

expect(responseBody.data.posts.edges.length).toBeGreaterThan(0);
expect(responseBody.data.posts.edges[0].node.title).toBe(exampleArticle);
});
113 changes: 113 additions & 0 deletions .github/workflows/playwright.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
name: WordPress (Composer Managed) Playwright Tests
on:
pull_request:
types:
- opened
- reopened
- synchronize
- ready_for_review

permissions:
contents: write

jobs:
playwright:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v4

- name: Generate lock files
run: |
npm install --package-lock-only
- name: Set up cache for dependencies
uses: actions/cache@v4
id: cache
with:
path: |
~/.composer/cache
./vendor
~/.npm
./node_modules
key: ${{ runner.os }}-deps-${{ hashFiles( '**/composer.json', '**/package-lock.json' ) }}
restore-keys: ${{ runner.os }}-deps-

- name: Install Composer dependencies
if: steps.cache.outputs.cache-hit != true
run: composer update --no-progress --prefer-dist --optimize-autoloader

- name: Install NPM dependencies
if: steps.cache.outputs.cache-hit != true
run: npm ci

- name: Install Playwright Browsers
run: npx playwright install --with-deps

- name: Install SSH keys
uses: webfactory/[email protected]
with:
ssh-private-key: ${{ secrets.SSH_PRIVATE_KEY }}

- name: Get latest Terminus release
uses: pantheon-systems/terminus-github-actions@v1
with:
pantheon-machine-token: ${{ secrets.TERMINUS_TOKEN }}
- name: Validate Pantheon Host Key
run: |
echo "Host *.drush.in HostKeyAlgorithms +ssh-rsa" >> ~/.ssh/config
echo "Host *.drush.in PubkeyAcceptedKeyTypes +ssh-rsa" >> ~/.ssh/config
echo "StrictHostKeyChecking no" >> ~/.ssh/config
- name: Log into Terminus
run: |
terminus auth:login --machine-token=${{ secrets.TERMINUS_TOKEN }}
- name: Create Site
run: |
if terminus site:info wpcm-playwright-tests; then
echo "Test site already exists, skipping site creation."
# If the site exists already, we should switch it to git mode.
terminus connection:set wpcm-playwright-tests.dev git -y
else
terminus site:create wpcm-playwright-tests 'WordPress (Composer Managed) Playwright Test Site' 'WordPress (Composer Managed)' --org=5ae1fa30-8cc4-4894-8ca9-d50628dcba17
fi
Comment on lines +67 to +73
Copy link
Member

Choose a reason for hiding this comment

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

site name and org UUID can/should remain config'd in this file, but can they be abstracted to global variables to ease identification (and replacement)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I was just avoiding it to avoid complexity. We can come back to this to refine it later, I was more concerned that we had tests.

- name: Clone the site locally and copy PR updates
env:
GH_TOKEN: ${{ github.token }}
run: |
git config --global user.email "[email protected]"
git config --global user.name "Pantheon WPCM Bot"
PR_NUMBER=$(echo ${{ github.event.pull_request.number }})
echo "Pull Request Number: ${PR_NUMBER}"
COMMIT_MSG=$(gh pr view ${PR_NUMBER} --json commits --jq '.commits[-1] | "\(.messageHeadline) \(.messageBody)"')
echo "Commit Message: ${COMMIT_MSG}"
terminus local:clone wpcm-playwright-tests
cd ~/pantheon-local-copies/wpcm-playwright-tests
rsync -a --exclude='.git' ${{ github.workspace }}/ .
git add -A
git commit -m "Update to latest commit: ${COMMIT_MSG}" || true
git push origin master || true
- name: Status Check
run: terminus wp wpcm-playwright-tests.dev -- cli info

- name: Install WordPress
run: |
terminus wp wpcm-playwright-tests.dev -- core install --title='WPCM Playwright Tests' --admin_user=wpcm [email protected]
terminus wp wpcm-playwright-tests.dev -- option update permalink_structure '/%postname%/'
terminus wp wpcm-playwright-tests.dev -- rewrite flush
terminus wp wpcm-playwright-tests.dev -- cache flush
- name: Install WP GraphQL
run: |
terminus workflow:wait wpcm-playwright-tests.dev
terminus connection:set wpcm-playwright-tests.dev sftp
terminus wp wpcm-playwright-tests.dev -- plugin install --activate wp-graphql
- name: Run Playwright Tests
run: npm run test .github/tests/wpcm.spec.ts

- name: Delete Site
if: success()
run: terminus site:delete wpcm-playwright-tests -y
12 changes: 12 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"name": "@pantheon-systems/wordpress-composer-managed",
"version": "1.31.0",
"description": "Automated testing for WordPress (Composer Managed).",
"scripts": {
"test": "playwright test --trace on",
"report": "playwright show-report"
},
"devDependencies": {
"@playwright/test": "^1.28.0"
}
}
6 changes: 3 additions & 3 deletions upstream-configuration/scripts/ComposerScripts.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ public static function applyComposerJsonUpdates(Event $event)
// is the same as the Pantheon PHP version (which is only major.minor).
// If they do not match, force an update to the platform PHP version. If they
// have the same major.minor version, then
$platformPhpVersion = static::getCurrentPlatformPhp($event);
$pantheonPhpVersion = static::getPantheonPhpVersion($event);
$updatedPlatformPhpVersion = static::bestPhpPatchVersion($pantheonPhpVersion);
$platformPhpVersion = static::getCurrentPlatformPhp($event) ?? '';
$pantheonPhpVersion = static::getPantheonPhpVersion($event) ?? '';
$updatedPlatformPhpVersion = static::bestPhpPatchVersion($pantheonPhpVersion) ?? '';
if ((substr($platformPhpVersion, 0, strlen($pantheonPhpVersion)) != $pantheonPhpVersion) && !empty($updatedPlatformPhpVersion)) {
$io->write("<info>Setting platform.php from '$platformPhpVersion' to '$updatedPlatformPhpVersion' to conform to pantheon php version.</info>");
$composerJson['config']['platform']['php'] = $updatedPlatformPhpVersion;
Expand Down
67 changes: 57 additions & 10 deletions web/app/mu-plugins/filters.php
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,28 @@ function fix_core_resource_urls( string $url ) : string {
}
}

/**
* Prepopulate GraphQL endpoint URL with default value if unset.
* This will ensure that the URL is not changed from /wp/graphql to /graphql by our other filtering unless that's what the user wants.
*
* @since 1.1.0
*/
function prepopulate_graphql_endpoint_url() {
$options = get_option( 'graphql_general_settings' );

// Bail early if options have already been set.
if ( $options ) {
return;
}

$options = [];
$site_path = site_url();
$endpoint = ( ! empty( $site_path ) || strpos( $site_path, 'wp' ) !== false ) ? 'graphql' : 'wp/graphql';
$options['graphql_endpoint'] = $endpoint;
update_option( 'graphql_general_settings', $options );
}
add_action( 'graphql_init', __NAMESPACE__ . '\\prepopulate_graphql_endpoint_url' );

/**
* Drop the /wp, if it exists, from URLs on the main site (single site or multisite).
*
Expand All @@ -106,6 +128,15 @@ function fix_core_resource_urls( string $url ) : string {
* @return string The filtered URL.
*/
function adjust_main_site_urls( string $url ) : string {
if ( doing_action( 'graphql_init' ) ) {
return $url;
}

// Explicit handling for /wp/graphql
if ( strpos( $url, '/graphql' ) !== false ) {
return $url;
}

// If this is the main site, drop the /wp.
if ( is_main_site() && ! __is_login_url( $url ) ) {
$url = str_replace( '/wp/', '/', $url );
Expand Down Expand Up @@ -157,11 +188,11 @@ function __is_login_url( string $url ) : bool {
}

// Check if the URL is a login or admin page
if (strpos($url, 'wp-login') !== false || strpos($url, 'wp-admin') !== false) {
if ( strpos( $url, 'wp-login' ) !== false || strpos($url, 'wp-admin' ) !== false) {
return true;
}

return false
return false;
}

/**
Expand All @@ -172,15 +203,31 @@ function __is_login_url( string $url ) : bool {
* @return string The normalized URL.
*/
function __normalize_wp_url( string $url ): string {
$scheme = parse_url( $url, PHP_URL_SCHEME );
$scheme_with_separator = $scheme ? $scheme . '://' : '';
// Parse the URL into components.
$parts = parse_url( $url );

// Remove the scheme from the URL if it exists.
$remaining_url = $scheme ? substr( $url, strlen($scheme_with_separator ) ) : $url;
// Normalize the URL to remove any double slashes.
if ( isset( $parts['path'] ) ) {
$parts['path'] = preg_replace( '#/+#', '/', $parts['path'] );
}

// Normalize the remaining URL to remove any double slashes.
$normalized_url = str_replace( '//', '/', $remaining_url );
// Rebuild and return the full normalized URL.
return __rebuild_url_from_parts( $parts );
}
Comment on lines +209 to +216
Copy link
Member

Choose a reason for hiding this comment

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

seems over-engineered with the same result but maybe I'm missing what's changed and how the bugs manifested.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not entirely convinced it's necessary but I'm also not convinced that it's not. It was one of the many steps on the path of resolving the bug(s).

Copy link
Member

@pwtyler pwtyler Jul 10, 2024

Choose a reason for hiding this comment

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

we need tests (I know you know this)


// Reconstruct and return the full normalized URL.
return $scheme_with_separator . $normalized_url;
/**
* Rebuild parsed URL from parts.
*
* @since 1.1.0
* @param array $parts URL parts from parse_url.
* @return string Re-parsed URL.
*/
function __rebuild_url_from_parts( array $parts ) : string {
Copy link
Member

Choose a reason for hiding this comment

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

feels very dirty that this doesn't exist already 😬

return trailingslashit(
( isset( $parts['scheme'] ) ? "{$parts['scheme']}:" : '' ) .
( isset( $parts['host'] ) ? "{$parts['host']}" : '' ) .
( isset( $parts['path'] ) ? untrailingslashit( "{$parts['path']}" ) : '' ) .
( isset( $parts['query'] ) ? str_replace( '/', '', "?{$parts['query']}" ) : '' ) .
( isset( $parts['fragment'] ) ? str_replace( '/', '', "#{$parts['fragment']}" ) : '' )
);
}
Loading