Skip to content

Commit

Permalink
Fix NPE issue when batch execution is called without any queries (#207)
Browse files Browse the repository at this point in the history
Motivation:
(Please describe the problem you are trying to solve, or the new feature
you are trying to add.)

we're using this R2DBC client with jOOQ.
NPE occurs when we give empty list to `DSLContext#batch`

```kotlin
val queries = emptyList()
dslContext.batch(queries).awaitLast()
```

this is because `StringBuilder` in `MySqlBatchingBatch` is not
initialized until `add` method is called.
so, `execute` method will refer to not-initialized `builder` when `add`
method is not called.

Modification:
(Please describe the changes you have made to the codebase, including
any new files, modified files, or deleted files.)

~~modify `execute` to return `Flux#empty` when ` builder` is not
initialized (i.e., `add` method is not called).~~
send empty string to client without NPE

Result:
(Please describe the expected outcome of your changes, and any potential
side effects or drawbacks.
If possible, please also include any relevant testing results.)

it now does not throw NPE ~~but returns empty flux instead.~~
  • Loading branch information
T45K authored Jan 22, 2024
1 parent 38dd8b4 commit fa2ed46
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 12 deletions.
18 changes: 6 additions & 12 deletions src/main/java/io/asyncer/r2dbc/mysql/MySqlBatchingBatch.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import io.asyncer.r2dbc.mysql.codec.Codecs;
import reactor.core.publisher.Flux;

import java.util.StringJoiner;

import static io.asyncer.r2dbc.mysql.internal.util.AssertUtils.requireNonNull;

/**
Expand All @@ -34,7 +36,7 @@ final class MySqlBatchingBatch extends MySqlBatch {

private final ConnectionContext context;

private StringBuilder builder;
private final StringJoiner queries = new StringJoiner(";");

MySqlBatchingBatch(Client client, Codecs codecs, ConnectionContext context) {
this.client = requireNonNull(client, "client must not be null");
Expand All @@ -50,9 +52,9 @@ public MySqlBatch add(String sql) {

if (index >= 0 && sql.charAt(index) == ';') {
// Skip last ';' and whitespaces that following last ';'.
requireBuilder().append(sql, 0, index);
queries.add(sql.substring(0, index));
} else {
requireBuilder().append(sql);
queries.add(sql);
}

return this;
Expand All @@ -75,15 +77,7 @@ public String toString() {
* @return current batching SQL statement
*/
String getSql() {
return builder.toString();
}

private StringBuilder requireBuilder() {
if (builder == null) {
return (builder = new StringBuilder());
}

return builder.append(';');
return queries.toString();
}

private static int lastNonWhitespace(String sql) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,11 @@ void add() {
void badAdd() {
assertThrows(IllegalArgumentException.class, () -> batch.add(null));
}

@Test
void addNothing() {
final MySqlBatchingBatch batch = new MySqlBatchingBatch(mock(Client.class), mock(Codecs.class),
ConnectionContextTest.mock());
assertEquals(batch.getSql(), "");
}
}

0 comments on commit fa2ed46

Please sign in to comment.