Skip to content

Commit 0ed32e0

Browse files
obenlandobenland
authored
Simplify return types for follow() API functions (#2384)
1 parent 70c7528 commit 0ed32e0

File tree

4 files changed

+104
-20
lines changed

4 files changed

+104
-20
lines changed

‎.github/changelog/2384-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: changed
3+
4+
Simplify follow() API return types to int|WP_Error for better predictability.

‎includes/collection/class-following.php‎

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ class Following {
6060
* @param \WP_Post|int $post The ID of the remote Actor.
6161
* @param int $user_id The ID of the WordPress User.
6262
*
63-
* @return int|false|\WP_Post|\WP_Error The Outbox ID or false on failure, the Actor post or a WP_Error.
63+
* @return int|\WP_Error The Outbox ID on success or a WP_Error on failure.
6464
*/
6565
public static function follow( $post, $user_id ) {
6666
$post = \get_post( $post );
@@ -73,25 +73,57 @@ public static function follow( $post, $user_id ) {
7373
$following = $all_meta[ self::FOLLOWING_META_KEY ] ?? array();
7474
$pending = $all_meta[ self::PENDING_META_KEY ] ?? array();
7575

76-
if ( ! \in_array( (string) $user_id, $following, true ) && ! \in_array( (string) $user_id, $pending, true ) ) {
77-
$actor = Actors::get_by_id( $user_id );
76+
if ( \in_array( (string) $user_id, $following, true ) || \in_array( (string) $user_id, $pending, true ) ) {
77+
$post_id_query = new \WP_Query(
78+
array(
79+
'post_type' => Outbox::POST_TYPE,
80+
'post_status' => 'any',
81+
'posts_per_page' => 1,
82+
'no_found_rows' => true,
83+
'author' => \max( $user_id, 0 ),
84+
'fields' => 'ids',
85+
'order' => 'DESC',
86+
'meta_query' => array( // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_query
87+
array(
88+
'key' => '_activitypub_object_id',
89+
'value' => $post->guid,
90+
),
91+
array(
92+
'key' => '_activitypub_activity_type',
93+
'value' => 'Follow',
94+
),
95+
),
96+
)
97+
);
7898

79-
if ( \is_wp_error( $actor ) ) {
80-
return $actor;
99+
if ( $post_id_query->posts ) {
100+
return $post_id_query->posts[0];
81101
}
82102

83-
\add_post_meta( $post->ID, self::PENDING_META_KEY, (string) $user_id );
103+
return new \WP_Error( 'activitypub_already_following', 'User is already following this actor but outbox activity not found.' );
104+
}
84105

85-
$follow = new Activity();
86-
$follow->set_type( 'Follow' );
87-
$follow->set_actor( $actor->get_id() );
88-
$follow->set_object( $post->guid );
89-
$follow->set_to( array( $post->guid ) );
106+
$actor = Actors::get_by_id( $user_id );
90107

91-
return add_to_outbox( $follow, null, $user_id, ACTIVITYPUB_CONTENT_VISIBILITY_PRIVATE );
108+
if ( \is_wp_error( $actor ) ) {
109+
return $actor;
92110
}
93111

94-
return $post;
112+
\add_post_meta( $post->ID, self::PENDING_META_KEY, (string) $user_id );
113+
114+
$follow = new Activity();
115+
$follow->set_type( 'Follow' );
116+
$follow->set_actor( $actor->get_id() );
117+
$follow->set_object( $post->guid );
118+
$follow->set_to( array( $post->guid ) );
119+
120+
$result = add_to_outbox( $follow, null, $user_id, ACTIVITYPUB_CONTENT_VISIBILITY_PRIVATE );
121+
122+
if ( ! $result ) {
123+
return new \WP_Error( 'activitypub_follow_failed', 'Failed to add follow activity to outbox.' );
124+
}
125+
126+
return $result;
95127
}
96128

97129
/**

‎includes/functions.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1598,7 +1598,7 @@ function add_to_outbox( $data, $activity_type = null, $user_id = 0, $content_vis
15981598
* @param string|int $remote_actor The Actor URL, WebFinger Resource or Post-ID of the remote Actor.
15991599
* @param int $user_id The ID of the WordPress User.
16001600
*
1601-
* @return int|false|\WP_Post|\WP_Error The Outbox ID or false on failure, the Actor post or a WP_Error.
1601+
* @return int|\WP_Error The Outbox ID on success or a WP_Error on failure.
16021602
*/
16031603
function follow( $remote_actor, $user_id ) {
16041604
if ( \is_numeric( $remote_actor ) ) {

‎tests/phpunit/tests/includes/collection/class-test-following.php‎

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,54 @@ public function test_accept_with_non_array_pending_meta() {
287287
\wp_delete_post( $post_id );
288288
}
289289

290+
/**
291+
* Test follow returns existing outbox activity when already following.
292+
*
293+
* @covers ::follow
294+
*/
295+
public function test_follow_returns_existing_activity_when_already_following() {
296+
\add_filter( 'activitypub_pre_http_get_remote_object', array( $this, 'mock_remote_actor' ), 10, 2 );
297+
298+
$user_id = self::factory()->user->create( array( 'role' => 'administrator' ) );
299+
\get_user_by( 'id', $user_id )->add_cap( 'activitypub' );
300+
301+
// First follow creates the relationship and outbox activity.
302+
$first_result = follow( 'https://example.com/actor/1', $user_id );
303+
$this->assertIsInt( $first_result, 'First follow should return an outbox activity ID' );
304+
305+
// Second follow should return the same outbox activity ID.
306+
$second_result = follow( 'https://example.com/actor/1', $user_id );
307+
$this->assertIsInt( $second_result, 'Second follow should return an outbox activity ID' );
308+
$this->assertEquals( $first_result, $second_result, 'Should return the same outbox activity ID' );
309+
310+
\remove_filter( 'activitypub_pre_http_get_remote_object', array( $this, 'mock_remote_actor' ) );
311+
}
312+
313+
/**
314+
* Test follow returns error when metadata exists but outbox activity is missing.
315+
*
316+
* @covers ::follow
317+
*/
318+
public function test_follow_returns_error_on_inconsistent_state() {
319+
// Create a remote actor post.
320+
$post_id = self::factory()->post->create(
321+
array(
322+
'post_title' => 'Test Remote Actor',
323+
'post_status' => 'publish',
324+
'post_type' => Remote_Actors::POST_TYPE,
325+
)
326+
);
327+
328+
// Manually add metadata to simulate following without an outbox activity.
329+
\add_post_meta( $post_id, Following::PENDING_META_KEY, '1' );
330+
331+
// Attempt to follow should return an error due to inconsistent state.
332+
$result = Following::follow( $post_id, 1 );
333+
334+
$this->assertWPError( $result, 'Should return WP_Error for inconsistent state' );
335+
$this->assertEquals( 'activitypub_already_following', $result->get_error_code() );
336+
}
337+
290338
/**
291339
* Test unfollow removes user from following list.
292340
*
@@ -306,16 +354,16 @@ public function test_unfollow_removes_user() {
306354

307355
// Use global follow() function to add a follow request.
308356
$remote_actor_url = \get_post( $post_id )->guid;
309-
\Activitypub\follow( $remote_actor_url, $user_id );
357+
follow( $remote_actor_url, $user_id );
310358
\clean_post_cache( $post_id );
311359

312360
// Verify user is in following list (pending or following).
313-
$following = \get_post_meta( $post_id, \Activitypub\Collection\Following::FOLLOWING_META_KEY, false );
314-
$pending = \get_post_meta( $post_id, \Activitypub\Collection\Following::PENDING_META_KEY, false );
361+
$following = \get_post_meta( $post_id, Following::FOLLOWING_META_KEY, false );
362+
$pending = \get_post_meta( $post_id, Following::PENDING_META_KEY, false );
315363
$this->assertTrue( in_array( (string) $user_id, $following, true ) || in_array( (string) $user_id, $pending, true ) );
316364

317365
// Remove following.
318-
$result = \Activitypub\Collection\Following::unfollow( $post_id, $user_id );
366+
$result = Following::unfollow( $post_id, $user_id );
319367

320368
\clean_post_cache( $post_id );
321369

@@ -324,8 +372,8 @@ public function test_unfollow_removes_user() {
324372
$this->assertEquals( $post_id, $result->ID );
325373

326374
// User should no longer be in following list.
327-
$following = \get_post_meta( $post_id, \Activitypub\Collection\Following::FOLLOWING_META_KEY, false );
328-
$pending = \get_post_meta( $post_id, \Activitypub\Collection\Following::PENDING_META_KEY, false );
375+
$following = \get_post_meta( $post_id, Following::FOLLOWING_META_KEY, false );
376+
$pending = \get_post_meta( $post_id, Following::PENDING_META_KEY, false );
329377

330378
$this->assertNotContains( (string) $user_id, $following );
331379
$this->assertNotContains( (string) $user_id, $pending );

0 commit comments

Comments
 (0)