Skip to content

Commit

Permalink
Merge pull request #52 from gravity9-tech/17-patchtestoperation-fails…
Browse files Browse the repository at this point in the history
…-when-patching-float-value

17 patchtestoperation fails when patching float value
  • Loading branch information
MateuszGravity9 authored Jul 3, 2023
2 parents 7c7f201 + 9f06a90 commit 7acf941
Show file tree
Hide file tree
Showing 15 changed files with 642 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/main/java/com/gravity9/jsonpatch/TestOperation.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.JsonNode;
import com.github.fge.jackson.JsonNumEquals;
import com.gravity9.jsonpatch.jackson.JsonNumEquals;
import com.jayway.jsonpath.JsonPath;

/**
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/com/gravity9/jsonpatch/diff/DiffProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@
package com.gravity9.jsonpatch.diff;

import com.fasterxml.jackson.databind.JsonNode;
import com.github.fge.jackson.JsonNumEquals;
import com.github.fge.jackson.jsonpointer.JsonPointer;
import com.gravity9.jsonpatch.JsonPatch;
import com.gravity9.jsonpatch.JsonPatchOperation;
import com.gravity9.jsonpatch.jackson.JsonNumEquals;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/gravity9/jsonpatch/diff/JsonDiff.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.github.fge.jackson.JacksonUtils;
import com.github.fge.jackson.JsonNumEquals;
import com.github.fge.jackson.NodeType;
import com.github.fge.jackson.jsonpointer.JsonPointer;
import com.github.fge.msgsimple.bundle.MessageBundle;
Expand All @@ -34,6 +33,7 @@
import com.gravity9.jsonpatch.JsonPatchMessages;
import com.gravity9.jsonpatch.JsonPatchOperation;
import com.gravity9.jsonpatch.RemoveOperation;
import com.gravity9.jsonpatch.jackson.JsonNumEquals;
import com.jayway.jsonpath.PathNotFoundException;

import javax.annotation.ParametersAreNonnullByDefault;
Expand Down
243 changes: 243 additions & 0 deletions src/main/java/com/gravity9/jsonpatch/jackson/JsonNumEquals.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,243 @@
package com.gravity9.jsonpatch.jackson;

import com.fasterxml.jackson.databind.JsonNode;
import com.github.fge.jackson.NodeType;

import javax.annotation.Nullable;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;

/**
* An {@code com.google.common.base.Equivalence} like strategy for JSON Schema equality
*
* <p>{@link JsonNode} does a pretty good job of obeying the {@link
* Object#equals(Object) equals()}/{@link Object#hashCode() hashCode()}
* contract. And in fact, it does it too well for JSON Schema.</p>
*
* <p>For instance, it considers numeric nodes {@code 1} and {@code 1.0} to be
* different nodes, which is true. But some IETF RFCs and drafts (among them,
* JSON Schema and JSON Patch) mandate that numeric JSON values be considered
* equal if their mathematical value is the same. This class implements this
* kind of equality.</p>
*/
public final class JsonNumEquals
{
private static final com.gravity9.jsonpatch.jackson.JsonNumEquals INSTANCE
= new com.gravity9.jsonpatch.jackson.JsonNumEquals();

private JsonNumEquals()
{
}

public static com.gravity9.jsonpatch.jackson.JsonNumEquals getInstance()
{
return INSTANCE;
}

@SuppressWarnings("ReferenceEquality")
public final boolean equivalent(@Nullable JsonNode a, @Nullable JsonNode b) {
if (a == b) {
return true;
}
if (a == null || b == null) {
return false;
}
return doEquivalent(a, b);
}

private boolean doEquivalent(final JsonNode a, final JsonNode b)
{
/*
* If both are numbers, delegate to the helper method
*/
if (a.isNumber() && b.isNumber())
return numEquals(a, b);

final NodeType typeA = NodeType.getNodeType(a);
final NodeType typeB = NodeType.getNodeType(b);

/*
* If they are of different types, no dice
*/
if (typeA != typeB)
return false;

/*
* For all other primitive types than numbers, trust JsonNode
*/
if (!a.isContainerNode())
return a.equals(b);

/*
* OK, so they are containers (either both arrays or objects due to the
* test on types above). They are obviously not equal if they do not
* have the same number of elements/members.
*/
if (a.size() != b.size())
return false;

/*
* Delegate to the appropriate method according to their type.
*/
return typeA == NodeType.ARRAY ? arrayEquals(a, b) : objectEquals(a, b);
}

public int hash(@Nullable JsonNode t) {
if (t == null) {
return 0;
}
return doHash(t);
}

private int doHash(final JsonNode t)
{
/*
* If this is a numeric node, we want the same hashcode for the same
* mathematical values. Go with double, its range is good enough for
* 99+% of use cases.
*/
if (t.isNumber())
return Double.valueOf(t.doubleValue()).hashCode();

/*
* If this is a primitive type (other than numbers, handled above),
* delegate to JsonNode.
*/
if (!t.isContainerNode())
return t.hashCode();

/*
* The following hash calculations work, yes, but they are poor at best.
* And probably slow, too.
*
* TODO: try and figure out those hash classes from Guava
*/
int ret = 0;

/*
* If the container is empty, just return
*/
if (t.size() == 0)
return ret;

/*
* Array
*/
if (t.isArray()) {
for (final JsonNode element: t)
ret = 31 * ret + hash(element);
return ret;
}

/*
* Not an array? An object.
*/
final Iterator<Map.Entry<String, JsonNode>> iterator = t.fields();

Map.Entry<String, JsonNode> entry;

while (iterator.hasNext()) {
entry = iterator.next();
ret = 31 * ret
+ (entry.getKey().hashCode() ^ hash(entry.getValue()));
}

return ret;
}

// private static boolean numEquals(final JsonNode a, final JsonNode b)
// {
// /*
// * If both numbers are integers, delegate to JsonNode.
// */
// if (a.isIntegralNumber() && b.isIntegralNumber())
// return a.equals(b);
//
// /*
// * Otherwise, compare decimal values.
// */
// return a.decimalValue().compareTo(b.decimalValue()) == 0;
// }

private boolean numEquals(JsonNode a, JsonNode b) {
return a.isNumber() && b.isNumber()
? areNumberNodesNumericallyEqual(a, b)
: a.equals(b);
}

private boolean areNumberNodesNumericallyEqual(JsonNode a, JsonNode b) {
if (a.isIntegralNumber() && b.isIntegralNumber()) {
return a.canConvertToLong() && b.canConvertToLong()
? a.longValue() == b.longValue()
: a.bigIntegerValue().equals(b.bigIntegerValue());
}

if(a.isFloatingPointNumber() && b.isFloatingPointNumber()){
return a.isFloat() || b.isFloat() ?
Float.compare(a.floatValue(), b.floatValue()) == 0
: Double.compare(a.doubleValue(), b.doubleValue()) == 0;
}

return a.decimalValue().compareTo(b.decimalValue()) == 0;
}

private boolean arrayEquals(final JsonNode a, final JsonNode b)
{
/*
* We are guaranteed here that arrays are the same size.
*/
final int size = a.size();

for (int i = 0; i < size; i++)
if (!equivalent(a.get(i), b.get(i)))
return false;

return true;
}

private boolean objectEquals(final JsonNode a, final JsonNode b)
{
/*
* Grab the key set from the first node
*/
final Set<String> keys = new HashSet<>();
Iterator<String> iterator1 = a.fieldNames();
while (iterator1.hasNext()) {
final String next = iterator1.next();
if (next != null) {
keys.add(next);
} else {
throw new NullPointerException();
}
}

/*
* Grab the key set from the second node, and see if both sets are the
* same. If not, objects are not equal, no need to check for children.
*/
final Set<String> set = new HashSet<>();
Iterator<String> iterator2 = b.fieldNames();
while (iterator2.hasNext()) {
final String next = iterator2.next();
if (next != null) {
set.add(next);
} else {
throw new NullPointerException();
}
}

if (!set.equals(keys))
return false;

/*
* Test each member individually.
*/
for (final String key: keys)
if (!equivalent(a.get(key), b.get(key)))
return false;

return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,12 @@
import com.fasterxml.jackson.databind.ObjectReader;
import com.github.fge.jackson.JacksonUtils;
import com.github.fge.jackson.JsonLoader;
import com.github.fge.jackson.JsonNumEquals;
import com.google.common.collect.Lists;
import java.io.IOException;
import java.util.Iterator;
import java.util.List;

import com.gravity9.jsonpatch.jackson.JsonNumEquals;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;

Expand Down
34 changes: 34 additions & 0 deletions src/test/java/com/gravity9/jsonpatch/TestOperationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,46 @@

package com.gravity9.jsonpatch;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.testng.annotations.Test;

import java.io.IOException;

public final class TestOperationTest extends JsonPatchOperationTest {

private static final TestDomain DOMAIN_TO_PATCH = new TestDomain(2022.4321f);
private static final String testOperation = "[{\"op\":\"test\",\"path\":\"/myValue\",\"value\":2022.4321}]";

public TestOperationTest()
throws IOException {
super("test");
}

@Test
void testPatchValueIsDoubleDomainValueIsFloat() throws Exception {
JsonPatch jsonPatch = new ObjectMapper().readValue(testOperation, JsonPatch.class);

JsonNode jsonNode = new ObjectMapper().valueToTree(DOMAIN_TO_PATCH);

jsonPatch.apply(jsonNode);
}

@SuppressWarnings("UnusedMethod")
private static class TestDomain {

private Float myValue;

public TestDomain(Float myValue) {
this.myValue = myValue;
}

public Float getMyValue() {
return myValue;
}

public void setMyValue(Float myValue) {
this.myValue = myValue;
}
}
}
3 changes: 2 additions & 1 deletion src/test/java/com/gravity9/jsonpatch/diff/JsonDiffTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,15 @@
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.github.fge.jackson.JsonLoader;
import com.github.fge.jackson.JsonNumEquals;
import com.google.common.collect.Lists;
import com.gravity9.jsonpatch.JsonPatch;
import com.gravity9.jsonpatch.JsonPatchException;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;

import com.gravity9.jsonpatch.jackson.JsonNumEquals;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.JsonNodeFactory;
import com.github.fge.jackson.JsonLoader;
import com.github.fge.jackson.JsonNumEquals;
import com.github.fge.msgsimple.bundle.MessageBundle;
import com.github.fge.msgsimple.load.MessageBundles;
import com.google.common.collect.Lists;
Expand All @@ -31,6 +30,8 @@
import java.io.IOException;
import java.util.Iterator;
import java.util.List;

import com.gravity9.jsonpatch.jackson.JsonNumEquals;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.JsonNodeFactory;
import com.github.fge.jackson.JsonLoader;
import com.github.fge.jackson.JsonNumEquals;
import com.github.fge.msgsimple.bundle.MessageBundle;
import com.github.fge.msgsimple.load.MessageBundles;
import com.google.common.collect.Lists;
Expand All @@ -31,6 +30,8 @@
import java.io.IOException;
import java.util.Iterator;
import java.util.List;

import com.gravity9.jsonpatch.jackson.JsonNumEquals;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;

Expand Down
Loading

0 comments on commit 7acf941

Please sign in to comment.