Use TreeMap in Response
- [x] I carefully read the contribution guidelines and agree to them.
- [x] I have tested the API against NewPipe.
Store the response headers in a TreeMap. This allows the header value retrieval to be optimised as TreeMap orders its keys using a comparator.
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.
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.
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.
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.