NewPipeExtractor icon indicating copy to clipboard operation
NewPipeExtractor copied to clipboard

Use TreeMap in Response

Open Isira-Seneviratne opened this issue 1 year ago • 2 comments


Store the response headers in a TreeMap. This allows the header value retrieval to be optimised as TreeMap orders its keys using a comparator.

Isira-Seneviratne avatar Jul 20 '24 07:07 Isira-Seneviratne

Good catch. iterating through the map elements does not make any sense. Using the default get / getOrDefault method is the proper way to go :+1:

However, I do not understand why we should specifically use a TreeMap. It guarantees a look-up cost of log(n). Same for put operations. However, the usually used HashMap has O(1) if the bucket size is good and not too many items are in the Map. There shouldn't be hundreds of elements in the map, but rather 25-50. So I think using passed map is more efficient than creating a new Map which needs to process all elements first. This is unnecessary pre-processing and comes with additional computation time and requires additional space.

TobiGr avatar Jul 20 '24 11:07 TobiGr

I've revised the code a bit, to reuse the map if it is already a SortedMap. That way, no extra objects need to be created in that scenario, such as with OkHttp's header map implementation.

Isira-Seneviratne avatar Jul 20 '24 11:07 Isira-Seneviratne

A related test is failing, something is wrong with the changes

Invalid response type (got "null", excepted a JSON response) (response code 200)
org.schabi.newpipe.extractor.exceptions.ExtractionException: Invalid response type (got "null", excepted a JSON response) (response code 200)
	at org.schabi.newpipe.extractor.services.youtube.extractors.YoutubeSuggestionExtractor.suggestionList(YoutubeSuggestionExtractor.java:71)
	at org.schabi.newpipe.extractor.services.youtube.YoutubeSuggestionExtractorTest.testIfSuggestions(YoutubeSuggestionExtractorTest.java:55)

I wasn't able to reproduce the issue locally, the header value is being returned as expected.

Isira-Seneviratne avatar Mar 19 '25 02:03 Isira-Seneviratne

The Exception is only thrown when running with mocks and happens here: https://github.com/TeamNewPipe/NewPipeExtractor/blob/0b99100dbddeca2f27554620378f251afd66b30b/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeSuggestionExtractor.java#L67-L72

The mock response has the header content-type while the code expects Content-Type. Both use a TreeMap. The one used in the normal Downloader is case insensitive and the mock one is not. Recording the mock again did not fix the issue. This might be a serialization issue in which the gson builder does something unexpected in MockDownloader.

TobiGr avatar Mar 19 '25 08:03 TobiGr