diff --git a/lib/MetaCPAN/Server/Controller/User/Favorite.pm b/lib/MetaCPAN/Server/Controller/User/Favorite.pm index 7f73ad78b..1f5228f40 100644 --- a/lib/MetaCPAN/Server/Controller/User/Favorite.pm +++ b/lib/MetaCPAN/Server/Controller/User/Favorite.pm @@ -3,8 +3,10 @@ package MetaCPAN::Server::Controller::User::Favorite; use strict; use warnings; +use List::Util qw( uniq ); +use MetaCPAN::ESConfig qw( es_doc_path ); +use MetaCPAN::Util qw( true false hit_total ); use Moose; -use MetaCPAN::Util qw( true false ); BEGIN { extends 'Catalyst::Controller::REST' } @@ -36,17 +38,41 @@ sub index_POST { sub index_DELETE { my ( $self, $c, $distribution ) = @_; - my $favorite - = $c->model('ESModel') - ->doc('favorite') - ->get( { user => $c->user->id, distribution => $distribution } ); - if ($favorite) { - $favorite->delete( { refresh => true } ); - $c->purge_author_key( $favorite->author ) - if $favorite->author; + my $user_id = $c->user->id; + + my $query = { + bool => { + must => [ + { term => { user => $user_id } }, + { term => { distribution => $distribution } }, + ], + }, + }; + + my $res = $c->model('ES')->search( + es_doc_path('favorite'), + body => { + query => $query, + size => 100, + }, + ); + + if ( hit_total($res) ) { + my @authors = uniq grep {defined} + map { $_->{_source}{author} } @{ $res->{hits}{hits} }; + + $c->model('ES')->delete_by_query( + es_doc_path('favorite'), + body => { query => $query }, + refresh => true, + ); + + for my $author (@authors) { + $c->purge_author_key($author); + } $c->purge_dist_key($distribution); - $self->status_ok( $c, - entity => $favorite->meta->get_data($favorite) ); + + $self->status_ok( $c, entity => $res->{hits}{hits}[0]{_source}, ); } else { $self->status_not_found( $c, message => 'Entity could not be found' ); diff --git a/t/lib/MetaCPAN/TestServer.pm b/t/lib/MetaCPAN/TestServer.pm index 0520edd34..4acc0c546 100644 --- a/t/lib/MetaCPAN/TestServer.pm +++ b/t/lib/MetaCPAN/TestServer.pm @@ -2,7 +2,7 @@ package MetaCPAN::TestServer; use MetaCPAN::Moose; -use MetaCPAN::ESConfig qw( es_config ); +use MetaCPAN::ESConfig qw( es_config es_doc_path ); use MetaCPAN::Script::Author (); use MetaCPAN::Script::BusFactor (); use MetaCPAN::Script::Cover (); @@ -291,6 +291,32 @@ sub prepare_user_test_data { ); $self->_create_test_favorites( $user->id ); + + # Favorite fixture with a duplicate (different ES doc ID) for delete tests. + my $user_id = $user->id; + + MetaCPAN::Server->model('ESModel')->doc('favorite')->put( + { + user => $user_id, + distribution => 'WWW-Mechanize', + release => 'WWW-Mechanize-1.00', + author => 'JESSE', + }, + { refresh => true } + ); + + $self->es_client->index( + es_doc_path('favorite'), + id => 'duplicate_favorite_doc', + body => { + user => $user_id, + distribution => 'WWW-Mechanize', + release => 'WWW-Mechanize-2.00', + author => 'SIMBABQUE', + date => '2024-01-01T00:00:00', + }, + refresh => 'true', + ); } sub _create_test_favorites { diff --git a/t/server/controller/user/favorite.t b/t/server/controller/user/favorite.t index fefcc1390..22367fff1 100644 --- a/t/server/controller/user/favorite.t +++ b/t/server/controller/user/favorite.t @@ -3,8 +3,10 @@ use warnings; use lib 't/lib'; use Cpanel::JSON::XS qw( encode_json ); -use MetaCPAN::Server::Test qw( app DELETE GET POST test_psgi ); +use MetaCPAN::ESConfig qw( es_doc_path ); +use MetaCPAN::Server::Test qw( app DELETE es GET POST test_psgi ); use MetaCPAN::TestHelpers qw( decode_json_ok ); +use MetaCPAN::Util qw( hit_total ); use Test::More; test_psgi app, sub { @@ -64,4 +66,127 @@ test_psgi app, sub { is( $user->code, 200, 'code 200' ); }; +subtest 'API enforces uniqueness on (user, distribution)' => sub { + test_psgi app, sub { + my $cb = shift; + + ok( my $user_res = $cb->( GET '/user?access_token=testing' ), + 'get user' ); + my $user_id = decode_json_ok($user_res)->{id}; + + my $res = $cb->( + POST '/user/favorite?access_token=testing', + Content_Type => 'application/json', + Content => encode_json( { + distribution => 'WWW-Mechanize', + release => 'WWW-Mechanize-2.00', + author => 'SIMBABQUE', + } ) + ); + is( $res->code, 201, 'second POST returns 201' ); + + my $search = es()->search( + es_doc_path('favorite'), + body => { + query => { + bool => { + must => [ + { term => { user => $user_id } }, + { term => { distribution => 'WWW-Mechanize' } }, + ], + }, + }, + }, + ); + is( hit_total($search), 2, 'upsert did not create a third doc' ); + }; +}; + +subtest 'DELETE removes all duplicates but preserves other users' => sub { + test_psgi app, sub { + my $cb = shift; + + ok( my $user_res = $cb->( GET '/user?access_token=testing' ), + 'get user' ); + my $user_id = decode_json_ok($user_res)->{id}; + + my $bot_res = $cb->( GET '/user?access_token=bot' ); + my $bot_id = decode_json_ok($bot_res)->{id}; + + my $res = $cb->( + POST '/user/favorite?access_token=bot', + Content_Type => 'application/json', + Content => encode_json( { + distribution => 'WWW-Mechanize', + release => 'WWW-Mechanize-1.00', + author => 'JESSE', + } ) + ); + is( $res->code, 201, 'bot user also favorited WWW-Mechanize' ); + + my $search = es()->search( + es_doc_path('favorite'), + body => { + query => { + bool => { + must => [ + { term => { user => $user_id } }, + { term => { distribution => 'WWW-Mechanize' } }, + ], + }, + }, + }, + ); + ok( hit_total($search) >= 2, + 'testing user has duplicate favorites before delete' ); + + ok( + $res = $cb->( + DELETE '/user/favorite/WWW-Mechanize?access_token=testing' + ), + 'DELETE duplicate favorites' + ); + is( $res->code, 200, 'delete returned 200' ); + + $search = es()->search( + es_doc_path('favorite'), + body => { + query => { + bool => { + must => [ + { term => { user => $user_id } }, + { term => { distribution => 'WWW-Mechanize' } }, + ], + }, + }, + }, + ); + is( hit_total($search), 0, 'all favorites for dist deleted' ); + + $search = es()->search( + es_doc_path('favorite'), + body => { + query => { + bool => { + must => [ + { term => { user => $bot_id } }, + { term => { distribution => 'WWW-Mechanize' } }, + ], + }, + }, + }, + ); + is( hit_total($search), 1, + 'favorite for same dist by other user are preserved' ); + }; +}; + +test_psgi app, sub { + my $cb = shift; + + my $res + = $cb->( DELETE '/user/favorite/No-Such-Dist?access_token=testing' ); + is( $res->code, 404, 'delete nonexistent favorite returns 404' ); +}; + done_testing;