oscerd opened a new pull request, #1810:
URL: https://github.com/apache/camel-kafka-connector/pull/1810
Fixes #1799.
## What
`FileTransforms.apply()` carried an Eclipse-generated catch stub:
```java
String c = null;
try {
c = FileUtils.readFileToString(message.getFile(),
StandardCharsets.UTF_8);
} catch (IOException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}
return r.newRecord(..., SchemaHelper.buildSchemaBuilderForType(c), c,
r.timestamp());
```
A file that could not be read was reported only to stderr, and then `c` was
still `null`, so the read
failure was silently turned into a record with a `null` value and a schema
derived from `null`.
This PR:
- propagates the failure as a `ConnectException` naming the file, so Kafka
Connect's error handling
and DLQ apply and the failure is attributed to the file that caused it;
- fixes the `else` branch, which called `r.value().getClass()` to log the
unexpected type and so
threw `NullPointerException` for a record whose value is `null`;
- reuses the local `value` (previously assigned and never used) instead of
calling `r.value()` three
times.
## What this deliberately does not do
`FileUtils.readFileToString` still materialises the whole file as one
`String`. Capping that would
mean a new SMT option and a default size limit — a behaviour change for
existing users, and the
project's Security Model puts resource exhaustion on the operator side
("Denial of service via
resource exhaustion ... operators apply ... JVM heap limits"). Happy to add
a cap if you'd prefer,
but it seemed like a separate decision rather than part of fixing the
swallowed exception.
## Tests
This is the first test in a connector module (none of the 231 currently have
one), so the pom needed
a `junit-jupiter` test dependency. It is placed **before** the `<!--START OF
GENERATED CODE-->`
marker so the connector generator does not overwrite it — verified by
running the generator via a
full root build, which left it intact. Surefire is already configured in
that pom.
Four cases: content is read, an unreadable file raises `ConnectException`
with the `IOException` as
cause, a `null` record value passes through, and a record of an unexpected
type passes through.
Confirmed both defects are actually caught — against the unpatched
transform, two of the four fail:
```
testUnreadableFileFailsTheRecordInsteadOfEmittingANullValue
expected: <ConnectException> but was: <java.lang.NullPointerException>
testRecordWithANullValueIsPassedThrough
NullPointer: Cannot invoke "Object.getClass()" because the return value of
"org.apache.kafka.connect.connector.ConnectRecord.value()" is null
```
## Verification
- `camel-file-kafka-connector`: 4/4 tests pass.
- Full reactor build from the repository root (`./mvnw clean install
-DskipTests`): BUILD SUCCESS.
Related: #1786 removed the open TODO/XXX comments in `core`; this one was in
a connector module and
was not covered by that sweep.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
