EVA-4110 upgrade project to Java 21 - spring 3.4#117
Conversation
nitin-ebi
force-pushed
the
java-21-upgrade
branch
2 times, most recently
from
e7c04d9 to
1c270dc
Compare
| * | ||
| * @param query JSON formatted Mongo query | ||
| */ | ||
| public void setQuery(String query) { |
There was a problem hiding this comment.
Removed the support for String queries. Will have to see if this breaks anything.
If the query is simple enough, we should convert that to a Java/Python query object and use that instead of a string.
Earlier, we had a class from Mongo that was doing the serialization, but that has been removed. So we would have to write something of our own, which is not complicated, but we are trying to do something that is the responsibility of Mongo. Better to avoid if we can
| import java.util.Objects; | ||
|
|
||
| public class MongoUtils { | ||
| public static MongoClientURI constructMongoClientURI(String host, Integer port, String databaseName, String userName, |
There was a problem hiding this comment.
Class MongoClientURI has been removed and replaced by ConnectionString, so I had to make a few changes accordingly.
|
|
||
| @Override | ||
| public void doWrite(List<? extends IVariant> variants) { | ||
| public void doWrite(Chunk<? extends IVariant> variants) { |
There was a problem hiding this comment.
MongoItemWriter's write method now takes a Chunk instead of a List of items.
This will have a ripple effect, as I assume this is a well-used function.
We can also maybe write an overloaded method that takes a list of items and internally calls this method by converting them to a Chunk. This way, the code in other repos that uses this method can remain as is.
| private Stream<? extends T> cursor; | ||
|
|
||
| private CloseableIterator<? extends T> cursor; | ||
| private Iterator<? extends T> cursorIterator; |
There was a problem hiding this comment.
Need a cursorIterator now, since the cursor has changed in the latest version and does not return an iterator, but returns a stream. This is being used internally by the read method, so it should not have any effect outside in other places.
| with: | ||
| distribution: temurin | ||
| java-version: ${{ matrix.java-version }} | ||
| - name: Install and Start MongoDB |
There was a problem hiding this comment.
Removed the dependence on an external MongoDB for tests and updated all such tests to use testContainers.
tcezard
left a comment
tcezard
left a comment
There was a problem hiding this comment.
The code change looks good and complete
| this.userName = userName; | ||
| public MongoConnectionStringBuilder username(final String userName) { | ||
| if (Objects.nonNull(userName)) { | ||
| this.userName = URLEncoder.encode(userName, StandardCharsets.UTF_8); |
There was a problem hiding this comment.
Did we ever need to URL encode the username ? ONly the password seems to have been needed.
There was a problem hiding this comment.
We don't need this for our current username; I added it for completeness and consistency.
apriltuesday
left a comment
apriltuesday
left a comment
There was a problem hiding this comment.
Great work! Appreciate the explanatory comments as well.
3 participants
No description provided.