DataAccessUtils result accessors with Optional return value#27735
DataAccessUtils result accessors with Optional return value#27735jhoeller merged 11 commits intospring-projects:mainfrom
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
|
Using the * form of import should be avoided - java.util.*. |
rstoyanchev
added
in: data
rstoyanchev
changed the title
rstoyanchev
removed
the
status: waiting-for-triage
| if (results == null) { | ||
| return null; | ||
| } | ||
| List<T> resultList = results.toList(); |
There was a problem hiding this comment.
Wouldn't it be better to do a
.limit(2).toList()
and then avoid opening the second stream and instead do
resultList.get(0)
There was a problem hiding this comment.
Hi @marschall,
thanks for reviewing, sounds good, i refactored this. Does it make more sense now?
throw new IncorrectResultSizeDataAccessException(1, resultList.size());
as we limited the resultList size with .limit, this would not report the original passed Stream results size though but instead a maximum of 2... is that an issue?
There was a problem hiding this comment.
Yes, looks better to me.
| return null; | ||
| } | ||
| Iterable<T> iterable = () -> results; | ||
| List<T> resultList = StreamSupport.stream(iterable.spliterator(), false).toList(); |
There was a problem hiding this comment.
Why a stream instead of just
results.next()
Thanks for the tip, i adjusted that. |
| if (results == null) { | ||
| return null; | ||
| } | ||
| List<T> resultList = results.limit(2).toList(); |
There was a problem hiding this comment.
There is a trade off here and in #optionalResult whether we should call #close on the stream:
On one hand Java convention is the code who creates a "resource" is responsible for disposing it, in this case the caller of this method would be responsible to call #close on the streams where required.
On the other hand failing to call #close on some streams, like the ones returned by JdbcOperations#queryForStream, can lead to resource leaks. Having the caller put streams in a try-with-resources block is both cumbersome and error prone.
There was a problem hiding this comment.
Makes sense. The passed stream is now closed in both methods. Also e.g. hibernate's Query.stream() documentation suggests the stream should be closed afterwards, so not so far fetched 👍
spring-tx/src/main/java/org/springframework/dao/support/DataAccessUtils.java
Outdated
Show resolved
Hide resolved
spring-tx/src/main/java/org/springframework/dao/support/DataAccessUtils.java
Outdated
Show resolved
Hide resolved
spring-tx/src/main/java/org/springframework/dao/support/DataAccessUtils.java
Outdated
Show resolved
Hide resolved
rkvigneswaran
left a comment
rkvigneswaran
left a comment
There was a problem hiding this comment.
Hi,
This is my first review comments in any open source project. Kindly let me know if my comments are invalid.
…cessUtils.java re-use newly added singleResult methods Co-authored-by: Vigneswaran <rkvigneswaran@gmail.com>
…cessUtils.java re-use newly added singleResult methods Co-authored-by: Vigneswaran <rkvigneswaran@gmail.com>
…cessUtils.java re-use newly added singleResult methods Co-authored-by: Vigneswaran <rkvigneswaran@gmail.com>
spring-tx/src/main/java/org/springframework/dao/support/DataAccessUtils.java
Outdated
Show resolved
Hide resolved
spring-tx/src/main/java/org/springframework/dao/support/DataAccessUtils.java
Outdated
Show resolved
Hide resolved
rkvigneswaran
left a comment
rkvigneswaran
left a comment
There was a problem hiding this comment.
Modifying the exception to make it clear that size of the underlying Stream/Iterator is not known
…cessUtils.java use "public IncorrectResultSizeDataAccessException(int expectedSize)" in stream and iterator method Co-authored-by: Kwangyong Kim <banana.yong@gmail.com>
…cessUtils.java use "public IncorrectResultSizeDataAccessException(int expectedSize)" in stream and iterator method Co-authored-by: Vigneswaran <rkvigneswaran@gmail.com>
8 participants
Closes #27728
(#27728)
Hi everyone, this is my first PR in any project here on github with the goal of improving my understanding of java. Hope i understood the enhancement request correctly.