Skip to content

Commit

Permalink
fix: add a floating point check (#76)
Browse files Browse the repository at this point in the history
  • Loading branch information
lavkesh authored Jan 12, 2023
1 parent c008f81 commit a018d59
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 21 deletions.
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ plugins {
}

group 'io.odpf'
version '0.3.6'
version '0.3.7'

repositories {
mavenCentral()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public Records convert(List<OdpfMessage> messages) {
} catch (EmptyMessageException | UnsupportedOperationException e) {
ErrorInfo errorInfo = new ErrorInfo(e, ErrorType.INVALID_MESSAGE_ERROR);
invalidRecords.add(new Record(message.getMetadata(), Collections.emptyMap(), index, errorInfo));
} catch (DeserializerException e) {
} catch (DeserializerException | IllegalArgumentException e) {
ErrorInfo errorInfo = new ErrorInfo(e, ErrorType.DESERIALIZATION_ERROR);
invalidRecords.add(new Record(message.getMetadata(), Collections.emptyMap(), index, errorInfo));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.odpf.depot.message.proto;

import com.google.api.client.util.DateTime;
import com.google.api.client.util.Preconditions;
import com.google.protobuf.Descriptors;
import com.google.protobuf.DynamicMessage;
import io.odpf.depot.common.Tuple;
Expand Down Expand Up @@ -89,13 +90,24 @@ private Map<String, Object> getMappings(DynamicMessage message, Properties colum
Tuple<String, Object> nestedColumns = getNestedColumnName(field, value);
row.put(nestedColumns.getFirst(), nestedColumns.getSecond());
} else {
floatCheck(fieldValue);
row.put(columnName, fieldValue);
}
}
});
return row;
}

private void floatCheck(Object fieldValue) {
if (fieldValue instanceof Float) {
float floatValue = ((Number) fieldValue).floatValue();
Preconditions.checkArgument(!Float.isInfinite(floatValue) && !Float.isNaN(floatValue));
} else if (fieldValue instanceof Double) {
double doubleValue = ((Number) fieldValue).doubleValue();
Preconditions.checkArgument(!Double.isInfinite(doubleValue) && !Double.isNaN(doubleValue));
}
}

private Tuple<String, Object> getNestedColumnName(Object field, Object value) {
try {
String columnName = getNestedColumnName((Properties) value);
Expand Down Expand Up @@ -127,6 +139,7 @@ private void addRepeatedFields(Map<String, Object> row, Object value, List<Objec
if (f instanceof Instant) {
repeatedNestedFields.add(new DateTime(((Instant) f).toEpochMilli()));
} else {
floatCheck(f);
repeatedNestedFields.add(f);
}
assert value instanceof String;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,35 +1,22 @@
package io.odpf.depot.message.proto;

import com.google.api.client.util.DateTime;
import com.google.protobuf.Descriptors;
import com.google.protobuf.Duration;
import com.google.protobuf.DynamicMessage;
import com.google.protobuf.ListValue;
import com.google.protobuf.Message;
import com.google.protobuf.Struct;
import com.google.protobuf.Timestamp;
import com.google.protobuf.Value;
import com.google.protobuf.*;
import com.google.protobuf.util.JsonFormat;
import io.odpf.depot.StatusBQ;
import io.odpf.depot.TestBookingLogMessage;
import io.odpf.depot.TestKeyBQ;
import io.odpf.depot.TestLocation;
import io.odpf.depot.TestMessage;
import io.odpf.depot.TestMessageBQ;
import io.odpf.depot.TestNestedMessageBQ;
import io.odpf.depot.TestNestedRepeatedMessageBQ;
import io.odpf.depot.TestTypesMessage;
import io.odpf.depot.*;
import io.odpf.depot.message.OdpfMessageSchema;
import io.odpf.depot.message.ParsedOdpfMessage;
import io.odpf.depot.message.proto.converter.fields.MessageProtoField;
import io.odpf.depot.message.proto.converter.fields.ProtoField;
import io.odpf.stencil.Parser;
import io.odpf.stencil.StencilClientFactory;
import io.odpf.stencil.client.StencilClient;
import org.apache.xerces.impl.dv.util.Base64;
import org.json.JSONArray;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.jupiter.api.Assertions;
import org.mockito.Mock;
import org.mockito.Mockito;

Expand All @@ -45,7 +32,7 @@

public class ProtoOdpfParsedMessageTest {

private static JsonFormat.Printer printer = JsonFormat.printer()
private static final JsonFormat.Printer PRINTER = JsonFormat.printer()
.preservingProtoFieldNames()
.omittingInsignificantWhitespace();
private Timestamp createdAt;
Expand Down Expand Up @@ -85,6 +72,8 @@ public void setUp() throws IOException, Descriptors.DescriptorValidationExceptio
put(String.format("%s", TestBookingLogMessage.TopicMetadata.class.getName()), TestBookingLogMessage.TopicMetadata.getDescriptor());
put(String.format("%s", TestTypesMessage.class.getName()), TestTypesMessage.getDescriptor());
put(String.format("%s", TestMessage.class.getName()), TestMessage.getDescriptor());
put(String.format("%s", FloatTest.class.getName()), FloatTest.getDescriptor());
put(String.format("%s", FloatTestContainer.class.getName()), FloatTestContainer.getDescriptor());
put("io.odpf.depot.TestMessageBQ.CurrentStateEntry", TestMessageBQ.getDescriptor().getNestedTypes().get(0));
put("com.google.protobuf.Struct.FieldsEntry", Struct.getDescriptor().getNestedTypes().get(0));
put("com.google.protobuf.Duration", com.google.protobuf.Duration.getDescriptor());
Expand All @@ -109,6 +98,15 @@ public void shouldReturnFieldsInProperties() throws IOException {
assertEquals(21, dateFields.get("day"));
}

@Test
public void shouldThrowExceptionWhenFloatingPointIsNaN() throws IOException {
String data = "ogQFJQAAwH8=";
byte[] decode = Base64.decode(data);
DynamicMessage message = DynamicMessage.parseFrom(FloatTest.getDescriptor(), decode);
OdpfMessageSchema odpfMessageSchema = odpfMessageParser.getSchema("io.odpf.depot.FloatTest", descriptorsMap);
Assertions.assertThrows(IllegalArgumentException.class, () -> new ProtoOdpfParsedMessage(message).getMapping(odpfMessageSchema));
}

@Test
public void shouldParseDurationMessageSuccessfully() throws IOException {
TestMessageBQ message = TestProtoUtil.generateTestMessage(now);
Expand Down Expand Up @@ -425,7 +423,7 @@ public void shouldGetRepeatableStructField() throws IOException {
JSONArray expectedArray = new JSONArray();
JSONArray actualArray = new JSONArray();
for (int ii = 0; ii < message.getAttributesCount(); ii++) {
expectedArray.put(printer.print(message.getAttributes(ii)));
expectedArray.put(PRINTER.print(message.getAttributes(ii)));
actualArray.put(attributes.get(ii));
}
Assert.assertEquals(expectedArray.toString(), actualArray.toString());
Expand Down
2 changes: 2 additions & 0 deletions src/test/java/io/odpf/depot/message/proto/TestProtoUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ public class TestProtoUtil {
public static final int TRIP_DURATION_NANOS = 1000;
private static final long TRIP_DURATION_SECONDS_1 = 12;
private static final long TRIP_DURATION_SECONDS_2 = 15;
private static final float PRICE = 12.12f;
private static int call = 0;

public static TestMessageBQ generateTestMessage(Instant now) {
Expand All @@ -25,6 +26,7 @@ public static TestMessageBQ generateTestMessage(Instant now) {
.setOrderUrl("order-url-" + call)
.setOrderDetails("order-details-" + call)
.setCreatedAt(createdAt)
.setPrice(PRICE)
.setStatus(StatusBQ.COMPLETED)
.setTripDuration(Duration.newBuilder().setSeconds(1).setNanos(TRIP_DURATION_NANOS).build())
.addUpdatedAt(createdAt)
Expand Down
8 changes: 8 additions & 0 deletions src/test/proto/TestMessage.proto
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,14 @@ message TestKey {
string order_url = 2;
}

message FloatTest {
FloatTestContainer container = 68;
}

message FloatTestContainer {
float value = 4;
}

message TestMessage {
string order_number = 1;
string order_url = 2;
Expand Down

0 comments on commit a018d59

Please sign in to comment.