Skip to content

DownloaderImpl: Auto-close resources and simplify headers setting#11969

Merged
Profpatsch merged 1 commit intoTeamNewPipe:devfrom
neosis91:dev
Jan 31, 2025
Merged

DownloaderImpl: Auto-close resources and simplify headers setting#11969
Profpatsch merged 1 commit intoTeamNewPipe:devfrom
neosis91:dev

Conversation

@neosis91
Copy link
Contributor

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

  • Optimized the execute method to improve readability and resource management.
  • Used try-with-resources to ensure automatic closing of the Response object.
  • Simplified HTTP header management with a more concise approach.
  • Introduced Optional to handle request bodies and responses, reducing null checks.

Due diligence

@github-actions github-actions bot added the size/medium PRs with less than 250 changed lines label Jan 28, 2025
@ShareASmile ShareASmile added feature request Issue is related to a feature in the app codequality Improvements to the codebase to improve the code quality labels Jan 31, 2025
The headers should be overwritten in the same way, based on how
`.header` is the same as `.removeHeader().addHeader()`.

We weren’t closing the request resources after using them, potentially
leaking file handles. This will add autoclosing for both the request
and the body objects.
@Profpatsch
Copy link
Contributor

Good catch, we need to close the Request.

I also added a try block for the body reader, which should also be closed.

@Profpatsch Profpatsch changed the title optimize execute method DownloaderImpl: Auto-close resources and simplify header setting Jan 31, 2025
@Profpatsch
Copy link
Contributor

Using Optional for simple null handling is a little silly in my opinion, especially cause we can convert to kotlin in the future.

@sonarqubecloud
Copy link

@Profpatsch Profpatsch added size/small PRs with less than 50 changed lines and removed feature request Issue is related to a feature in the app size/medium PRs with less than 250 changed lines labels Jan 31, 2025
@AudricV AudricV added the bug Issue is related to a bug label Jan 31, 2025
@Profpatsch Profpatsch merged commit ba86ce1 into TeamNewPipe:dev Jan 31, 2025
8 checks passed
@AudricV AudricV changed the title DownloaderImpl: Auto-close resources and simplify header setting DownloaderImpl: Auto-close resources and simplify headers setting Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issue is related to a bug codequality Improvements to the codebase to improve the code quality size/small PRs with less than 50 changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants