remove join parameter

haarg · haarg · commit ab05bf244539 · 2024-04-30T22:42:15.000+02:00
The join feature is something we try to add on top of Elasticsearch. It
complicates our interactions with external queries and we don't use them
anywhere in our own code.
diff --git a/docs/API-docs.md b/docs/API-docs.md
@@ -51,36 +51,6 @@ Performing a search without any constraints is an easy way to get sample data
 * [`/rating/_search`](https://fastapi.metacpan.org/v1/rating/_search)
 * [`/release/_search`](https://fastapi.metacpan.org/v1/release/_search)
 
-## Joins
-
-ElasticSearch itself doesn't support joining data across multiple types. The API server can, however, handle a `join` query parameter if the underlying type was set up accordingly. Browse https://github.com/metacpan/metacpan-api/blob/master/lib/MetaCPAN/Server/Controller/ to see all join conditions. Here are some examples.
-
-Joins on documents:
-
-* [/author/PERLER?join=favorite](https://fastapi.metacpan.org/v1/author/PERLER?join=favorite)
-* [/author/PERLER?join=favorite&join=release](https://fastapi.metacpan.org/v1/author/PERLER?join=favorite&join=release)
-* [/release/Moose?join=author](https://fastapi.metacpan.org/v1/release/Moose?join=author)
-* [/module/Moose?join=release](https://fastapi.metacpan.org/v1/module/Moose?join=release)
-
-Joins on search results is work in progress.
-
-Restricting the joined results can be done by using the [boolean "should"](https://www.elastic.co/guide/en/elasticsearch/reference/2.4/query-dsl-bool-query.html) occurrence type:
-
-```sh
-curl -XPOST https://fastapi.metacpan.org/v1/author/PERLER?join=release -d '
-{
-    "query": {
-        "bool": {
-            "should": [{
-                "term": {
-                    "release.status": "latest"
-                }
-            }]
-        }
-    }
-}'
-```
-
 ## JSONP
 
 Simply add a `callback` query parameter with the name of your callback, and you'll get a JSONP response.
diff --git a/lib/MetaCPAN/Server/Controller.pm b/lib/MetaCPAN/Server/Controller.pm
@@ -26,14 +26,6 @@ has type => (
     default => sub { shift->action_namespace },
 );
 
-has relationships => (
-    is      => 'ro',
-    isa     => HashRef,
-    default => sub { {} },
-    traits  => ['Hash'],
-    handles => { has_relationships => 'count' },
-);
-
 my $MAX_SIZE = 5000;
 
 # apply "filters" like \&model but for fabricated data
@@ -128,90 +120,6 @@ sub search : Path('_search') : ActionClass('~Deserialize') {
     } or do { $self->internal_error( $c, $@ ) };
 }
 
-sub join : ActionClass('~Deserialize') {
-    my ( $self, $c ) = @_;
-    my $joins     = $self->relationships;
-    my @req_joins = $c->req->param('join');
-    my $is_get    = ref $c->stash->{hits} ? 0 : 1;
-    my $query
-        = $c->req->params->{q}
-        ? { query => { query_string => { query => $c->req->params->{q} } } }
-        : $c->req->data ? $c->req->data
-        :                 { query => { match_all => {} } };
-    $c->detach(
-        '/not_allowed',
-        [
-            'unknown join type, valid values are '
-                . Moose::Util::english_list( keys %$joins )
-        ]
-    ) if ( scalar grep { !$joins->{$_} } @req_joins );
-
-    while ( my ( $join, $config ) = each %$joins ) {
-        my $has_many = ref $config->{type};
-        my ($type)   = $has_many ? @{ $config->{type} } : $config->{type};
-        my $cself    = $config->{self} || $join;
-        next unless ( grep { $_ eq $join } @req_joins );
-        my $data
-            = $is_get
-            ? [ $c->stash ]
-            : [
-            map {
-                $_->{_source}
-                    || single_valued_arrayref_to_scalar( $_->{fields} )
-            } @{ $c->stash->{hits}->{hits} }
-            ];
-        my @ids = List::AllUtils::uniq grep {defined}
-            map { ref $cself eq 'CODE' ? $cself->($_) : $_->{$cself} } @$data;
-        my $filter   = { terms => { $config->{foreign} => [@ids] } };
-        my $filtered = {%$query};    # don't work on $query
-        $filtered->{filter}
-            = $query->{filter}
-            ? { and => [ $filter, $query->{filter} ] }
-            : $filter;
-        my $foreign = eval {
-            $c->model("CPAN::$type")->query( $filtered->{query} )
-                ->filter( $filtered->{filter} )->size(1000)->raw->all;
-        } or do { $self->internal_error( $c, $@ ) };
-        $c->detach(
-            "/not_allowed",
-            [
-                'The number of joined documents exceeded the allowed number of 1000 documents by '
-                    . ( $foreign->{hits}->{total} - 1000 )
-                    . '. Please reduce the number of documents or apply additional filters.'
-            ]
-        ) if ( $foreign->{hits}->{total} > 1000 );
-        $c->stash->{took} += $foreign->{took} unless ($is_get);
-
-        if ($has_many) {
-            my $many;
-            for ( @{ $foreign->{hits}->{hits} } ) {
-                my $list = $many->{ $_->{_source}->{ $config->{foreign} } }
-                    ||= [];
-                push( @$list, $_ );
-            }
-            $foreign = $many;
-        }
-        else {
-            $foreign = { map { $_->{_source}->{ $config->{foreign} } => $_ }
-                    @{ $foreign->{hits}->{hits} } };
-        }
-        for (@$data) {
-            my $key = ref $cself eq 'CODE' ? $cself->($_) : $_->{$cself};
-            next unless ($key);
-            my $result = $foreign->{$key};
-            $_->{$join}
-                = $has_many
-                ? {
-                hits => {
-                    hits  => $result,
-                    total => scalar @{ $result || [] }
-                }
-                }
-                : $result;
-        }
-    }
-}
-
 sub not_found : Private {
     my ( $self, $c ) = @_;
     $c->cdn_never_cache(1);
@@ -238,57 +146,8 @@ sub internal_error {
 
 sub end : Private {
     my ( $self, $c ) = @_;
-    $c->forward('join')
-        if ( $self->has_relationships && $c->req->param('join') );
     $c->forward('/end');
 }
 
 __PACKAGE__->meta->make_immutable;
 1;
-
-__END__
-
-=head1 ATTRIBUTES
-
-=head2 relationships
-
- MetaCPAN::Server::Controller::Author->config(
-     relationships => {
-         release => {
-             type    => ['Release'],
-             self    => 'pauseid',
-             foreign => 'author',
-         }
-     }
- );
-
-Contains a HashRef of relationships with other controllers.
-If C<type> is an ArrayRef, the relationship is considered a
-I<has many> relationship.
-
-Unless a C<self> exists, the name of the relationship is used
-as key to join on. C<self> can also be a CodeRef, if the foreign
-key is build from several local keys. In this case, again the name of
-the relationship is used as key in the result.
-
-C<foreign> refers to the foreign key on the C<type> controller the data
-is joined with.
-
-=head1 ACTIONS
-
-=head2 join
-
-This action is called if the controller has L</relationships> defined
-and if one or more C<join> query parameters are defined. It then
-does a I<left join> based on the information provided by
-L</relationships>.
-
-This works both for GET requests, where only one document is requested
-and search requests, where a number of documents is returned.
-It also passes through search data (either the C<q> query string or
-the request body).
-
-B<The number of documents that can be joined is limited to 1000 per
-relationship for now.>
-
-=cut
diff --git a/lib/MetaCPAN/Server/Controller/Author.pm b/lib/MetaCPAN/Server/Controller/Author.pm
@@ -10,21 +10,6 @@ BEGIN { extends 'MetaCPAN::Server::Controller' }
 
 with 'MetaCPAN::Server::Role::JSONP';
 
-__PACKAGE__->config(
-    relationships => {
-        release => {
-            type    => ['Release'],
-            self    => 'pauseid',
-            foreign => 'author',
-        },
-        favorite => {
-            type    => ['Favorite'],
-            self    => 'user',
-            foreign => 'user',
-        }
-    }
-);
-
 # https://fastapi.metacpan.org/v1/author/LLAP
 sub get : Path('') : Args(1) {
     my ( $self, $c, $id ) = @_;
diff --git a/lib/MetaCPAN/Server/Controller/Changes.pm b/lib/MetaCPAN/Server/Controller/Changes.pm
@@ -12,8 +12,6 @@ BEGIN { extends 'MetaCPAN::Server::Controller' }
 
 with 'MetaCPAN::Server::Role::JSONP';
 
-# TODO: __PACKAGE__->config(relationships => ?)
-
 has '+type' => ( default => 'file' );
 
 sub index : Chained('/') : PathPart('changes') : CaptureArgs(0) {
diff --git a/lib/MetaCPAN/Server/Controller/File.pm b/lib/MetaCPAN/Server/Controller/File.pm
@@ -11,23 +11,6 @@ BEGIN { extends 'MetaCPAN::Server::Controller' }
 
 with 'MetaCPAN::Server::Role::JSONP';
 
-__PACKAGE__->config(
-    relationships => {
-        author => {
-            type    => 'Author',
-            foreign => 'pauseid',
-        },
-        release => {
-            type => 'Release',
-            self => sub {
-                ElasticSearchX::Model::Util::digest( $_[0]->{author},
-                    $_[0]->{release} );
-            },
-            foreign => 'id',
-        }
-    }
-);
-
 sub find : Path('') {
     my ( $self, $c, $author, $release, @path ) = @_;
 
diff --git a/lib/MetaCPAN/Server/Controller/Release.pm b/lib/MetaCPAN/Server/Controller/Release.pm
@@ -10,15 +10,6 @@ BEGIN { extends 'MetaCPAN::Server::Controller' }
 
 with 'MetaCPAN::Server::Role::JSONP';
 
-__PACKAGE__->config(
-    relationships => {
-        author => {
-            type    => 'Author',
-            foreign => 'pauseid',
-        }
-    }
-);
-
 sub find : Path('') : Args(1) {
     my ( $self, $c, $name ) = @_;
     my $file = $self->model($c)->find($name);
diff --git a/t/server/controller/author.t b/t/server/controller/author.t
@@ -40,7 +40,7 @@ my %tests = (
 test_psgi app, sub {
     my $cb = shift;
     while ( my ( $k, $v ) = each %tests ) {
-        ok( my $res = $cb->( GET $k), "GET $k" );
+        ok( my $res = $cb->( GET $k ), "GET $k" );
         is( $res->code, $v->{code}, "code " . $v->{code} );
         is(
             $res->header('content-type'),
@@ -79,73 +79,13 @@ test_psgi app, sub {
         'POST _search'
     );
 
-    my $json = decode_json_ok($res);
-    is( @{ $json->{hits}->{hits} }, 0, '0 results' );
+    ok( $res = $cb->( GET '/author/DOY' ), 'GET /author/DOY' );
 
-    ok( $res = $cb->( GET '/author/DOY?join=release' ),
-        'GET /author/DOY?join=release' );
+    my $doy = decode_json_ok($res);
 
-    $json = decode_json_ok($res);
-    is( @{ $json->{release}->{hits}->{hits} }, 4, 'joined 4 releases' );
+    is( $doy->{pauseid}, 'DOY', 'found author' );
 
-    ok(
-        $res = $cb->(
-            POST '/author/DOY?join=release',
-            Content => encode_json( {
-                query => {
-                    constant_score =>
-                        { filter => { term => { status => 'latest' } } }
-                }
-            } )
-        ),
-        'POST /author/DOY?join=release with query body',
-    );
-
-    $json = decode_json_ok($res);
-    is( @{ $json->{release}->{hits}->{hits} }, 1, 'joined 1 release' );
-    is( $json->{release}->{hits}->{hits}->[0]->{_source}->{status},
-        'latest', '1 release has status latest' );
-
-    ok(
-        $res = $cb->(
-            POST '/author/_search?join=release',
-            Content => encode_json( {
-                query => {
-                    constant_score => {
-                        filter => {
-                            bool => {
-                                should => [
-                                    {
-                                        term => {
-                                            'status' => 'latest'
-                                        }
-                                    },
-                                    {
-                                        term => { 'pauseid' => 'DOY' }
-                                    }
-                                ]
-                            }
-                        }
-                    }
-                }
-            } )
-        ),
-        'POST /author/_search?join=release with query body'
-    );
-
-    my $doy = $json;
-    $json = decode_json_ok($res);
-
-    is( @{ $json->{hits}->{hits} }, 1, '1 hit' );
-
-    my $release_count = delete $doy->{release_count};
-    is_deeply(
-        [ sort keys %{$release_count} ],
-        [qw< backpan-only cpan latest >],
-        'release_count has the correct keys'
-    );
-
-    my $links = delete $doy->{links};
+    my $links = $doy->{links};
     is_deeply(
         [ sort keys %{$links} ],
         [
@@ -154,9 +94,6 @@ test_psgi app, sub {
         'links has the correct keys'
     );
 
-    my $source = $json->{hits}->{hits}->[0]->{_source};
-    is_deeply( $doy, $source, 'same result as direct get' );
-
     {
         ok( my $res = $cb->( GET '/author/_search?q=*&size=99999' ),
             'GET size=99999' );
