[PR] Fix #1800: scope the SMT TypeConverter and CamelContext to the instance [camel-kafka-connector]

["oscerd (via GitHub)" <gi...@apache.org>]

oscerd opened a new pull request, #1811:
URL: https://github.com/apache/camel-kafka-connector/pull/1811

   Fixes #1800.
   
   ## What
   
   Three related things in the bundled type-converter SMT.
   
   **1. `static` field assigned from an instance method**
   
   ```java
   private static TypeConverter typeConverter;
   ...
   public void configure(Map<String, ?> props) {
       ...
       typeConverter = getCamelContext().getTypeConverter();   // per-instance context
   }
   ```
   
   The last SMT instance configured in a plugin classloader repointed the converter used by every other
   connector's transforms in that worker. Now an instance field.
   
   **2. The per-instance `CamelContext` was never released**
   
   `CamelTransformSupport` creates a `DefaultCamelContext` per instance and `close()` was empty, so Kafka
   Connect re-instantiating transforms on every connector configuration update left those contexts
   accumulating for the worker's lifetime. `CamelTransformSupport` now exposes `stopCamelContext()` and
   `CamelTypeConverterTransform.close()` calls it. `CamelSinkTask` / `CamelSourceTask` already stop their
   context in `stop()`; this brings the SMTs in line.
   
   **3. The record value was formatted into the exception message**
   
   ```java
   throw new DataException(String.format("CamelTypeConverter was not able to convert value `%s` ...", originalValue, ...));
   ```
   
   Kafka Connect surfaces that message in the worker log and in the task status over the REST API. It now
   reports the source and target *type*, which is what is actually useful for diagnosing the failure.
   
   ## Tests
   
   `testConverterIsScopedToTheInstanceThatConfiguredIt` registers a converter for a test-local target type
   on **one** instance's context, then configures a second instance and asserts the first still converts.
   
   This distinction matters: my first attempt at this test passed with *and* without the fix, because
   each `DefaultCamelContext` resolves an equivalent converter, so overwriting the static field is not
   observable through the default registry — which is exactly why the existing four tests never caught
   it. Registering a converter on one context only makes it observable. Confirmed against the unpatched
   field:
   
   ```
   CamelTypeConverterTransformTest.testConverterIsScopedToTheInstanceThatConfiguredIt
     » DataException: CamelTypeConverter was not able to convert a value of type
       `java.lang.String` to target type of `Marker`
   ```
   
   `testConversionFailureDoesNotEchoTheRecordValue` likewise fails on the old message
   (`expected: <false> but was: <true>`), and `testCloseStopsTheCamelContext` covers the lifecycle.
   
   ## Verification
   
   - `core`: 105/105 tests pass (was 101, +4).
   - Full reactor build from the repository root (`./mvnw clean install -DskipTests`): BUILD SUCCESS.


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix #1800: scope the SMT TypeConverter and CamelContext to the instance [camel-kafka-connector]

["oscerd (via GitHub)" <gi...@apache.org>]

oscerd merged PR #1811:
URL: https://github.com/apache/camel-kafka-connector/pull/1811


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org