Introduce ResultQueryFetchStream Refaster rule #501
Conversation
maxsumrall
force-pushed
the
maxsumrall/resultQueryFetchStream
branch
from
1489ec5
to
56ed83b
Compare
|
rebased on |
|
Looks good. No mutations were possible for these changes. |
1 similar comment
|
Looks good. No mutations were possible for these changes. |
| <dependency> | ||
| <groupId>org.jooq</groupId> | ||
| <artifactId>jooq</artifactId> | ||
| <version>3.16.14</version> |
There was a problem hiding this comment.
This is the latest version of jOOQ supporting Java 11. From 3.17.x, it is not supported.
| @@ -1178,6 +1183,8 @@ | |||
| | BSD 3-clause | |||
| | BSD License 3 | |||
| | Eclipse Distribution License (New BSD License) | |||
| | Eclipse Distribution License - v 1.0 | |||
There was a problem hiding this comment.
Source:
[WARNING] License: 'Eclipse Distribution License - v 1.0' used by 1 dependencies:
-Jakarta XML Binding API (jakarta.xml.bind:jakarta.xml.bind-api:3.0.0 - https://github.com/eclipse-ee4j/jaxb-api/jakarta.xml.bind-api)
| @@ -1178,6 +1183,8 @@ | |||
| | BSD 3-clause | |||
| | BSD License 3 | |||
| | Eclipse Distribution License (New BSD License) | |||
| | Eclipse Distribution License - v 1.0 | |||
| | EDL 1.0 | |||
There was a problem hiding this comment.
Source:
[WARNING] License: 'EDL 1.0' used by 1 dependencies:
-Jakarta Activation (com.sun.activation:jakarta.activation:2.0.0 - https://github.com/eclipse-ee4j/jaf/jakarta.activation)
There was a problem hiding this comment.
Very useful rule!
We can apply the same to the synonym fetchStream() and almost the same for the equally dangerous fetchStreamInto(...) variants.
A less opinionated version of this rule may be to only make this replacement when the .stream() is invoked outside of a try_with_resources block.
That would certainly reduce false positives 👍🏼
|
|
||
| /** Prefer eagerly fetching data over (lazy) streaming to avoid potentially leaking resources. */ | ||
| @Description( | ||
| "Prefer eagerly fetching data over (lazy) streaming to avoid potentially leaking resources.") |
There was a problem hiding this comment.
Would it be useful to link to the Jooq blogpost here?
There was a problem hiding this comment.
While it would be useful, it might not be wise as the blogpost might move URLs one day, and then the code is not correct. I think that would not be prudent in a library such as this. Perhaps linking to it in this PR is enough for curious folks to find one day if they go looking for the motivations for this rule.
If that reduces the false positive rate I think it'd be smart to introduce a |
Thanks for the tip! I'll give it a try! |
When using the jOOQ SQL library for querying a database, you can either fetch the result set of your query into memory and proceed with application code, or you can return a lazy stream from the database and process the results as a stream. See here for examples and explanations of fetching with jOOQ. A common issue with the
.stream()approach is that it is usually used unintentionally. This is dangerous as the stream returned here is not managed and not auto-closed, leaving applications potentially at risk to resource leaking.A less opinionated version of this rule may be to only make this replacement when the
.stream()is invoked outside of a try_with_resources block.