#7817 Accessors should only handle ConvertedMap and ConvertedList as Collection, non-scalar types

Fixes #7823
This commit is contained in:
Armin 2017-07-26 21:56:05 +02:00 committed by Armin Braun
parent 8282b8b95b
commit 299fb34772
4 changed files with 69 additions and 69 deletions

View file

@ -4,12 +4,12 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
public class Accessors {
class Accessors {
private Map<String, Object> data;
private final ConvertedMap data;
protected Map<String, Object> lut;
public Accessors(Map<String, Object> data) {
public Accessors(final ConvertedMap data) {
this.data = data;
this.lut = new HashMap<>(); // reference -> target LUT
}
@ -24,16 +24,16 @@ public class Accessors {
final FieldReference field = PathCache.cache(reference);
final Object target = findCreateTarget(field);
final String key = field.getKey();
if (target instanceof Map) {
((Map<String, Object>) target).put(key, value);
} else if (target instanceof List) {
if (target instanceof ConvertedMap) {
((ConvertedMap) target).put(key, value);
} else if (target instanceof ConvertedList) {
int i;
try {
i = Integer.parseInt(key);
} catch (NumberFormatException e) {
return null;
}
int size = ((List<Object>) target).size();
int size = ((ConvertedList) target).size();
if (i >= size) {
// grow array by adding trailing null items
// this strategy reflects legacy Ruby impl behaviour and is backed by specs
@ -45,7 +45,7 @@ public class Accessors {
((List<Object>) target).add(value);
} else {
int offset = listIndex(i, ((List) target).size());
((List<Object>) target).set(offset, value);
((ConvertedList) target).set(offset, value);
}
} else {
throw newCollectionException(target);
@ -57,9 +57,9 @@ public class Accessors {
FieldReference field = PathCache.cache(reference);
Object target = findTarget(field);
if (target != null) {
if (target instanceof Map) {
return ((Map<String, Object>) target).remove(field.getKey());
} else if (target instanceof List) {
if (target instanceof ConvertedMap) {
return ((ConvertedMap) target).remove(field.getKey());
} else if (target instanceof ConvertedList) {
try {
int i = Integer.parseInt(field.getKey());
int offset = listIndex(i, ((List) target).size());
@ -78,11 +78,11 @@ public class Accessors {
final FieldReference field = PathCache.cache(reference);
final Object target = findTarget(field);
final String key = field.getKey();
return target instanceof Map && ((Map<String, Object>) target).containsKey(key) ||
target instanceof List && foundInList(key, (List<Object>) target);
return target instanceof ConvertedMap && ((ConvertedMap) target).containsKey(key) ||
target instanceof ConvertedList && foundInList(key, (ConvertedList) target);
}
private static boolean foundInList(final String key, final List<Object> target) {
private static boolean foundInList(final String key, final ConvertedList target) {
try {
return foundInList(target, Integer.parseInt(key));
} catch (NumberFormatException e) {
@ -122,14 +122,14 @@ public class Accessors {
for (String key : field.getPath()) {
Object result = fetch(target, key);
if (result == null) {
result = new HashMap<String, Object>();
if (target instanceof Map) {
((Map<String, Object>)target).put(key, result);
} else if (target instanceof List) {
result = new ConvertedMap(1);
if (target instanceof ConvertedMap) {
((ConvertedMap) target).put(key, result);
} else if (target instanceof ConvertedList) {
try {
int i = Integer.parseInt(key);
// TODO: what about index out of bound?
((List<Object>)target).set(i, result);
((ConvertedList) target).set(i, result);
} catch (NumberFormatException e) {
continue;
}
@ -145,7 +145,7 @@ public class Accessors {
return target;
}
private static boolean foundInList(List<Object> target, int index) {
private static boolean foundInList(ConvertedList target, int index) {
try {
int offset = listIndex(index, target.size());
return target.get(offset) != null;
@ -156,13 +156,12 @@ public class Accessors {
}
private static Object fetch(Object target, String key) {
if (target instanceof Map) {
Object result = ((Map<String, Object>) target).get(key);
return result;
} else if (target instanceof List) {
if (target instanceof ConvertedMap) {
return ((ConvertedMap) target).get(key);
} else if (target instanceof ConvertedList) {
try {
int offset = listIndex(Integer.parseInt(key), ((List) target).size());
return ((List<Object>) target).get(offset);
int offset = listIndex(Integer.parseInt(key), ((ConvertedList) target).size());
return ((ConvertedList) target).get(offset);
} catch (IndexOutOfBoundsException|NumberFormatException e) {
return null;
}
@ -174,14 +173,11 @@ public class Accessors {
}
private static boolean isCollection(Object target) {
if (target == null) {
return false;
}
return (target instanceof Map || target instanceof List);
return target instanceof ConvertedList || target instanceof ConvertedMap;
}
private static ClassCastException newCollectionException(Object target) {
return new ClassCastException("expecting List or Map, found " + target.getClass());
return new ClassCastException("expecting ConvertedList or ConvertedMap, found " + target.getClass());
}
/*

View file

@ -10,7 +10,7 @@ public final class ConvertedMap extends HashMap<String, Object> {
private static final long serialVersionUID = -4651798808586901122L;
private ConvertedMap(final int size) {
ConvertedMap(final int size) {
super((size << 2) / 3 + 2);
}

View file

@ -23,8 +23,8 @@ import static org.logstash.ObjectMappers.JSON_MAPPER;
public final class Event implements Cloneable, Queueable {
private boolean cancelled;
private Map<String, Object> data;
private Map<String, Object> metadata;
private ConvertedMap data;
private ConvertedMap metadata;
private Timestamp timestamp;
private Accessors accessors;
private Accessors metadata_accessors;
@ -43,8 +43,8 @@ public final class Event implements Cloneable, Queueable {
public Event()
{
this.metadata = new HashMap<>();
this.data = new HashMap<>();
this.metadata = new ConvertedMap(10);
this.data = new ConvertedMap(10);
this.data.put(VERSION, VERSION_ONE);
this.cancelled = false;
this.timestamp = new Timestamp();
@ -75,9 +75,9 @@ public final class Event implements Cloneable, Queueable {
}
if (this.data.containsKey(METADATA)) {
this.metadata = (Map<String, Object>) this.data.remove(METADATA);
this.metadata = ConvertedMap.newFromMap((Map) this.data.remove(METADATA));
} else {
this.metadata = new HashMap<>();
this.metadata = new ConvertedMap(10);
}
this.metadata_accessors = new Accessors(this.metadata);
@ -98,11 +98,11 @@ public final class Event implements Cloneable, Queueable {
}
}
public Map<String, Object> getData() {
public ConvertedMap getData() {
return this.data;
}
public Map<String, Object> getMetadata() {
public ConvertedMap getMetadata() {
return this.metadata;
}
@ -155,7 +155,7 @@ public final class Event implements Cloneable, Queueable {
// TODO(talevy): check type of timestamp
this.accessors.set(reference, value);
} else if (reference.equals(METADATA_BRACKETS) || reference.equals(METADATA)) {
this.metadata = (Map<String, Object>) value;
this.metadata = ConvertedMap.newFromMap((Map) value);
this.metadata_accessors = new Accessors(this.metadata);
} else if (reference.startsWith(METADATA_BRACKETS)) {
this.metadata_accessors.set(reference.substring(METADATA_BRACKETS.length()), value);
@ -270,7 +270,7 @@ public final class Event implements Cloneable, Queueable {
@Override
public Event clone() {
return new Event(Cloner.deep(this.data));
return new Event(Cloner.<Map>deep(this.data));
}
public String toString() {

View file

@ -1,5 +1,6 @@
package org.logstash;
import java.io.Serializable;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
@ -11,6 +12,7 @@ import org.junit.experimental.theories.Theories;
import org.junit.experimental.theories.Theory;
import org.junit.rules.ExpectedException;
import org.junit.runner.RunWith;
import org.logstash.bivalues.StringBiValue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
@ -19,9 +21,9 @@ import static org.junit.Assert.assertTrue;
public class AccessorsTest {
public class TestableAccessors extends Accessors {
private static final class TestableAccessors extends Accessors {
public TestableAccessors(Map<String, Object> data) {
public TestableAccessors(ConvertedMap data) {
super(data);
}
@ -32,7 +34,7 @@ public class AccessorsTest {
@Test
public void testBareGet() throws Exception {
Map<String, Object> data = new HashMap<>();
final ConvertedMap data = new ConvertedMap(1);
data.put("foo", "bar");
String reference = "foo";
@ -44,8 +46,9 @@ public class AccessorsTest {
@Test
public void testAbsentBareGet() throws Exception {
Map<String, Object> data = new HashMap<>();
data.put("foo", "bar");
final Map<Serializable, Object> java = new HashMap<>(1);
java.put("foo", "bar");
final ConvertedMap data = ConvertedMap.newFromMap(java);
String reference = "baz";
TestableAccessors accessors = new TestableAccessors(data);
@ -56,20 +59,21 @@ public class AccessorsTest {
@Test
public void testBareBracketsGet() throws Exception {
Map<String, Object> data = new HashMap<>();
data.put("foo", "bar");
final Map<Serializable, Object> java = new HashMap<>(1);
java.put("foo", "bar");
final ConvertedMap data = ConvertedMap.newFromMap(java);
String reference = "[foo]";
TestableAccessors accessors = new TestableAccessors(data);
assertNull(accessors.lutGet(reference));
assertEquals("bar", accessors.get(reference));
assertEquals(new StringBiValue("bar"), accessors.get(reference));
assertEquals(data, accessors.lutGet(reference));
}
@Test
public void testDeepMapGet() throws Exception {
Map<String, Object> data = new HashMap<>();
Map<String, Object> inner = new HashMap<>();
final ConvertedMap data = new ConvertedMap(1);
Map<String, Object> inner = new ConvertedMap(1);
data.put("foo", inner);
inner.put("bar", "baz");
@ -83,8 +87,8 @@ public class AccessorsTest {
@Test
public void testAbsentDeepMapGet() throws Exception {
Map<String, Object> data = new HashMap<>();
Map<String, Object> inner = new HashMap<>();
final ConvertedMap data = new ConvertedMap(1);
Map<String, Object> inner = new ConvertedMap(1);
data.put("foo", inner);
inner.put("bar", "baz");
@ -98,8 +102,8 @@ public class AccessorsTest {
@Test
public void testDeepListGet() throws Exception {
Map<String, Object> data = new HashMap<>();
List inner = new ArrayList();
final ConvertedMap data = new ConvertedMap(1);
List inner = new ConvertedList(1);
data.put("foo", inner);
inner.add("bar");
@ -113,8 +117,8 @@ public class AccessorsTest {
@Test
public void testAbsentDeepListGet() throws Exception {
Map<String, Object> data = new HashMap<>();
List inner = new ArrayList();
final ConvertedMap data = new ConvertedMap(1);
List inner = new ConvertedList(1);
data.put("foo", inner);
inner.add("bar");
@ -133,8 +137,8 @@ public class AccessorsTest {
*/
@Test
public void testInvalidIdList() throws Exception {
Map<String, Object> data = new HashMap<>();
List inner = new ArrayList();
final ConvertedMap data = new ConvertedMap(1);
List inner = new ConvertedList(1);
data.put("map1", inner);
inner.add("obj1");
inner.add("obj2");
@ -152,7 +156,7 @@ public class AccessorsTest {
@Test
public void testBarePut() throws Exception {
Map<String, Object> data = new HashMap<>();
final ConvertedMap data = new ConvertedMap(1);
String reference = "foo";
TestableAccessors accessors = new TestableAccessors(data);
@ -164,7 +168,7 @@ public class AccessorsTest {
@Test
public void testBareBracketsPut() throws Exception {
Map<String, Object> data = new HashMap<>();
final ConvertedMap data = new ConvertedMap(1);
String reference = "[foo]";
TestableAccessors accessors = new TestableAccessors(data);
@ -176,7 +180,7 @@ public class AccessorsTest {
@Test
public void testDeepMapSet() throws Exception {
Map<String, Object> data = new HashMap<>();
final ConvertedMap data = new ConvertedMap(1);
String reference = "[foo][bar]";
@ -189,8 +193,8 @@ public class AccessorsTest {
@Test
public void testDel() throws Exception {
Map<String, Object> data = new HashMap<>();
List inner = new ArrayList();
final ConvertedMap data = new ConvertedMap(1);
List inner = new ConvertedList(1);
data.put("foo", inner);
inner.add("bar");
data.put("bar", "baz");
@ -205,7 +209,7 @@ public class AccessorsTest {
@Test
public void testNilInclude() throws Exception {
Map<String, Object> data = new HashMap<>();
ConvertedMap data = new ConvertedMap(1);
data.put("nilfield", null);
TestableAccessors accessors = new TestableAccessors(data);
assertTrue(accessors.includes("nilfield"));
@ -213,7 +217,7 @@ public class AccessorsTest {
@Test
public void testInvalidPath() throws Exception {
Map<String, Object> data = new HashMap<>();
ConvertedMap data = new ConvertedMap(1);
Accessors accessors = new Accessors(data);
assertEquals(1, accessors.set("[foo]", 1));
@ -222,7 +226,7 @@ public class AccessorsTest {
@Test
public void testStaleTargetCache() throws Exception {
Map<String, Object> data = new HashMap<>();
ConvertedMap data = new ConvertedMap(1);
Accessors accessors = new Accessors(data);
assertNull(accessors.get("[foo][bar]"));