Rework content and sort filter framework #904

evermind-zz · 2022-08-19T13:37:03Z

I carefully read the contribution guidelines and agree to them.
I have tested the API against NewPipe.
I agree to create a pull request for NewPipe as soon as possible to make it compatible with the changed API.

Sort filters reworked

framework for creating filter items
implemented sort filters for each service
reworked content filters

Closes:

YouTube search filters #106

Superseeds

Youtube search filters #124

Questions

Q: I've added singletons for some *QueryHandlerFactory as we rely now on the same object sets during a search. Was there a good reason to always recreate those objects?
A: Rework content and sort filter framework #904 (review)

opusforlife2 · 2022-08-19T18:56:09Z

Holy shit it's happening

FireMasterK · 2022-08-25T19:46:03Z

I like the approach of using proto files with wire! Any reason why these classes are in git and not auto-generated, perhaps with a gradle plugin? See: https://github.com/square/wire/blob/master/wire-library/docs/wire_compiler.md

evermind-zz · 2022-08-25T21:24:48Z

I like the approach of using proto files with wire! Any reason why these classes are in git and not auto-generated, perhaps with a gradle plugin? See: https://github.com/square/wire/blob/master/wire-library/docs/wire_compiler.md

They are not in git. They will be auto-generated by the wire gradle plugin. Why do you think they are?

FireMasterK · 2022-08-26T04:32:11Z

I'm having a look at the Filter, FilterGroup, FilterItem, SearchFiltersBase, etc classes. They look auto-generated to me, are they not?

evermind-zz · 2022-08-26T04:50:09Z

Am 26. August 2022 06:32:22 MESZ schrieb Kavin ***@***.***>:

I'm having a look at the `Filter`, `FilterGroup`, `FilterItem`, `SearchFiltersBase`, etc classes. They look auto-generated to me, are they not?

Nope as only YouTube uses proto buffer for their parameter. The classes you mentioned are for describing the sort and content filters within each service. They abstract the differences. I'll convert Filter* to interfaces as they are.

FireMasterK

Looks good for the most part, I just had a look at the general changes and YouTube-specific changes.

The code currently fails checkstyle, which is why tests cannot currently be run. Two tests appear to be commented out, but that's a minor issue and can be fixed easily.

TobiGr

Thank you for implementing this long awaited feature.
I briefly skimmed over the most important code changes.
I noticed that there are no tests for the newly added classes and methods yet.

I've added singletons for some *QueryHandlerFactory as we rely now on the same object sets during a search. Was there a good reason to always recreate those objects?

IIRC some of the link handlers were not stateless in the beginning. But I think they are stateless now. So converting those into singletons should be fine.

extractor/src/main/java/org/schabi/newpipe/extractor/search/filter/SearchFiltersBase.java

...est/java/org/schabi/newpipe/extractor/services/soundcloud/search/SoundcloudSearchQHTest.java

.../src/test/java/org/schabi/newpipe/extractor/services/youtube/search/YoutubeSearchQHTest.java

extractor/src/main/java/org/schabi/newpipe/extractor/search/filter/Filter.java

extractor/src/main/java/org/schabi/newpipe/extractor/linkhandler/ListLinkHandler.java

extractor/src/main/java/org/schabi/newpipe/extractor/linkhandler/SearchQueryHandlerFactory.java

extractor/src/main/java/org/schabi/newpipe/extractor/linkhandler/ListLinkHandlerFactory.java

extractor/src/main/java/org/schabi/newpipe/extractor/services/peertube/PeertubeHelpers.java

...a/org/schabi/newpipe/extractor/services/soundcloud/extractors/SoundcloudSearchExtractor.java

...chabi/newpipe/extractor/services/peertube/linkHandler/PeertubeSearchQueryHandlerFactory.java

...i/newpipe/extractor/services/soundcloud/linkHandler/SoundcloudSearchQueryHandlerFactory.java

...pipe/extractor/services/youtube/search/filter/YoutubeProtoBufferSearchParameterAccessor.java

extractor/src/main/java/org/schabi/newpipe/extractor/linkhandler/ListLinkHandlerFactory.java

Isira-Seneviratne · 2022-11-12T12:08:29Z

@evermind-zz I went through your changes a bit, and they look good to me overall. I've suggested a few changes; for some of these (the Utils encode and decode methods), a rebase will be needed.

… within the searchfilters The class LibraryStringIds is a enum. This enum's identify a message. The name of the enum constants reflect the actual message. This message has to be transformed by the library user into a meaningful string.

…onnull

…nnull Annotate methods in classes that got some signatures changed due to the searchfilters integration

DividerItem was inserted in the content filter framework in the NewPipeExtractor to have a section title for YoutubeMusic. But as UI releated stuff seems a bit out of place in the Extractor I came up with injecting the DividerItem aka section title in the frontend without having to change too much in the frontend.

Stypox

Here you are. I finished rebasing, but left some fixup commits for clearness. Those will need to either be squashed or get a better commit description. I also kind of reviewed the code, but I will need to review it more.

Now there are only a couple of tests to fix. I pushed a commit to update mocks just so that tests don't fail for that reason. The first two failures in the image are caused by the emergency info filter not being used for YouTube search, while the last one is for PeerTube endpoints not being used anymore.

Stypox · 2023-12-30T09:07:16Z

.../schabi/newpipe/extractor/services/youtube/linkHandler/YoutubeSearchQueryHandlerFactory.java

-    private static final String MUSIC_SEARCH_URL = "https://music.youtube.com/search?q=";
+    private YoutubeSearchQueryHandlerFactory() {
+        super(new YoutubeFilters());
+    }

    @Nonnull
    public static YoutubeSearchQueryHandlerFactory getInstance() {


I saw that the creation of link handler factory instances was being done with a lazy getInstance, before the rebase. Since in the meantime we also added getInstance in the current extractor, I kept the current version to reduce changes, and thus discarded the lazy initialization. However we might want to check if it's better to lazily initialize all link handler factories to reduce static initialization time (but this can be done in a separate PR).

Stypox · 2023-12-30T09:16:34Z

extractor/src/main/java/org/schabi/newpipe/extractor/linkhandler/ListLinkHandlerFactory.java

+                         final List<FilterItem> contentFilter,
+                         final List<FilterItem> sortFilter,


Some other @Nonnull and @Nullable annotations are missing in this file (and all derivate classes)

Stypox · 2023-12-30T09:17:29Z

extractor/src/main/java/org/schabi/newpipe/extractor/linkhandler/ListLinkHandlerFactory.java

+                                  @Nonnull List<FilterItem> contentFilter,
+                                  @Nullable List<FilterItem> sortFilter)


Why @Nonnull content filter but @Nullable sort filter?

Stypox · 2023-12-30T09:17:41Z

extractor/src/main/java/org/schabi/newpipe/extractor/linkhandler/ListLinkHandlerFactory.java

+                                  @Nonnull List<FilterItem> contentFilter,
+                                  @Nullable List<FilterItem> sortFilter)


Shouldn't we just use one array with FilterItem and stop making the distinction between content and sort filters? What is the advantage of having two separate arrays?

a content filter might have a different set of sort filters.

But there can already be content filters from various available filter groups, so why can't sort filters be just another filter group? Maybe I'm missing something

I'll have to review my code again. I think I thought about that.

Stypox · 2023-12-30T09:22:02Z

...chabi/newpipe/extractor/services/peertube/linkHandler/PeertubeSearchQueryHandlerFactory.java

-    public static final String SEARCH_ENDPOINT_PLAYLISTS = "/api/v1/search/video-playlists";
-    public static final String SEARCH_ENDPOINT_VIDEOS = "/api/v1/search/videos";
-    public static final String SEARCH_ENDPOINT_CHANNELS = "/api/v1/search/video-channels";


I think this broke PeerTube filters. In theory using resultType=videos/playlists/channels should work, however it doesn't seem to, see query with resultType and query with different endpoints. PeerTube on the web uses the endpoints, too.

Fixed with 70a2b63

Stypox · 2023-12-30T09:24:30Z

.../schabi/newpipe/extractor/services/youtube/linkHandler/YoutubeSearchQueryHandlerFactory.java

-                case MUSIC_ARTISTS:
-                    return "";
-                default:
-                    return "8AEB";


We need to update the protobuf file in order to include the "allow emergency requests" field, and then always set that field to true. I didn't do this while rebasing because I have no idea how the protobuf file is obtained.

could you point me more information about the emergency requests? Where is it handled in eg. the browser youtube client.

evermind-zz · 2023-12-30T10:29:38Z

@AudricV

I will h

Here you are. I finished rebasing, but left some fixup commits for clearness. Those will need to either be squashed or get a better commit description. I also kind of reviewed the code, but I will need to review it more.

Now there are only a couple of tests to fix. I pushed a commit to update mocks just so that tests don't fail for that reason. The first two failures in the image are caused by the emergency info filter not being used for YouTube search, while the last one is for PeerTube endpoints not being used anymore.

thx for rebasing. I might also have some fixes/changes in my 'BraveNewPipe extractor fork. I'll going to look into it.

…laylists search

'ALL' option not possible as Peertube uses different endpoints for video/channels/playlist search

…ontent filter search

…able encountered IOException The following Exception crashes NewPipe. Adding Serializable to FilterItem solves it: java.lang.RuntimeException: Parcelable encountered IOException writing serializable object (name = org.schabi.newpipe.extractor.linkhandler.ListLinkHandler) ... Caused by: java.io.NotSerializableException: org.schabi.newpipe.extractor.search.filter.FilterItem

Isira-Seneviratne · 2024-01-04T03:17:20Z

Base64 support has been added to the desugar library, so we'll be able to remove the uses of ByteString from this PR.

Isira-Seneviratne · 2024-01-04T04:10:32Z

extractor/src/main/java/org/schabi/newpipe/extractor/search/filter/FilterContainer.java

+
+    public FilterContainer(@Nonnull final FilterGroup[] filterGroups) {
+        this.filterGroups =
+                Arrays.stream(filterGroups).collect(Collectors.toCollection(ArrayList::new));


Suggested change

Arrays.stream(filterGroups).collect(Collectors.toCollection(ArrayList::new));

Arrays.asList(filterGroups);

extractor/src/main/java/org/schabi/newpipe/extractor/search/filter/FilterGroup.java

evermind-zz · 2024-01-25T11:32:14Z

At the moment I have not enough time to look into it. But I will do it whenever I have some time for it.

AudricV · 2024-03-14T05:46:13Z

could you point me more information about the emergency requests? Where is it handled in eg. the browser youtube client.

@evermind-zz As I can't anwser you as a comment as I have a pending review of your PR (in which I also explained you where to implement this property), see #1127.

I will try to finish the review soon, but I can't promise you anything.

evermind-zz mentioned this pull request Aug 19, 2022

Add support for advanced content and sort filters in searches TeamNewPipe/NewPipe#8837

Open

5 tasks

opusforlife2 mentioned this pull request Aug 19, 2022

[HELP NEEDED] Major planned/missing features TeamNewPipe/NewPipe#6448

Open

34 tasks

FireMasterK mentioned this pull request Aug 27, 2022

more filters TeamPiped/Piped#1336

Closed

3 tasks

FireMasterK requested changes Aug 27, 2022

View reviewed changes

TobiGr reviewed Aug 27, 2022

View reviewed changes

evermind-zz marked this pull request as draft August 27, 2022 13:03

evermind-zz force-pushed the content-sort-filter-framework branch 2 times, most recently from 4a7a7bb to 4c420fb Compare October 12, 2022 00:05

evermind-zz marked this pull request as ready for review October 12, 2022 00:21

evermind-zz requested review from TobiGr and FireMasterK and removed request for TobiGr and FireMasterK October 12, 2022 00:21

evermind-zz force-pushed the content-sort-filter-framework branch from 4c420fb to b09eda4 Compare October 20, 2022 22:26

FireMasterK mentioned this pull request Oct 22, 2022

Sort by date TeamPiped/Piped#275

Closed

evermind-zz force-pushed the content-sort-filter-framework branch from b09eda4 to bb97de7 Compare November 4, 2022 17:44

evermind-zz requested review from FireMasterK and TobiGr and removed request for TobiGr and FireMasterK November 4, 2022 22:40

Isira-Seneviratne requested changes Nov 12, 2022

View reviewed changes

evermind-zz and others added 12 commits December 30, 2023 10:31

searchfilters: Test: adjust MediaCCC tests

1fc0fc7

searchfilters: Test: adjust Bandcamp tests

019ef62

searchfilters: Test: adjust and extend YouTube tests

8280fd4

Fix yt tests

3e68e17

searchfilters: unit tests for all derived BaseSearchFilters classes

5a02063

searchfilters: convert to using LibraryStringIds to identify strings

78c8a60

searchfilters: annotate methods and their parameters as Nullable or N…

11d268f

…onnull

searchfilters: annotate changed method signatures with Nullable or No…

4210592

…nnull Annotate methods in classes that got some signatures changed due to the searchfilters integration

searchfilters: use base64 methods from okio to support <= Android Oreo

0985afa

[Youtube] Update search mocks

e81796f

Stypox requested changes Dec 30, 2023

View reviewed changes

Stypox force-pushed the content-sort-filter-framework branch from b938a3c to e81796f Compare December 30, 2023 09:33

evermind-zz added 5 commits December 30, 2023 14:59

searchfilters: Peertube: handle different endpoints for channel and p…

23fe41d

…laylists search

searchfilters: Peertube: remove search option ID_CF_MAIN_ALL

659deb0

'ALL' option not possible as Peertube uses different endpoints for video/channels/playlist search

searchfilters: bump squareup wire version to 4.5.2

dd252d3

searchfilters: Peertube restore different endpoints to fix peertube c…

70a2b63

…ontent filter search

Isira-Seneviratne reviewed Jan 4, 2024

View reviewed changes

extractor/src/main/java/org/schabi/newpipe/extractor/search/filter/FilterGroup.java Show resolved Hide resolved

opusforlife2 mentioned this pull request Mar 26, 2024

[Feature Request] Add filter option to ChannelInfo.getInfo (to get top videos by channel) #270

Open

Bnyro mentioned this pull request May 17, 2024

Enhanced search filters libre-tube/LibreTube#6049

Closed

3 tasks

HJX-001 approved these changes Jun 8, 2024

View reviewed changes

This comment was marked as resolved.

Sign in to view

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework content and sort filter framework #904

Rework content and sort filter framework #904

evermind-zz commented Aug 19, 2022 •

edited

Loading

opusforlife2 commented Aug 19, 2022

FireMasterK commented Aug 25, 2022

evermind-zz commented Aug 25, 2022

FireMasterK commented Aug 26, 2022

evermind-zz commented Aug 26, 2022 via email •

edited

Loading

FireMasterK left a comment

TobiGr left a comment

Isira-Seneviratne commented Nov 12, 2022

Stypox left a comment

Stypox Dec 30, 2023

Stypox Dec 30, 2023

Stypox Dec 30, 2023

Stypox Dec 30, 2023

evermind-zz Dec 30, 2023

Stypox Dec 30, 2023

evermind-zz Dec 30, 2023

Stypox Dec 30, 2023

evermind-zz Jan 4, 2024

Stypox Dec 30, 2023

evermind-zz Mar 13, 2024

evermind-zz commented Dec 30, 2023

Isira-Seneviratne commented Jan 4, 2024 •

edited

Loading

Isira-Seneviratne Jan 4, 2024 •

edited

Loading

evermind-zz commented Jan 25, 2024

AudricV commented Mar 14, 2024

This comment was marked as resolved.

This comment was marked as resolved.

		final List<FilterItem> contentFilter,
		final List<FilterItem> sortFilter,

		@Nonnull List<FilterItem> contentFilter,
		@Nullable List<FilterItem> sortFilter)

	Arrays.stream(filterGroups).collect(Collectors.toCollection(ArrayList::new));
	Arrays.asList(filterGroups);

Rework content and sort filter framework #904

Are you sure you want to change the base?

Rework content and sort filter framework #904

Conversation

evermind-zz commented Aug 19, 2022 • edited Loading

opusforlife2 commented Aug 19, 2022

FireMasterK commented Aug 25, 2022

evermind-zz commented Aug 25, 2022

FireMasterK commented Aug 26, 2022

evermind-zz commented Aug 26, 2022 via email • edited Loading

FireMasterK left a comment

Choose a reason for hiding this comment

TobiGr left a comment

Choose a reason for hiding this comment

Isira-Seneviratne commented Nov 12, 2022

Stypox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evermind-zz commented Dec 30, 2023

Isira-Seneviratne commented Jan 4, 2024 • edited Loading

Isira-Seneviratne Jan 4, 2024 • edited Loading

Choose a reason for hiding this comment

evermind-zz commented Jan 25, 2024

AudricV commented Mar 14, 2024

This comment was marked as resolved.

This comment was marked as resolved.

evermind-zz commented Aug 19, 2022 •

edited

Loading

evermind-zz commented Aug 26, 2022 via email •

edited

Loading

Isira-Seneviratne commented Jan 4, 2024 •

edited

Loading

Isira-Seneviratne Jan 4, 2024 •

edited

Loading