Skip to content

Commit 395cbb4

Browse files
westonruterwestonruter
committed
Remove transitional XPath
1 parent b7c4475 commit 395cbb4

File tree

6 files changed

+22
-183
lines changed

6 files changed

+22
-183
lines changed

‎plugins/optimization-detective/class-od-element.php‎

Lines changed: 1 addition & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,6 @@ class OD_Element implements ArrayAccess, JsonSerializable {
3333
*/
3434
protected $data;
3535

36-
/**
37-
* Transitional XPath.
38-
*
39-
* @since n.e.x.t
40-
* @todo Remove logic related to transitional_xpath in a subsequent release once URL Metrics have been collected with the new format.
41-
* @var non-empty-string|null
42-
*/
43-
protected $transitional_xpath = null;
44-
4536
/**
4637
* URL Metric that this element belongs to.
4738
*
@@ -99,9 +90,6 @@ public function get_url_metric_group(): ?OD_URL_Metric_Group {
9990
* @return mixed|null The property value, or null if not set.
10091
*/
10192
public function get( string $key ) {
102-
if ( 'xpath' === $key ) {
103-
return $this->get_xpath();
104-
}
10593
return $this->data[ $key ] ?? null;
10694
}
10795

@@ -132,58 +120,11 @@ public function is_lcp_candidate(): bool {
132120
*
133121
* @since 0.7.0
134122
* @since n.e.x.t Returns the transitional XPath format. To access the underlying raw XPath, access the 'xpath' key of the jsonSerialize response.
135-
* @todo Remove logic related to transitional_xpath in a subsequent release once URL Metrics have been collected with the new format.
136123
*
137124
* @return non-empty-string XPath.
138125
*/
139126
public function get_xpath(): string {
140-
141-
if ( ! isset( $this->transitional_xpath ) ) {
142-
$replacements = array(
143-
144-
/*
145-
* Convert the original XPath format for elements in the BODY.
146-
*
147-
* Example:
148-
* /*[1][self::HTML]/*[2][self::BODY]/*[1][self::DIV]/*[1][self::IMG]
149-
* =>
150-
* /HTML/BODY/DIV/*[1][self::IMG]
151-
*/
152-
'#^/\*\[1]\[self::HTML]/\*\[2]\[self::BODY]/\*\[\d+]\[self::([a-zA-Z0-9:_-]+)]#' => '/HTML/BODY/$1',
153-
154-
/*
155-
* Convert the original XPath format for elements in the HEAD.
156-
*
157-
* Example:
158-
* /*[1][self::HTML]/*[1][self::HEAD]/*[1][self::META]
159-
* =>
160-
* /HTML/HEAD/*[1][self::META]
161-
*/
162-
'#^/\*\[1\]\[self::HTML\]/\*\[1\]\[self::HEAD]#' => '/HTML/HEAD',
163-
164-
/*
165-
* Convert the new XPath format for elements in the BODY.
166-
*
167-
* Note that the new XPath format for elements in the HEAD does not need to be converted to the
168-
* transitional format since disambiguating attributes are not used in the HEAD.
169-
*
170-
* Example:
171-
* /HTML/BODY/DIV[@id='page']/*[1][self::IMG]
172-
* =>
173-
* /HTML/BODY/DIV/*[1][self::IMG]
174-
*/
175-
'#^(/HTML/BODY/\w+)\[@[^\]]+?]#' => '$1',
176-
);
177-
foreach ( $replacements as $search => $replace ) {
178-
$xpath = preg_replace( $search, $replace, $this->data['xpath'], -1, $count );
179-
if ( $count > 0 ) {
180-
$this->transitional_xpath = $xpath;
181-
break;
182-
}
183-
}
184-
}
185-
186-
return $this->transitional_xpath ?? $this->data['xpath'];
127+
return $this->data['xpath'];
187128
}
188129

189130
/**
@@ -250,9 +191,6 @@ public function offsetExists( $offset ): bool {
250191
*/
251192
#[ReturnTypeWillChange]
252193
public function offsetGet( $offset ) {
253-
if ( 'xpath' === $offset ) {
254-
return $this->get_xpath();
255-
}
256194
return $this->data[ $offset ] ?? null;
257195
}
258196

‎plugins/optimization-detective/class-od-html-tag-processor.php‎

Lines changed: 16 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -185,19 +185,6 @@ final class OD_HTML_Tag_Processor extends WP_HTML_Tag_Processor {
185185
*/
186186
private $bookmarked_open_stacks = array();
187187

188-
/**
189-
* Stored XPath for the current tag.
190-
*
191-
* This is used so that repeated calls to {@see self::get_stored_xpath()} won't needlessly reconstruct the string.
192-
* This gets cleared whenever {@see self::open_tags()} iterates to the next tag.
193-
*
194-
* @todo Remove this once the XPath transitional period is over.
195-
*
196-
* @since 0.4.0
197-
* @var string|null
198-
*/
199-
private $current_stored_xpath = null;
200-
201188
/**
202189
* (Transitional) XPath for the current tag.
203190
*
@@ -319,8 +306,7 @@ public function expects_closer( ?string $tag_name = null ): bool {
319306
* @return bool Whether a token was parsed.
320307
*/
321308
public function next_token(): bool {
322-
$this->current_stored_xpath = null; // Clear cache.
323-
$this->current_xpath = null; // Clear cache.
309+
$this->current_xpath = null; // Clear cache.
324310
++$this->cursor_move_count;
325311
if ( ! parent::next_token() ) {
326312
$this->open_stack_tags = array();
@@ -649,23 +635,25 @@ private function is_foreign_element(): bool {
649635
* This predicate will be included once the transitional period is over.
650636
*
651637
* @since 0.4.0
652-
* @todo Replace the logic herein with what is in get_stored_xpath() once the transitional period is over.
653638
*
654639
* @return string XPath.
655640
*/
656641
public function get_xpath(): string {
657-
/*
658-
* This transitional format is used by default for all extensions. The non-transitional format is used only in
659-
* od_optimize_template_output_buffer() when setting the data-od-xpath attribute. This is so that the new format
660-
* will replace the old format as new URL Metrics are collected. After a month of the new format being live, the
661-
* transitional format can be eliminated. See the corresponding logic in OD_Element for normalizing both the
662-
* old and new XPath formats to use the transitional format.
663-
*/
664642
if ( null === $this->current_xpath ) {
665643
$this->current_xpath = '';
666644
foreach ( $this->get_indexed_breadcrumbs() as $i => list( $tag_name, $index, $attributes ) ) {
667-
if ( $i < 2 || ( 2 === $i && '/HTML/BODY' === $this->current_xpath ) ) {
645+
if ( $i < 2 ) {
668646
$this->current_xpath .= "/$tag_name";
647+
} elseif ( 2 === $i && '/HTML/BODY' === $this->current_xpath ) {
648+
$segment = "/$tag_name";
649+
foreach ( $attributes as $attribute_name => $attribute_value ) {
650+
$segment .= sprintf(
651+
"[@%s='%s']",
652+
$attribute_name,
653+
$attribute_value // Note: $attribute_value has already been validated to only contain safe characters /^[a-zA-Z0-9_.\s:-]*/ which do not need escaping.
654+
);
655+
}
656+
$this->current_xpath .= $segment;
669657
} else {
670658
$this->current_xpath .= sprintf( '/*[%d][self::%s]', $index + 1, $tag_name );
671659
}
@@ -677,39 +665,17 @@ public function get_xpath(): string {
677665
/**
678666
* Gets stored XPath for the current open tag.
679667
*
680-
* This method is temporary for a transition period while new URL Metrics are collected for active installs. Once
681-
* the transition period is over, the logic in this method can be moved to {@see self::get_xpath()} and this method
682-
* can simply be an alias for that one. See related logic in {@see OD_Element::get_xpath()}. This function is only
683-
* used internally by Optimization Detective in {@see od_optimize_template_output_buffer()}.
668+
* This method was temporary for a transition period while new URL Metrics are collected for active installs
684669
*
685670
* @since n.e.x.t
686-
* @todo Move the logic in this method to the get_xpath() method and let this be an alias for that method once the transitional period is over.
687671
* @access private
672+
* @deprecated
673+
* @codeCoverageIgnore
688674
*
689675
* @return string XPath.
690676
*/
691677
public function get_stored_xpath(): string {
692-
if ( null === $this->current_stored_xpath ) {
693-
$this->current_stored_xpath = '';
694-
foreach ( $this->get_indexed_breadcrumbs() as $i => list( $tag_name, $index, $attributes ) ) {
695-
if ( $i < 2 ) {
696-
$this->current_stored_xpath .= "/$tag_name";
697-
} elseif ( 2 === $i && '/HTML/BODY' === $this->current_stored_xpath ) {
698-
$segment = "/$tag_name";
699-
foreach ( $attributes as $attribute_name => $attribute_value ) {
700-
$segment .= sprintf(
701-
"[@%s='%s']",
702-
$attribute_name,
703-
$attribute_value // Note: $attribute_value has already been validated to only contain safe characters /^[a-zA-Z0-9_.\s:-]*/ which do not need escaping.
704-
);
705-
}
706-
$this->current_stored_xpath .= $segment;
707-
} else {
708-
$this->current_stored_xpath .= sprintf( '/*[%d][self::%s]', $index + 1, $tag_name );
709-
}
710-
}
711-
}
712-
return $this->current_stored_xpath;
678+
return $this->get_xpath();
713679
}
714680

715681
/**

‎plugins/optimization-detective/optimization.php‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -249,9 +249,7 @@ function od_optimize_template_output_buffer( string $buffer ): string {
249249
$processor->release_bookmark( $current_tag_bookmark );
250250

251251
if ( $tracked_in_url_metrics && $needs_detection ) {
252-
// TODO: Replace get_stored_xpath with get_xpath once the transitional period is over.
253-
$xpath = $processor->get_stored_xpath();
254-
$processor->set_meta_attribute( 'xpath', $xpath );
252+
$processor->set_meta_attribute( 'xpath', $processor->get_xpath() );
255253
}
256254
} while ( $processor->next_open_tag() );
257255

‎plugins/optimization-detective/tests/storage/test-rest-api.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,6 @@ function ( OD_URL_Metric_Store_Request_Context $context ) use ( &$stored_context
119119
$this->assertSame( $valid_params['viewport']['width'], $url_metrics[0]->get_viewport_width() );
120120
$element = $url_metrics[0]->get( 'elements' )[0];
121121
$this->assertStringStartsWith( '/HTML/BODY/DIV[@id=\'page\']/', $element->jsonSerialize()['xpath'] );
122-
$this->assertStringStartsWith( '/HTML/BODY/DIV/', $element->get_xpath() ); // TODO: Remove once the XPath transitional period is over.
123122

124123
$expected_data = $valid_params;
125124
unset( $expected_data['hmac'], $expected_data['slug'], $expected_data['current_etag'], $expected_data['cache_purge_post_id'] );

‎plugins/optimization-detective/tests/test-class-od-element.php‎

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -149,56 +149,4 @@ static function ( array $schema ): array {
149149
}
150150
$this->assertInstanceOf( Exception::class, $exception ); // @phpstan-ignore method.impossibleType (It is thrown by offsetUnset actually.)
151151
}
152-
153-
/**
154-
* Data provider.
155-
*
156-
* @return array<string, mixed>
157-
*/
158-
public function data_provider_test_transitional_get_xpath(): array {
159-
return array(
160-
'current-without-disambiguating-attr-on-body-child' => array(
161-
'xpath' => '/HTML/BODY/HEADER/*[1][self::IMG]',
162-
'expected' => '/HTML/BODY/HEADER/*[1][self::IMG]',
163-
),
164-
'current-with-disambiguating-attr-on-body-child' => array(
165-
'xpath' => '/HTML/BODY/DIV[@id=\'page\']/*[1][self::IMG]',
166-
'expected' => '/HTML/BODY/DIV/*[1][self::IMG]',
167-
),
168-
'old-format-body' => array(
169-
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::DIV]/*[1][self::IMG]',
170-
'expected' => '/HTML/BODY/DIV/*[1][self::IMG]',
171-
),
172-
'old-format-head' => array(
173-
'xpath' => '/*[1][self::HTML]/*[1][self::HEAD]/*[1][self::META]',
174-
'expected' => '/HTML/HEAD/*[1][self::META]',
175-
),
176-
);
177-
}
178-
179-
/**
180-
* Test that get_xpath() converts to the transitional format.
181-
*
182-
* @dataProvider data_provider_test_transitional_get_xpath
183-
* @covers ::get_xpath
184-
*/
185-
public function test_transitional_get_xpath( string $xpath, string $expected ): void {
186-
187-
$element_data = array(
188-
'xpath' => $xpath,
189-
'isLCP' => false,
190-
'isLCPCandidate' => true,
191-
'intersectionRatio' => 0.123,
192-
'intersectionRect' => $this->get_sample_dom_rect(),
193-
'boundingClientRect' => $this->get_sample_dom_rect(),
194-
);
195-
196-
$url_metric = $this->get_sample_url_metric( array( 'element' => $element_data ) );
197-
$element = $url_metric->get_elements()[0];
198-
199-
$this->assertSame( $expected, $element->get_xpath() );
200-
$this->assertSame( $expected, $element->get( 'xpath' ) );
201-
$this->assertSame( $expected, $element['xpath'] );
202-
$this->assertSame( $xpath, $url_metric->jsonSerialize()['elements'][0]['xpath'] );
203-
}
204152
}

‎plugins/optimization-detective/tests/test-class-od-html-tag-processor.php‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -439,26 +439,16 @@ public function data_provider_sample_documents(): array {
439439
*/
440440
public function test_next_tag_and_get_xpath( string $document, array $open_tags, array $xpath_breadcrumbs ): void {
441441
$p = new OD_HTML_Tag_Processor( $document );
442-
$this->assertSame( '', $p->get_stored_xpath(), 'Expected empty XPath since iteration has not started.' );
442+
$this->assertSame( '', $p->get_xpath(), 'Expected empty XPath since iteration has not started.' );
443443
$actual_open_tags = array();
444444
$actual_xpath_breadcrumbs_mapping = array();
445445
while ( $p->next_open_tag() ) {
446446
$actual_open_tags[] = $p->get_tag();
447447

448-
$xpath = $p->get_stored_xpath();
448+
$xpath = $p->get_xpath();
449449
$this->assertArrayNotHasKey( $xpath, $actual_xpath_breadcrumbs_mapping, 'Each tag must have a unique XPath.' );
450450

451451
$actual_xpath_breadcrumbs_mapping[ $xpath ] = $p->get_breadcrumbs();
452-
453-
$transitional_xpath = $p->get_xpath();
454-
$this->assertRegExp(
455-
'#^/HTML(
456-
/HEAD(/\*\[\d+]\[self::\w+])?
457-
|
458-
/BODY(/DIV(/\*\[\d+]\[self::\w+])*)?
459-
)?$#x',
460-
$transitional_xpath
461-
);
462452
}
463453

464454
$this->assertSame( $open_tags, $actual_open_tags, "Expected list of open tags to match.\nSnapshot: " . $this->export_array_snapshot( $actual_open_tags, true ) );
@@ -703,7 +693,7 @@ public function test_bookmarking_and_seeking(): void {
703693
$bookmarks[] = $bookmark;
704694
$actual_figure_contents[] = array(
705695
'tag' => $processor->get_tag(),
706-
'xpath' => $processor->get_stored_xpath(),
696+
'xpath' => $processor->get_xpath(),
707697
'depth' => $processor->get_current_depth(),
708698
);
709699
}
@@ -744,7 +734,7 @@ public function test_bookmarking_and_seeking(): void {
744734
$processor->seek( $bookmark );
745735
$sought_actual_contents[] = array(
746736
'tag' => $processor->get_tag(),
747-
'xpath' => $processor->get_stored_xpath(),
737+
'xpath' => $processor->get_xpath(),
748738
'depth' => $processor->get_current_depth(),
749739
);
750740
}

0 commit comments

Comments
 (0)