diff --git a/logstash-core/src/main/java/org/logstash/ConvertedMap.java b/logstash-core/src/main/java/org/logstash/ConvertedMap.java index 2cf235889..99d33a926 100644 --- a/logstash-core/src/main/java/org/logstash/ConvertedMap.java +++ b/logstash-core/src/main/java/org/logstash/ConvertedMap.java @@ -99,6 +99,6 @@ public final class ConvertedMap extends IdentityHashMap { * @return Interned String */ private static String convertKey(final RubyString key) { - return FieldReference.from(key.getByteList()).getKey(); + return FieldReference.from(key).getKey(); } } diff --git a/logstash-core/src/main/java/org/logstash/FieldReference.java b/logstash-core/src/main/java/org/logstash/FieldReference.java index ade18c419..041500a64 100644 --- a/logstash-core/src/main/java/org/logstash/FieldReference.java +++ b/logstash-core/src/main/java/org/logstash/FieldReference.java @@ -5,6 +5,7 @@ import java.util.Arrays; import java.util.HashMap; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import org.jruby.RubyString; public final class FieldReference { @@ -41,9 +42,15 @@ public final class FieldReference { new FieldReference(EMPTY_STRING_ARRAY, Event.METADATA, META_PARENT); /** - * Cache of all existing {@link FieldReference}. + * Cache of all existing {@link FieldReference} by their {@link RubyString} source. */ - private static final Map CACHE = + private static final Map RUBY_CACHE = + new ConcurrentHashMap<>(64, 0.2F, 1); + + /** + * Cache of all existing {@link FieldReference} by their {@link String} source. + */ + private static final Map CACHE = new ConcurrentHashMap<>(64, 0.2F, 1); private final String[] path; @@ -65,7 +72,16 @@ public final class FieldReference { hash = calculateHash(this.key, this.path, this.type); } - public static FieldReference from(final CharSequence reference) { + public static FieldReference from(final RubyString reference) { + // atomicity between the get and put is not important + final FieldReference result = RUBY_CACHE.get(reference); + if (result != null) { + return result; + } + return RUBY_CACHE.computeIfAbsent(reference.newFrozen(), ref -> from(ref.asJavaString())); + } + + public static FieldReference from(final String reference) { // atomicity between the get and put is not important final FieldReference result = CACHE.get(reference); if (result != null) { @@ -138,7 +154,7 @@ public final class FieldReference { return prime * hash + type; } - private static FieldReference parseToCache(final CharSequence reference) { + private static FieldReference parseToCache(final String reference) { FieldReference result = parse(reference); if (CACHE.size() < 10_000) { result = deduplicate(result); diff --git a/logstash-core/src/main/java/org/logstash/ext/JrubyEventExtLibrary.java b/logstash-core/src/main/java/org/logstash/ext/JrubyEventExtLibrary.java index 6e2ec8548..debb83037 100644 --- a/logstash-core/src/main/java/org/logstash/ext/JrubyEventExtLibrary.java +++ b/logstash-core/src/main/java/org/logstash/ext/JrubyEventExtLibrary.java @@ -82,14 +82,14 @@ public final class JrubyEventExtLibrary { { return Rubyfier.deep( context.runtime, - this.event.getUnconvertedField(FieldReference.from(reference.getByteList())) + this.event.getUnconvertedField(FieldReference.from(reference)) ); } @JRubyMethod(name = "set", required = 2) public IRubyObject ruby_set_field(ThreadContext context, RubyString reference, IRubyObject value) { - final FieldReference r = FieldReference.from(reference.getByteList()); + final FieldReference r = FieldReference.from(reference); if (r.equals(FieldReference.TIMESTAMP_REFERENCE)) { if (!(value instanceof JrubyTimestampExtLibrary.RubyTimestamp)) { throw context.runtime.newTypeError("wrong argument type " + value.getMetaClass() + " (expected LogStash::Timestamp)"); @@ -124,7 +124,7 @@ public final class JrubyEventExtLibrary { @JRubyMethod(name = "include?", required = 1) public IRubyObject ruby_includes(ThreadContext context, RubyString reference) { return RubyBoolean.newBoolean( - context.runtime, this.event.includes(FieldReference.from(reference.getByteList())) + context.runtime, this.event.includes(FieldReference.from(reference)) ); } @@ -132,7 +132,7 @@ public final class JrubyEventExtLibrary { public IRubyObject ruby_remove(ThreadContext context, RubyString reference) { return Rubyfier.deep( context.runtime, - this.event.remove(FieldReference.from(reference.getByteList())) + this.event.remove(FieldReference.from(reference)) ); } diff --git a/logstash-core/src/test/java/org/logstash/AccessorsTest.java b/logstash-core/src/test/java/org/logstash/AccessorsTest.java index 51415979f..34751a0b8 100644 --- a/logstash-core/src/test/java/org/logstash/AccessorsTest.java +++ b/logstash-core/src/test/java/org/logstash/AccessorsTest.java @@ -194,20 +194,20 @@ public class AccessorsTest { set(data, "[foo][bar]", "Another String"); } - private static Object get(final ConvertedMap data, final CharSequence reference) { + private static Object get(final ConvertedMap data, final String reference) { return Accessors.get(data, FieldReference.from(reference)); } - private static Object set(final ConvertedMap data, final CharSequence reference, + private static Object set(final ConvertedMap data, final String reference, final Object value) { return Accessors.set(data, FieldReference.from(reference), value); } - private static Object del(final ConvertedMap data, final CharSequence reference) { + private static Object del(final ConvertedMap data, final String reference) { return Accessors.del(data, FieldReference.from(reference)); } - private static boolean includes(final ConvertedMap data, final CharSequence reference) { + private static boolean includes(final ConvertedMap data, final String reference) { return Accessors.includes(data, FieldReference.from(reference)); } } diff --git a/logstash-core/src/test/java/org/logstash/FieldReferenceTest.java b/logstash-core/src/test/java/org/logstash/FieldReferenceTest.java index cd757efda..63fd16f67 100644 --- a/logstash-core/src/test/java/org/logstash/FieldReferenceTest.java +++ b/logstash-core/src/test/java/org/logstash/FieldReferenceTest.java @@ -51,7 +51,7 @@ public final class FieldReferenceTest { final FieldReference emptyReference = FieldReference.from(""); assertNotNull(emptyReference); assertEquals( - emptyReference, FieldReference.from(RubyUtil.RUBY.newString("").getByteList()) + emptyReference, FieldReference.from(RubyUtil.RUBY.newString("")) ); } diff --git a/logstash-core/src/test/java/org/logstash/ext/JrubyEventExtLibraryTest.java b/logstash-core/src/test/java/org/logstash/ext/JrubyEventExtLibraryTest.java index e531582f7..8cd1e5327 100644 --- a/logstash-core/src/test/java/org/logstash/ext/JrubyEventExtLibraryTest.java +++ b/logstash-core/src/test/java/org/logstash/ext/JrubyEventExtLibraryTest.java @@ -43,6 +43,18 @@ public final class JrubyEventExtLibraryTest { } } + @Test + public void correctlyHandlesNonAsciiKeys() { + final RubyString key = rubyString("[テストフィールド]"); + final RubyString value = rubyString("someValue"); + final ThreadContext context = RubyUtil.RUBY.getCurrentContext(); + final JrubyEventExtLibrary.RubyEvent event = + JrubyEventExtLibrary.RubyEvent.newRubyEvent(context.runtime); + event.ruby_set_field(context, key, value); + Assertions.assertThat(event.ruby_to_json(context, new IRubyObject[0]).asJavaString()) + .contains("\"テストフィールド\":\"someValue\""); + } + private static RubyString rubyString(final String java) { return RubyUtil.RUBY.newString(java); }