KAFKA-165: Add change.stream.show.expanded.events property #172
Conversation
src/main/java/com/mongodb/kafka/connect/source/MongoSourceConfig.java
Outdated
Show resolved
Hide resolved
src/main/java/com/mongodb/kafka/connect/source/schema/AvroSchemaDefaults.java
Show resolved
Hide resolved
src/main/java/com/mongodb/kafka/connect/source/StartedMongoSourceTask.java
Show resolved
Hide resolved
| @@ -148,7 +153,7 @@ void testMissingChangeEventData() { | |||
| new SinkDocument( | |||
| null, | |||
| BsonDocument.parse( | |||
| "{documentKey: {}, updateDescription: {updatedFields: 1}}")))), | |||
| "{documentKey: {}, updateDescription: {updatedFields: 1, removedFields: []}}")))), | |||
There was a problem hiding this comment.
[q] what's the reason for changing this test?
There was a problem hiding this comment.
If you were to adjust updatedFields to a valid value, the test would still fail because removedFields is a required field. This more closely matches the expected behavior of validating updatedFields instead of failing due to another reason.
src/test/java/com/mongodb/kafka/connect/source/producer/SchemaAndValueProducerTest.java
Outdated
Show resolved
Hide resolved
| @@ -135,6 +136,10 @@ public boolean isAtLeastSixDotZero() { | |||
| return getMaxWireVersion() >= SIX_DOT_ZERO_WIRE_VERSION; | |||
| } | |||
|
|
|||
| public boolean isAtLeastSevenDotZero() { | |||
There was a problem hiding this comment.
[q] thoughts on adding integration tests against different major versions? It looks like we're missing 6 and 7.
This can be done as part of a separate ticket.
There was a problem hiding this comment.
Yeah that's a good idea, I added this ticket to the backlog.
| Struct update = (Struct) records.get(1).value(); | ||
| assertEquals(OperationType.UPDATE.getValue(), update.getString("operationType")); | ||
| Struct updateDescription = (Struct) update.get("updateDescription"); | ||
| assertEquals("{}", updateDescription.getString("disambiguatedPaths")); |
There was a problem hiding this comment.
you might have already mentioned this offline, but why aren't we checking that it contains the expected fields similar to this example?
There was a problem hiding this comment.
Ah this is because I wasn't able to produce disambiguatedPaths results because trying to update a field with a . just updates the nested structure instead.
There was a problem hiding this comment.
I'm going to take another look at this to make sure I wasn't missing something obvious 😄
No description provided.