Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Addressing review comments
  • Loading branch information
ankitk-me committed Jan 10, 2024
commit c10639fe40b61946b184bda102847e3a6d61e4af
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public int encode(
SchemaRegistryPrefixFW prefix = prefixRW.rewrap().schemaId(schemaId).build();
next.accept(prefix.buffer(), prefix.offset(), prefix.sizeof());
int valLength = encoder.accept(schemaId, data, index, length, next);
return valLength != 0 ? prefix.sizeof() + valLength : -1;
return valLength > 0 ? prefix.sizeof() + valLength : -1;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ protected DescriptorTree findByName(
return current;
}

protected DescriptorTree findByIndexes(
protected Descriptor findByIndexes(
List<Integer> indexes)
{
DescriptorTree current = this;
Expand All @@ -86,10 +86,10 @@ protected DescriptorTree findByIndexes(
current = current.findChild(index);
if (current == null)
{
return null;
break;
}
}
return current;
return current != null ? current.descriptor : null;
}

private DescriptorTree findParent(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,9 @@ public int padding(

if (schemaId == NO_SCHEMA_ID)
{
if (catalog.id != NO_SCHEMA_ID)
{
schemaId = catalog.id;
}
else
{
schemaId = handler.resolve(subject, catalog.version);
}
schemaId = catalog.id != NO_SCHEMA_ID
? catalog.id
: handler.resolve(subject, catalog.version);
}
padding = supplyJsonFormatPadding(schemaId);
}
Expand Down Expand Up @@ -144,17 +139,15 @@ private int validate(
DescriptorTree tree = supplyDescriptorTree(schemaId);
if (tree != null)
{
tree = tree.findByIndexes(indexes);
if (tree != null)
Descriptors.Descriptor descriptor = tree.findByIndexes(indexes);
if (descriptor != null)
{
Descriptors.Descriptor descriptor = tree.descriptor;
in.wrap(data, index, length);
DynamicMessage.Builder builder = supplyDynamicMessageBuilder(descriptor);
validate:
try
{
builder.mergeFrom(in);
DynamicMessage message = builder.build();
DynamicMessage message = builder.mergeFrom(in).build();
builder.clear();
if (!message.getUnknownFields().asMap().isEmpty())

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 .asMap() still has allocation logic, but it may be the best we can do with these APIs.

{
Expand All @@ -175,9 +168,9 @@ private int validate(
valLength = length;
}
}
catch (IOException e)
catch (IOException ex)
{
e.printStackTrace();
ex.printStackTrace();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@
import com.google.protobuf.Descriptors.DescriptorValidationException;
import com.google.protobuf.Descriptors.FileDescriptor;
import com.google.protobuf.DynamicMessage;
import com.google.protobuf.InvalidProtocolBufferException;
import com.google.protobuf.util.JsonFormat;

import io.aklivity.zilla.runtime.engine.catalog.CatalogHandler;
import io.aklivity.zilla.runtime.engine.config.CatalogedConfig;
Expand All @@ -49,9 +47,12 @@

public class ProtobufValidator
{
protected static final byte ZERO_INDEX = 0x0;
protected static final byte[] ZERO_INDEX = new byte[]{0x0};
protected static final String FORMAT_JSON = "json";

private static final int JSON_FIELD_STRUCTURE_LENGTH = "\"\":\"\",".length();
private static final int JSON_OBJECT_CURLY_BRACES = 2;

protected final SchemaConfig catalog;
protected final CatalogHandler handler;
protected final String subject;
Expand Down Expand Up @@ -213,13 +214,13 @@ private int calculateJsonFormatPadding(

if (descriptor != null)
{
try
{
padding = 2 + JsonFormat.printer().print(descriptor.toProto()).length();
}
catch (InvalidProtocolBufferException e)
for (Descriptors.Descriptor message : descriptor.getMessageTypes())
{
e.printStackTrace();
padding += JSON_OBJECT_CURLY_BRACES;
for (Descriptors.FieldDescriptor field : message.getFields())
{
padding += field.getName().getBytes().length + JSON_FIELD_STRUCTURE_LENGTH;
}
}

}
Expand Down Expand Up @@ -249,9 +250,9 @@ private FileDescriptor createDescriptors(
{
descriptor = FileDescriptor.buildFrom(listener.build(), dependencies);
}
catch (DescriptorValidationException e)
catch (DescriptorValidationException ex)
{
e.printStackTrace();
ex.printStackTrace();
}
}
return descriptor;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,13 @@ private boolean validate(
DynamicMessage.Builder builder = supplyDynamicMessageBuilder(descriptor);
try
{
builder.mergeFrom(in);
DynamicMessage message = builder.build();
DynamicMessage message = builder.mergeFrom(in).build();
builder.clear();
status = message.getUnknownFields().asMap().isEmpty();
}
catch (IOException e)
catch (IOException ex)
{
e.printStackTrace();
ex.printStackTrace();
}
}
}
Expand All @@ -151,10 +150,10 @@ private int encode(
int length,
ValueConsumer next)
{
int valLength = 0;
int valLength = -1;
if (indexes.size() == 2 && indexes.get(0) == 1 && indexes.get(1) == 0)
{
indexesRO.wrap(new byte[]{ZERO_INDEX});
indexesRO.wrap(ZERO_INDEX);
valLength = 1;
}
else
Expand Down Expand Up @@ -183,6 +182,7 @@ private int serializeJsonRecord(
if (tree != null)
{
Descriptors.Descriptor descriptor = tree.descriptor;
indexes.clear();
indexes.add(tree.indexes.size());
indexes.addAll(tree.indexes);
DynamicMessage.Builder builder = supplyDynamicMessageBuilder(descriptor);
Expand All @@ -199,9 +199,9 @@ private int serializeJsonRecord(
valLength = encode(schemaId, out.buffer(), 0, out.position(), next);
}
}
catch (IOException e)
catch (IOException ex)
{
e.printStackTrace();
ex.printStackTrace();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ public void shouldVerifyJsonFormatPaddingLength()

DirectBuffer data = new UnsafeBuffer();

assertEquals(1584, validator.padding(data, 0, data.capacity()));
assertEquals(71, validator.padding(data, 0, data.capacity()));
}

@Test
Expand Down