Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@
import org.apache.kafka.common.requests.MetadataRequest;
import org.apache.kafka.common.requests.MetadataResponse;
import org.apache.kafka.common.serialization.Deserializer;
import org.apache.kafka.common.serialization.ExtendedDeserializer;
import org.apache.kafka.common.utils.CloseableIterator;
import org.apache.kafka.common.utils.LogContext;
import org.apache.kafka.common.utils.Time;
Expand All @@ -89,7 +88,6 @@
import java.util.concurrent.atomic.AtomicReference;

import static java.util.Collections.emptyList;
import static org.apache.kafka.common.serialization.ExtendedDeserializer.Wrapper.ensureExtended;

/**
* This class manage the fetching process with the brokers.
Expand All @@ -112,8 +110,8 @@ public class Fetcher<K, V> implements SubscriptionState.Listener, Closeable {
private final SubscriptionState subscriptions;
private final ConcurrentLinkedQueue<CompletedFetch> completedFetches;
private final BufferSupplier decompressionBufferSupplier = BufferSupplier.create();
private final ExtendedDeserializer<K> keyDeserializer;
private final ExtendedDeserializer<V> valueDeserializer;
private final Deserializer<K> keyDeserializer;
private final Deserializer<V> valueDeserializer;
private final IsolationLevel isolationLevel;
private final Map<Integer, FetchSessionHandler> sessionHandlers;
private final AtomicReference<RuntimeException> cachedListOffsetsException = new AtomicReference<>();
Expand Down Expand Up @@ -150,8 +148,8 @@ public Fetcher(LogContext logContext,
this.fetchSize = fetchSize;
this.maxPollRecords = maxPollRecords;
this.checkCrcs = checkCrcs;
this.keyDeserializer = ensureExtended(keyDeserializer);
this.valueDeserializer = ensureExtended(valueDeserializer);
this.keyDeserializer = keyDeserializer;
this.valueDeserializer = valueDeserializer;
this.completedFetches = new ConcurrentLinkedQueue<>();
this.sensors = new FetchManagerMetrics(metrics, metricsRegistry);
this.retryBackoffMs = retryBackoffMs;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,13 @@
import org.apache.kafka.common.record.AbstractRecords;
import org.apache.kafka.common.record.CompressionType;
import org.apache.kafka.common.record.RecordBatch;
import org.apache.kafka.common.serialization.ExtendedSerializer;
import org.apache.kafka.common.serialization.Serializer;
import org.apache.kafka.common.utils.AppInfoParser;
import org.apache.kafka.common.utils.KafkaThread;
import org.apache.kafka.common.utils.LogContext;
import org.apache.kafka.common.utils.Time;
import org.slf4j.Logger;

import static org.apache.kafka.common.serialization.ExtendedSerializer.Wrapper.ensureExtended;

/**
* A Kafka client that publishes records to the Kafka cluster.
Expand Down Expand Up @@ -250,8 +248,8 @@ public class KafkaProducer<K, V> implements Producer<K, V> {
private final CompressionType compressionType;
private final Sensor errors;
private final Time time;
private final ExtendedSerializer<K> keySerializer;
private final ExtendedSerializer<V> valueSerializer;
private final Serializer<K> keySerializer;
private final Serializer<V> valueSerializer;
private final ProducerConfig producerConfig;
private final long maxBlockTimeMs;
private final int requestTimeoutMs;
Expand Down Expand Up @@ -361,20 +359,20 @@ public KafkaProducer(Properties properties, Serializer<K> keySerializer, Seriali
this.partitioner = config.getConfiguredInstance(ProducerConfig.PARTITIONER_CLASS_CONFIG, Partitioner.class);
long retryBackoffMs = config.getLong(ProducerConfig.RETRY_BACKOFF_MS_CONFIG);
if (keySerializer == null) {
this.keySerializer = ensureExtended(config.getConfiguredInstance(ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG,
Serializer.class));
this.keySerializer = config.getConfiguredInstance(ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG,
Serializer.class);
this.keySerializer.configure(config.originals(), true);
} else {
config.ignore(ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG);
this.keySerializer = ensureExtended(keySerializer);
this.keySerializer = keySerializer;
}
if (valueSerializer == null) {
this.valueSerializer = ensureExtended(config.getConfiguredInstance(ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG,
Serializer.class));
this.valueSerializer = config.getConfiguredInstance(ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG,
Serializer.class);
this.valueSerializer.configure(config.originals(), false);
} else {
config.ignore(ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG);
this.valueSerializer = ensureExtended(valueSerializer);
this.valueSerializer = valueSerializer;
}

// load interceptors and make sure they get clientId
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import org.apache.kafka.common.TopicPartition;
import org.apache.kafka.common.errors.ProducerFencedException;
import org.apache.kafka.common.record.RecordBatch;
import org.apache.kafka.common.serialization.ExtendedSerializer;
import org.apache.kafka.common.serialization.Serializer;

import java.util.ArrayDeque;
Expand All @@ -41,8 +40,6 @@
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;

import static org.apache.kafka.common.serialization.ExtendedSerializer.Wrapper.ensureExtended;

/**
* A mock of the producer interface you can use for testing code that uses Kafka.
* <p>
Expand All @@ -59,8 +56,8 @@ public class MockProducer<K, V> implements Producer<K, V> {
private final Map<TopicPartition, Long> offsets;
private final List<Map<String, Map<TopicPartition, OffsetAndMetadata>>> consumerGroupOffsets;
private Map<String, Map<TopicPartition, OffsetAndMetadata>> uncommittedConsumerGroupOffsets;
private final ExtendedSerializer<K> keySerializer;
private final ExtendedSerializer<V> valueSerializer;
private final Serializer<K> keySerializer;
private final Serializer<V> valueSerializer;
private boolean autoComplete;
private boolean closed;
private boolean transactionInitialized;
Expand Down Expand Up @@ -93,8 +90,8 @@ public MockProducer(final Cluster cluster,
this.cluster = cluster;
this.autoComplete = autoComplete;
this.partitioner = partitioner;
this.keySerializer = ensureExtended(keySerializer);
this.valueSerializer = ensureExtended(valueSerializer);
this.keySerializer = keySerializer;
this.valueSerializer = valueSerializer;
this.offsets = new HashMap<>();
this.sent = new ArrayList<>();
this.uncommittedSends = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
*/
package org.apache.kafka.common.serialization;

import org.apache.kafka.common.header.Headers;

import java.nio.ByteBuffer;
import java.util.Map;

Expand Down Expand Up @@ -44,6 +46,11 @@ public byte[] serialize(String topic, ByteBuffer data) {
return ret;
}

@Override
public byte[] serialize(String topic, Headers headers, ByteBuffer data) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may have been left behind from a previous iteration, but shouldn't we just rely on the default implementation?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I'll correct it.

return new byte[0];
}

public void close() {
// nothing to do
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
*/
package org.apache.kafka.common.serialization;

import org.apache.kafka.common.header.Headers;

import java.io.Closeable;
import java.util.Map;

Expand Down Expand Up @@ -43,7 +45,20 @@ public interface Deserializer<T> extends Closeable {
* @param data serialized bytes; may be null; implementations are recommended to handle null by returning a value or null rather than throwing an exception.
* @return deserialized typed data; may be null
*/
T deserialize(String topic, byte[] data);
default T deserialize(String topic, byte[] data) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @viktorsomogyi ,

Sorry I missed the KIP discussion, but I just read the KIP over and didn't see a explanation of this:

Can you explain the reason to provide a default implementation of this method?

Thanks,
-John

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I just now saw the discussion in the vote thread. It sounds like you're going to remove this default. Sorry about that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Thank you for the review though. :)
I'll updte this piece of code soon.

return null;
}

/**
* Deserialize a record value from a byte array into a value or object.
* @param topic topic associated with the data
* @param headers headers associated with the record; may be empty.
* @param data serialized bytes; may be null; implementations are recommended to handle null by returning a value or null rather than throwing an exception.
* @return deserialized typed data; may be null
*/
default T deserialize(String topic, Headers headers, byte[] data) {
return deserialize(topic, data);
}

@Override
void close();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@
*
* A class that implements this interface is expected to have a constructor with no parameters.
* @param <T>
* @deprecated This class has been deprecated and will be removed in a future release. Please use {@link Deserializer} instead.
*/
@Deprecated
public interface ExtendedDeserializer<T> extends Deserializer<T> {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@
*
* A class that implements this interface is expected to have a constructor with no parameters.
* @param <T>
* @deprecated This class has been deprecated and will be removed in a future release. Please use {@link Serializer} instead.
*/
@Deprecated
public interface ExtendedSerializer<T> extends Serializer<T> {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
*/
package org.apache.kafka.common.serialization;

import org.apache.kafka.common.header.Headers;

import java.io.Closeable;
import java.util.Map;

Expand Down Expand Up @@ -44,7 +46,21 @@ public interface Serializer<T> extends Closeable {
* @param data typed data
* @return serialized bytes
*/
byte[] serialize(String topic, T data);
default byte[] serialize(String topic, T data) {
return new byte[0];
}

/**
* Convert {@code data} into a byte array.
*
* @param topic topic associated with data
* @param headers headers associated with the record
* @param data typed data
* @return serialized bytes
*/
default byte[] serialize(String topic, Headers headers, T data) {
return serialize(topic, data);
}

/**
* Close this serializer.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.apache.kafka.common.network.Selectable;
import org.apache.kafka.common.serialization.ByteArraySerializer;
import org.apache.kafka.common.serialization.ExtendedSerializer;
import org.apache.kafka.common.serialization.Serializer;
import org.apache.kafka.common.serialization.StringSerializer;
import org.apache.kafka.common.utils.MockTime;
import org.apache.kafka.common.utils.Time;
Expand Down Expand Up @@ -416,15 +417,25 @@ public void testTopicRefreshInMetadata() throws Exception {
Assert.assertTrue("Topic should still exist in metadata", metadata.containsTopic(topic));
}

@SuppressWarnings("unchecked") // safe as generic parameters won't vary
@PrepareOnlyThisForTest(Metadata.class)
@Test
public void testHeadersWithExtendedClasses() throws Exception {
doTestHeaders(ExtendedSerializer.class);
}

@SuppressWarnings("unchecked")
@PrepareOnlyThisForTest(Metadata.class)
@Test
public void testHeaders() throws Exception {
doTestHeaders(Serializer.class);
}

private <T extends Serializer<String>> void doTestHeaders(Class<T> serializerClassToMock) throws Exception {
Properties props = new Properties();
props.setProperty(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, "localhost:9999");
@SuppressWarnings("unchecked") // it is safe to suppress, since this is a mock class
ExtendedSerializer<String> keySerializer = PowerMock.createNiceMock(ExtendedSerializer.class);
@SuppressWarnings("unchecked")
ExtendedSerializer<String> valueSerializer = PowerMock.createNiceMock(ExtendedSerializer.class);
T keySerializer = PowerMock.createNiceMock(serializerClassToMock);
T valueSerializer = PowerMock.createNiceMock(serializerClassToMock);

KafkaProducer<String, String> producer = new KafkaProducer<>(props, keySerializer, valueSerializer);
Metadata metadata = PowerMock.createNiceMock(Metadata.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,61 +66,55 @@ class PlaintextConsumerTest extends BaseConsumerTest {
}
}

@Test
def testHeadersExtendedSerializerDeserializer() {
val numRecords = 1
val record = new ProducerRecord(tp.topic, tp.partition, null, "key".getBytes, "value".getBytes)

val extendedSerializer = new ExtendedSerializer[Array[Byte]] {

var serializer = new ByteArraySerializer()

override def serialize(topic: String, headers: Headers, data: Array[Byte]): Array[Byte] = {
headers.add("content-type", "application/octet-stream".getBytes)
serializer.serialize(topic, data)
}
trait SerializerImpl {
var serializer = new ByteArraySerializer()

override def configure(configs: util.Map[String, _], isKey: Boolean): Unit = serializer.configure(configs, isKey)

override def close(): Unit = serializer.close()

override def serialize(topic: String, data: Array[Byte]): Array[Byte] = {
fail("method should not be invoked")
null
}
def serialize(topic: String, headers: Headers, data: Array[Byte]): Array[Byte] = {
headers.add("content-type", "application/octet-stream".getBytes)
serializer.serialize(topic, data)
}

def configure(configs: util.Map[String, _], isKey: Boolean): Unit = serializer.configure(configs, isKey)

val extendedDeserializer = new ExtendedDeserializer[Array[Byte]] {

var deserializer = new ByteArrayDeserializer()
def close(): Unit = serializer.close()

override def deserialize(topic: String, headers: Headers, data: Array[Byte]): Array[Byte] = {
val header = headers.lastHeader("content-type")
assertEquals("application/octet-stream", if (header == null) null else new String(header.value()))
deserializer.deserialize(topic, data)
}
def serialize(topic: String, data: Array[Byte]): Array[Byte] = {
fail("method should not be invoked")
null
}
}

override def configure(configs: util.Map[String, _], isKey: Boolean): Unit = deserializer.configure(configs, isKey)
trait DeserializerImpl {
var deserializer = new ByteArrayDeserializer()

def deserialize(topic: String, headers: Headers, data: Array[Byte]): Array[Byte] = {
val header = headers.lastHeader("content-type")
assertEquals("application/octet-stream", if (header == null) null else new String(header.value()))
deserializer.deserialize(topic, data)
}

override def close(): Unit = deserializer.close()
def configure(configs: util.Map[String, _], isKey: Boolean): Unit = deserializer.configure(configs, isKey)

override def deserialize(topic: String, data: Array[Byte]): Array[Byte] = {
fail("method should not be invoked")
null
}
def close(): Unit = deserializer.close()

def deserialize(topic: String, data: Array[Byte]): Array[Byte] = {
fail("method should not be invoked")
null
}
}

private def testHeadersSerializeDeserialize(serializer: Serializer[Array[Byte]], deserializer: Deserializer[Array[Byte]]): Unit = {
val numRecords = 1
val record = new ProducerRecord(tp.topic, tp.partition, null, "key".getBytes, "value".getBytes)

val producer = createProducer(
keySerializer = new ByteArraySerializer,
valueSerializer = extendedSerializer)
valueSerializer = serializer)
producer.send(record)

val consumer = createConsumer(
keyDeserializer = new ByteArrayDeserializer,
valueDeserializer = extendedDeserializer)
valueDeserializer = deserializer)
assertEquals(0, consumer.assignment.size)
consumer.assign(List(tp).asJava)
assertEquals(1, consumer.assignment.size)
Expand All @@ -131,6 +125,22 @@ class PlaintextConsumerTest extends BaseConsumerTest {
assertEquals(numRecords, records.size)
}

@Test
def testHeadersExtendedSerializerDeserializer(): Unit = {
val extendedSerializer = new ExtendedSerializer[Array[Byte]] with SerializerImpl
val extendedDeserializer = new ExtendedDeserializer[Array[Byte]] with DeserializerImpl

testHeadersSerializeDeserialize(extendedSerializer, extendedDeserializer)
}

@Test
def testHeadersSerializerDeserializer(): Unit = {
val extendedSerializer = new Serializer[Array[Byte]] with SerializerImpl
val extendedDeserializer = new Deserializer[Array[Byte]] with DeserializerImpl

testHeadersSerializeDeserialize(extendedSerializer, extendedDeserializer)
}

@Test
def testMaxPollRecords() {
val maxPollRecords = 2
Expand Down
Loading