MBS-13963: Add web service support for browsing events by event(s)

[mwiencek]

Problem

MBS-13963

Solution

• First, add support for browsing events by another event (linked via relationships).
• Next, add support for specifying multiple event MBID parameters (up to 25) in the browse query.
• Finally, make the event browsing query recursive.

We can probably generalize this feature to other types of browse queries too, but let's do a trial implementation for events to start with.

Testing

Added some automated tests.

[reosarevok]

I expect this would be a lot better if it was recursive (to get stuff like Festival -> Day -> Stage) but it's probably quite a bit better than before already to do one query for Festival -> Day and then one multi-event query for Days -> Stages, and I guess it's more easily generalized later for other entities where we might not want recursion.

[reosarevok]

Fixed a perlcritic error here, this needs to be quoted to specify if we want interpolation or not. We generally put the query under the whole call, not in the middle of it, but I guess this could be more clear, so fine by me if you prefer it.

[mwiencek]

IIRC we don't have a line length rule for Perl, but I was only doing that to keep it under 80 chars. Even this doesn't work:

    $self->query_to_list_limited(
        <<~"SQL", [$event_ids], $limit, $offset, undef, dollar_placeholders => 1); // exceeds 80 chars

We could also assign the query to a variable first, but putting it in the middle looks fine to me.

[mwiencek]

I expect this would be a lot better if it was recursive (to get stuff like Festival -> Day -> Stage) but it's probably quite a bit better than before already to do one query for Festival -> Day and then one multi-event query for Days -> Stages, and I guess it's more easily generalized later for other entities where we might not want recursion.

Do you mean making the query in find_by_events recursive? I hadn't considered that, but we could try it and see how it performs. I'm not sure how consistent that is with other browse-by-relationship endpoints, though, and if we do that then I'm not sure if we should keep the multiple-event-MBID-parameter support at all. I'd have to check the performance of the paginated recursive query across 50 events in the production DB.

[reosarevok]

I did mean that, but if @ReIaxo is happy enough with this then this seems more consistent for other entities later.

[ReIaxo]

I am happy enough. Recursion is some kind of dangerous here if there are endless loops. So you have better control from client site and with the allowance for burst requests it is totally fine to mage up to three requests per Event.

The only thing to consider: Do I have to lookup the last level (Main->Day->STAGE) to get full information or is it included as the rels from level 2 (Day)? For the former 50 as a limit might be to small. Wacken with ~10 stages and 7 days = 70 events on stage level. I think 100 can be considered a maximum. If we have an event bigger then that, we can split up the request.

[sanojjonas]

@ReIaxo i think the limit of 50 is how many you can look up at once. so i guess for wacken it would be:
request 1: main event (1 item) -> this will respond with 1 item that has 7 day relations
request 2: day events (7 items) -> this will respond with 7 items that have 10 stage relations
request 3: stage events day 1-5 (50 items) -> this will respond with 50 items that have band relations
request 4: stage events day 6 & 7 (20 items) -> this will respond with 20 items that have band relations

the change that is now propsed should already decrease the wait time from 1 + 7 + 70 = 78 seconds down to 4.
if the limit would change to 100 then that would only make it 1 second faster. to compare that with the 74 seconds it now already is faster.

the only place where i think it maybe make a biggere difference is glastonbury with it 5 days and 100 staged per day but i don't think anybody actually added a full edition of glastonbury yet...

[mwiencek]

@ReIaxo @sanojjonas I've updated the PR to use a recursive query. So now you'll only need two requests, one to get the top-level event (if needed), and another to browse all sub-events by the top-level event MBID.

Browsing by multiple event MBIDs is still supported. So, for example, you can browse all sub-events for multiple festivals at once (up to 50). The results will be paginated, of course, using the limit/offset parameters (the maximum limit is 100, IIRC).

[mwiencek]

Added some more test cases for browsing events containing a cycle in l_event_event.

There are also some Autocomplete2 commits included here which attempt to resolve some persistent Selenium failures I was getting frustrated with.

[mwiencek]

you can browse all sub-events for multiple festivals at once (up to 50)

I didn't observe any performance issues with 50 event IDs in find_by_event. The SQL queries executed in under 10ms on our PG standby. But I lowered the maximum number of allowed event MBID parameters to 25, just to be safe, since it will be much harder to reduce this limit if needed (potentially breaking peoples' code) than to increase it. Also, now that the browsing is recursive, there should be much less of a need to specify so many parameters at once.

[ReIaxo]

Looks good. When I request multiple events, then each one will be returned with all sub events? Is a flat array returned or a tree?

[mwiencek]

Looks good. When I request multiple events, then each one will be returned with all sub events? Is a flat array returned or a tree?

Currently, the browse endpoint doesn't return the events you request, only the events linked to them (directly or indirectly). That's consistent with how other browse queries work, although there's actually no prior precedent for browsing an entity type by the same entity type. So it might make sense to return the source entities too in that case.

All linked events are returned as a flat list, but if you specify inc=event-rels, then you can determine the hierarchy from the relationships.

[ReIaxo]

Okay. Will be fine. Thank you.

[sanojjonas]

does this mean that i can request up to 25 festivals with one request, that i will get a response with in there all the day/stage events of the 25 festivals. that i then have to page through them at 100 a time?

[sanojjonas]

So it might make sense to return the source entities too in that case.

maybe when you add an extra parameter to include the browseItem aswel. something like inc=browse-item ofzo?
then if you would doe a normal request you would get it like it is now, and stuff don't magically add extra stuff for existing stuff.

it would change the lookup & browse change in to only browse stuff.

but maybe that needs to be added via a different pull request so this can already be tested?

(i hope this isn't all to vague with all the stuff)

[sanojjonas]

one question i have. will this also function when i for instance browse all events for place or a series?
will then the response also include the sub events?

[mwiencek]

@sanojjonas:

does this mean that i can request up to 25 festivals with one request, that i will get a response with in there all the day/stage events of the 25 festivals. that i then have to page through them at 100 a time?

Correct.

maybe when you add an extra parameter to include the browseItem aswel. something like inc=browse-item ofzo?

I'd rather always include them (when the entity types are the same) if they're useful, but I agree we should test the current implementation first.

one question i have. will this also function when i for instance browse all events for place or a series? will then the response also include the sub events?

No, this feature only works when browsing events by other events.

[sanojjonas]

oke, last question, when can i try this?

[mwiencek]

oke, last question, when can i try this?

It's now deployed to https://test.musicbrainz.org/ :)

But the PR needs an approval first, then we can deploy it to the beta server where it can be tested against live data.

[sanojjonas]

https://test.musicbrainz.org/ws/2/event?event=afa8f716-18ee-48b8-8c1f-2649f0024033&event=a52a6938-bd5b-463b-9079-d812ccb89334&offset=0&limit=100&inc=aliases+artist-rels+event-rels&fmt=json

nice, seems like its working 2 festivals, 51 events with one request

who needs to approve? so i can test and adjust my userscript on the beta server?

[sanojjonas]

@mwiencek any news on the status of this PR?

[reosarevok]

We were all having our in-person meeting last week and a lot of the team is traveling for a while longer. I'll try to review this further this week, but until everyone is back we probably won't merge it, so maybe a couple weeks more if all is good?

[mwiencek]

I'll be traveling until the end of the month, but should have time to update it if only small changes are needed.

[reosarevok]

Looks good, tested a bit locally and it seemed to work nicely too. Let's get it out on beta and let people with apps which would use this test it further?