Skip to content

Commit 724e3f3

Browse files
pfefferlepfefferle
authored
Refactor tombstone and delete handler logic (#2446)
1 parent 62d1549 commit 724e3f3

File tree

4 files changed

+266
-18
lines changed

4 files changed

+266
-18
lines changed

‎.github/changelog/2446-from-description‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Significance: patch
2+
Type: fixed
3+
4+
Improved delete handling for remote replies by streamlining tombstone detection and simplifying object deletion for more reliable and consistent behavior.

‎includes/class-tombstone.php‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -88,17 +88,20 @@ public static function exists_remote( $url ) {
8888
*/
8989
\do_action( 'activitypub_pre_http_is_tombstone', $url );
9090

91-
$response = \wp_safe_remote_get( $url, array( 'headers' => array( 'Accept' => 'application/activity+json' ) ) );
92-
$code = \wp_remote_retrieve_response_code( $response );
91+
$response = Http::get( $url );
9392

94-
if ( in_array( (int) $code, self::$codes, true ) ) {
95-
return true;
93+
if ( ! \is_wp_error( $response ) ) {
94+
$data = \wp_remote_retrieve_body( $response );
95+
$data = \json_decode( $data, true );
96+
97+
return self::check_array( $data );
9698
}
9799

98-
$data = \wp_remote_retrieve_body( $response );
99-
$data = \json_decode( $data, true );
100+
if ( in_array( (int) $response->get_error_code(), self::$codes, true ) ) {
101+
return true;
102+
}
100103

101-
return self::check_array( $data );
104+
return false;
102105
}
103106

104107
/**

‎includes/handler/class-delete.php‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
use Activitypub\Collection\Remote_Actors;
1313
use Activitypub\Tombstone;
1414

15-
use function Activitypub\is_activity_reply;
1615
use function Activitypub\object_to_uri;
1716

1817
/**
@@ -104,10 +103,9 @@ public static function handle_delete( $activity, $user_ids ) {
104103
* @param int|int[] $user_ids The user ID(s).
105104
*/
106105
public static function delete_object( $activity, $user_ids ) {
107-
// Check for private and/or direct messages.
108-
if ( is_activity_reply( $activity ) ) {
109-
$result = self::maybe_delete_interaction( $activity );
110-
} else {
106+
$result = self::maybe_delete_interaction( $activity );
107+
108+
if ( ! $result ) {
111109
$result = self::maybe_delete_post( $activity );
112110
}
113111

@@ -248,12 +246,7 @@ public static function delete_posts( $id ) {
248246
* @return bool True on success, false otherwise.
249247
*/
250248
public static function maybe_delete_interaction( $activity ) {
251-
if ( is_array( $activity['object'] ) ) {
252-
$id = $activity['object']['id'];
253-
} else {
254-
$id = $activity['object'];
255-
}
256-
249+
$id = object_to_uri( $activity['object'] );
257250
$comments = Interactions::get_by_id( $id );
258251

259252
if ( $comments && Tombstone::exists( $id ) ) {

‎tests/phpunit/tests/includes/handler/class-test-delete.php‎

Lines changed: 248 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,254 @@ function ( $actor_id ) use ( &$result ) {
262262
\remove_all_actions( 'activitypub_delete_remote_actor_posts', 5 );
263263
}
264264

265+
/**
266+
* Test delete_object with Tombstone type object having only an id.
267+
*
268+
* This tests the scenario where a Delete activity contains a Tombstone object
269+
* with only an 'id' field, which is common in ActivityPub implementations.
270+
*
271+
* @covers ::delete_object
272+
* @covers ::maybe_delete_interaction
273+
* @covers ::maybe_delete_post
274+
*/
275+
public function test_delete_object_with_tombstone_id_only() {
276+
$post_id = self::factory()->post->create(
277+
array(
278+
'post_author' => self::$user_id,
279+
)
280+
);
281+
282+
$actor_url = 'https://example.com/users/testactor';
283+
$object_url = 'https://example.com/objects/123';
284+
285+
// Create a comment (interaction) that will be deleted.
286+
$comment_id = self::factory()->comment->create(
287+
array(
288+
'comment_post_ID' => $post_id,
289+
'comment_type' => 'like',
290+
'comment_author' => 'Test Actor',
291+
'comment_author_url' => $actor_url,
292+
'comment_meta' => array(
293+
'protocol' => 'activitypub',
294+
'source_id' => $object_url,
295+
),
296+
)
297+
);
298+
299+
// Create a Delete activity with Tombstone type and only an id.
300+
$activity = array(
301+
'type' => 'Delete',
302+
'actor' => $actor_url,
303+
'object' => array(
304+
'type' => 'Tombstone',
305+
'id' => $object_url,
306+
),
307+
);
308+
309+
// Mock HTTP request to return 410 Gone for the tombstone check.
310+
$filter = function ( $preempt, $args, $url ) use ( $object_url ) {
311+
if ( $url === $object_url ) {
312+
return array(
313+
'body' => '',
314+
'response' => array( 'code' => 410 ),
315+
);
316+
}
317+
return $preempt;
318+
};
319+
\add_filter( 'pre_http_request', $filter, 10, 3 );
320+
321+
// Verify comment exists before delete.
322+
$this->assertInstanceOf( 'WP_Comment', \get_comment( $comment_id ), 'Comment should exist before delete' );
323+
324+
// Call delete_object.
325+
Delete::delete_object( $activity, array( self::$user_id ) );
326+
327+
// Verify comment was deleted.
328+
$this->assertNull( \get_comment( $comment_id ), 'Comment should be deleted after delete_object' );
329+
330+
\remove_filter( 'pre_http_request', $filter );
331+
}
332+
333+
/**
334+
* Test delete_object with Tombstone type for post deletion.
335+
*
336+
* Tests deleting a post from the Posts collection using a Tombstone object.
337+
*
338+
* @covers ::delete_object
339+
* @covers ::maybe_delete_post
340+
*/
341+
public function test_delete_object_tombstone_deletes_post() {
342+
$object_url = 'https://example.com/notes/456';
343+
$actor_url = 'https://example.com/users/testactor';
344+
345+
// Create a post in the Posts collection.
346+
$post_id = \wp_insert_post(
347+
array(
348+
'post_type' => \Activitypub\Collection\Posts::POST_TYPE,
349+
'post_title' => 'Test Note',
350+
'post_content' => 'Test content',
351+
'post_status' => 'publish',
352+
'guid' => $object_url,
353+
)
354+
);
355+
356+
// Create Delete activity with Tombstone.
357+
$activity = array(
358+
'type' => 'Delete',
359+
'actor' => $actor_url,
360+
'object' => array(
361+
'type' => 'Tombstone',
362+
'id' => $object_url,
363+
),
364+
);
365+
366+
// Mock HTTP request to return 410 Gone for the tombstone check.
367+
$filter = function ( $preempt, $args, $url ) use ( $object_url ) {
368+
if ( $url === $object_url ) {
369+
return array(
370+
'body' => '',
371+
'response' => array( 'code' => 410 ),
372+
);
373+
}
374+
return $preempt;
375+
};
376+
\add_filter( 'pre_http_request', $filter, 10, 3 );
377+
378+
// Verify post exists before delete.
379+
$this->assertInstanceOf( 'WP_Post', \get_post( $post_id ), 'Post should exist before delete' );
380+
381+
// Call delete_object.
382+
Delete::delete_object( $activity, array( self::$user_id ) );
383+
384+
// Verify post was deleted.
385+
$this->assertNull( \get_post( $post_id ), 'Post should be deleted after delete_object' );
386+
387+
\remove_filter( 'pre_http_request', $filter );
388+
}
389+
390+
/**
391+
* Test delete_object with Tombstone but no matching content.
392+
*
393+
* Verifies that delete_object handles gracefully when there's nothing to delete.
394+
*
395+
* @covers ::delete_object
396+
* @covers ::maybe_delete_interaction
397+
* @covers ::maybe_delete_post
398+
*/
399+
public function test_delete_object_tombstone_no_matching_content() {
400+
$object_url = 'https://example.com/nonexistent/789';
401+
$actor_url = 'https://example.com/users/testactor';
402+
403+
// Create Delete activity with Tombstone for non-existent content.
404+
$activity = array(
405+
'type' => 'Delete',
406+
'actor' => $actor_url,
407+
'object' => array(
408+
'type' => 'Tombstone',
409+
'id' => $object_url,
410+
),
411+
);
412+
413+
// Mock HTTP request to return 410 Gone for the tombstone check.
414+
$filter = function ( $preempt, $args, $url ) use ( $object_url ) {
415+
if ( $url === $object_url ) {
416+
return array(
417+
'body' => '',
418+
'response' => array( 'code' => 410 ),
419+
);
420+
}
421+
return $preempt;
422+
};
423+
\add_filter( 'pre_http_request', $filter, 10, 3 );
424+
425+
// Track if the action was fired.
426+
$action_fired = false;
427+
$action_success = null;
428+
429+
\add_action(
430+
'activitypub_handled_delete',
431+
function ( $act, $users, $success ) use ( &$action_fired, &$action_success ) {
432+
$action_fired = true;
433+
$action_success = $success;
434+
},
435+
10,
436+
3
437+
);
438+
439+
// Call delete_object - should not throw errors.
440+
Delete::delete_object( $activity, array( self::$user_id ) );
441+
442+
// Verify action was fired but success is false (nothing to delete).
443+
$this->assertTrue( $action_fired, 'activitypub_handled_delete action should fire' );
444+
$this->assertFalse( $action_success, 'Success should be false when nothing was deleted' );
445+
446+
\remove_filter( 'pre_http_request', $filter );
447+
\remove_all_actions( 'activitypub_handled_delete' );
448+
}
449+
450+
/**
451+
* Test delete_object with Tombstone as string ID.
452+
*
453+
* Tests the case where the object is just a string URL (without type field).
454+
*
455+
* @covers ::delete_object
456+
* @covers ::maybe_delete_interaction
457+
*/
458+
public function test_delete_object_with_tombstone_string_id() {
459+
$post_id = self::factory()->post->create(
460+
array(
461+
'post_author' => self::$user_id,
462+
)
463+
);
464+
465+
$actor_url = 'https://example.com/users/testactor';
466+
$object_url = 'https://example.com/objects/string-test';
467+
468+
// Create a comment.
469+
$comment_id = self::factory()->comment->create(
470+
array(
471+
'comment_post_ID' => $post_id,
472+
'comment_type' => 'announce',
473+
'comment_author' => 'Test Actor',
474+
'comment_author_url' => $actor_url,
475+
'comment_meta' => array(
476+
'protocol' => 'activitypub',
477+
'source_id' => $object_url,
478+
),
479+
)
480+
);
481+
482+
// Create Delete activity with object as string (common pattern).
483+
$activity = array(
484+
'type' => 'Delete',
485+
'actor' => $actor_url,
486+
'object' => $object_url,
487+
);
488+
489+
// Mock HTTP request to return 404 Not Found for the tombstone check.
490+
$filter = function ( $preempt, $args, $url ) use ( $object_url ) {
491+
if ( $url === $object_url ) {
492+
return array(
493+
'body' => '',
494+
'response' => array( 'code' => 404 ),
495+
);
496+
}
497+
return $preempt;
498+
};
499+
\add_filter( 'pre_http_request', $filter, 10, 3 );
500+
501+
// Verify comment exists.
502+
$this->assertInstanceOf( 'WP_Comment', \get_comment( $comment_id ), 'Comment should exist before delete' );
503+
504+
// Call delete_object.
505+
Delete::delete_object( $activity, array( self::$user_id ) );
506+
507+
// Verify comment was deleted.
508+
$this->assertNull( \get_comment( $comment_id ), 'Comment should be deleted when object is string URL' );
509+
510+
\remove_filter( 'pre_http_request', $filter );
511+
}
512+
265513
/**
266514
* Get remote metadata by actor.
267515
*

0 commit comments

Comments
 (0)