Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Converting lazy abstract map to HashMap in ObjectFlatteners #14920

Closed
wants to merge 1 commit into from
Closed

Converting lazy abstract map to HashMap in ObjectFlatteners #14920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 26 additions & 0 deletions processing/src/main/java/org/apache/druid/java/util/common/collect/Utils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,13 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Comparator;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import java.util.stream.Collector;

public class Utils
{
Expand Down Expand Up @@ -108,4 +111,27 @@ public static <T> Iterator<T> mergeSorted(
{
return new MergeIterator<>(sortedIterators, comparator);
}

/**
* Custom toMap collectors to avoid merge due to bug in the JDK, it throws NPE on null values in map.
* https://bugs.openjdk.org/browse/JDK-8148463
*/
public static <T, K, V> Collector<T, Map<K, V>, Map<K, V>> toMap(
final Function<? super T, K> keyMapper,
final Function<T, V> valueMapper
)
{
return Collector.of(
HashMap::new,
(kvMap, t) -> {
kvMap.put(keyMapper.apply(t), valueMapper.apply(t));
},
(kvMap, kvMap2) -> {
kvMap.putAll(kvMap2);
return kvMap;
},
Function.identity(),
Collector.Characteristics.IDENTITY_FINISH
);
}
}
214 changes: 103 additions & 111 deletions processing/src/main/java/org/apache/druid/java/util/common/parsers/ObjectFlatteners.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,15 @@

package org.apache.druid.java.util.common.parsers;

import com.google.common.base.Functions;
import com.google.common.collect.Iterables;
import com.jayway.jsonpath.spi.json.JsonProvider;
import org.apache.druid.guice.annotations.ExtensionPoint;
import org.apache.druid.java.util.common.IAE;
import org.apache.druid.java.util.common.UOE;
import org.apache.druid.java.util.common.collect.Utils;

import javax.annotation.Nullable;
import java.util.AbstractMap;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
Expand Down Expand Up @@ -80,135 +81,126 @@ public static <T> ObjectFlattener<T> create(

return new ObjectFlattener<T>()
{
private final Set<String> extractorsKeySet = extractors.keySet();

@Override
public Map<String, Object> flatten(final T obj)
{
return new AbstractMap<String, Object>()
{
@Override
public int size()
{
return keySet().size();
}

@Override
public boolean isEmpty()
{
return keySet().isEmpty();
}

@Override
public boolean containsKey(final Object key)
{
if (key == null) {
return false;
}

return keySet().contains(key.toString());
}

@Override
public boolean containsValue(final Object value)
{
throw new UnsupportedOperationException();
Set<String> keySet = getRequiredKeySet(obj);
Map<String, Object> map = getCustomHashMap(keySet.stream().collect(Utils.toMap(Functions.identity(), key -> {
final Function<T, Object> extractor = extractors.get(key);
if (extractor != null) {
return extractor.apply(obj);
} else {
return flattenerMaker.getRootField(obj, key);
}
})));
return map;
}

@Override
public Object get(final Object key)
{
final String keyString = key.toString();
final Function<T, Object> extractor = extractors.get(keyString);
if (extractor != null) {
return extractor.apply(obj);
} else {
return flattenerMaker.getRootField(obj, keyString);
}
private Set<String> getRequiredKeySet(final T obj)
{
if (flattenSpec.isUseFieldDiscovery()) {
final Iterable<String> rootFields = flattenerMaker.discoverRootFields(obj);
if (extractors.isEmpty() && rootFields instanceof Set) {
return (Set<String>) rootFields;
} else {
final Set<String> keys = new LinkedHashSet<>(this.extractorsKeySet);
Iterables.addAll(keys, rootFields);
return keys;
}
} else {
return this.extractorsKeySet;
}
}

@Override
public Object put(final String key, final Object value)
{
throw new UnsupportedOperationException();
}
@Override
public Map<String, Object> toMap(T obj)
{
return flattenerMaker.toMap(obj);
}
};
}

@Override
public Object remove(final Object key)
{
throw new UnsupportedOperationException();
}
private static Map<String, Object> getCustomHashMap(Map<String, Object> initMap)
{
return new HashMap<String, Object>(initMap)
{
@Override
public boolean containsKey(final Object key)
{
if (key == null) {
return false;
}

@Override
public void putAll(final Map<? extends String, ?> m)
{
throw new UnsupportedOperationException();
}
return keySet().contains(key.toString());
}

@Override
public void clear()
{
throw new UnsupportedOperationException();
}
@Override
public boolean containsValue(final Object value)
{
throw new UnsupportedOperationException();
}

@Override
public Set<String> keySet()
{
if (flattenSpec.isUseFieldDiscovery()) {
final Iterable<String> rootFields = flattenerMaker.discoverRootFields(obj);
if (extractors.isEmpty() && rootFields instanceof Set) {
return (Set<String>) rootFields;
} else {
final Set<String> keys = new LinkedHashSet<>(extractors.keySet());
Iterables.addAll(keys, rootFields);
return keys;
}
} else {
return extractors.keySet();
}
}
@Override
public Object put(final String key, final Object value)
{
throw new UnsupportedOperationException();
}

@Override
public Collection<Object> values()
{
throw new UnsupportedOperationException();
}
@Override
public Object remove(final Object key)
{
throw new UnsupportedOperationException();
}

@Override
public Set<Entry<String, Object>> entrySet()
{
return keySet().stream()
.map(
field -> {
return new Entry<String, Object>()
{
@Override
public String getKey()
{
return field;
}
@Override
public void putAll(final Map<? extends String, ?> m)
{
throw new UnsupportedOperationException();
}

@Override
public Object getValue()
{
return get(field);
}
@Override
public void clear()
{
throw new UnsupportedOperationException();
}

@Override
public Object setValue(final Object value)
{
throw new UnsupportedOperationException();
}
};
}
)
.collect(Collectors.toCollection(LinkedHashSet::new));
}
};
@Override
public Collection<Object> values()
{
throw new UnsupportedOperationException();
}

@Override
public Map<String, Object> toMap(T obj)
public Set<Entry<String, Object>> entrySet()
{
return flattenerMaker.toMap(obj);
return keySet().stream()
.map(
field -> {
return new Entry<String, Object>()
{
@Override
public String getKey()
{
return field;
}

@Override
public Object getValue()
{
return get(field);
}

@Override
public Object setValue(final Object value)
{
throw new UnsupportedOperationException();
}
};
}
)
.collect(Collectors.toCollection(LinkedHashSet::new));
}
};
}
Expand Down
37 changes: 35 additions & 2 deletions processing/src/test/java/org/apache/druid/java/util/common/parsers/ObjectFlattenersTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import com.google.common.collect.ImmutableSet;
import org.junit.Assert;
import org.junit.Test;

import java.util.Map;

public class ObjectFlattenersTest
Expand All @@ -54,7 +53,7 @@ public void testFlatten() throws JsonProcessingException
Assert.assertNull(flat.get("foo"));
Assert.assertEquals(1L, flat.get("bar"));
Assert.assertEquals(1L, flat.get("extract"));
Assert.assertEquals("{\"extract\":1,\"foo\":null,\"bar\":1}", OBJECT_MAPPER.writeValueAsString(flat));
Assert.assertEquals(3, flat.size());
}

@Test
Expand All @@ -74,4 +73,38 @@ public void testToMapNull() throws JsonProcessingException
Assert.assertNull(FLATTENER_MAKER.toPlainJavaType(node));
Assert.assertEquals(ImmutableMap.of(), flat);
}

@Test
public void testNestedFlattenBadExpression() throws JsonProcessingException
{
ObjectFlattener flattener = ObjectFlatteners.create(
new JSONPathSpec(
true,
ImmutableList.of(new JSONPathFieldSpec(JSONPathFieldType.PATH, "extract", "$.badfoo.bar.foobar.f1"))
),
FLATTENER_MAKER
);
JsonNode node = OBJECT_MAPPER.readTree("{\"foo\":{\"bar\":{\"foobar\":{\"f1\":100}}}}");
Map<String, Object> flat = flattener.flatten(node);
Assert.assertNotNull(flat);
Assert.assertEquals(flat.size(), 1);
Assert.assertEquals(null, flat.get("extract"));
}

@Test
public void testNestedFlatten() throws JsonProcessingException
{
ObjectFlattener flattener = ObjectFlatteners.create(
new JSONPathSpec(
true,
ImmutableList.of(new JSONPathFieldSpec(JSONPathFieldType.PATH, "extract", "$.foo.bar.foobar.f1"))
),
FLATTENER_MAKER
);
JsonNode node = OBJECT_MAPPER.readTree("{\"foo\":{\"bar\":{\"foobar\":{\"f1\":100}}}}");
Map<String, Object> flat = flattener.flatten(node);
Assert.assertNotNull(flat);
Assert.assertEquals(flat.size(), 1);
Assert.assertEquals(100L, flat.get("extract"));
}
}