-
Notifications
You must be signed in to change notification settings - Fork 3.8k
[#13307] Refactor parsing logic to use KeyValueTokenizer
#13315
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -24,6 +24,7 @@ | |||||||||||
| import com.navercorp.pinpoint.bootstrap.plugin.jdbc.StringMaker; | ||||||||||||
| import com.navercorp.pinpoint.bootstrap.plugin.jdbc.UnKnownDatabaseInfo; | ||||||||||||
| import com.navercorp.pinpoint.common.trace.ServiceType; | ||||||||||||
| import com.navercorp.pinpoint.common.util.KeyValueTokenizer; | ||||||||||||
| import com.navercorp.pinpoint.common.util.StringUtils; | ||||||||||||
|
|
||||||||||||
| import java.util.Arrays; | ||||||||||||
|
|
@@ -73,10 +74,10 @@ private DatabaseInfo parseURL(String url) { | |||||||||||
| String host = DEFAULT_HOST; | ||||||||||||
| String port = DEFAULT_PORT; | ||||||||||||
| if (StringUtils.hasText(hostPort)) { | ||||||||||||
| String[] hostPortInfo = hostPort.split(":", 2); | ||||||||||||
| host = hostPortInfo[0]; | ||||||||||||
| if (hostPortInfo.length > 1) { | ||||||||||||
| port = hostPortInfo[1]; | ||||||||||||
| KeyValueTokenizer.KeyValue hostPortInfo = KeyValueTokenizer.tokenize(hostPort, ":"); | ||||||||||||
| host = hostPortInfo.getKey(); | ||||||||||||
| if (!hostPortInfo.getValue().isEmpty()) { | ||||||||||||
| port = hostPortInfo.getValue(); | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
|
|
@@ -122,11 +123,11 @@ private Map<String, String> parseQuery(String propString) { | |||||||||||
| if (!StringUtils.hasText(v)) { | ||||||||||||
| continue; | ||||||||||||
| } | ||||||||||||
| String[] kv = v.split("=", 2); | ||||||||||||
| if (kv.length > 1) { | ||||||||||||
| map.put(kv[0], kv[1]); | ||||||||||||
| KeyValueTokenizer.KeyValue keyValue = KeyValueTokenizer.tokenize(v, "="); | ||||||||||||
|
||||||||||||
| KeyValueTokenizer.KeyValue keyValue = KeyValueTokenizer.tokenize(v, "="); | |
| KeyValueTokenizer.KeyValue keyValue = KeyValueTokenizer.tokenize(v, "="); | |
| if (keyValue == null) { | |
| continue; | |
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -16,6 +16,8 @@ | |||||||||||||||||||
|
|
||||||||||||||||||||
| package com.navercorp.pinpoint.collector.grpc.channelz.service; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import com.navercorp.pinpoint.common.util.KeyValueTokenizer; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import javax.annotation.Nullable; | ||||||||||||||||||||
| import java.util.Collection; | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
@@ -48,16 +50,16 @@ private SocketEntry(String remoteAddr, Integer localPort, long socketId) { | |||||||||||||||||||
| * @return entry of socket index | ||||||||||||||||||||
| */ | ||||||||||||||||||||
| public static SocketEntry compose(Object remote, Object local, long socketId) { | ||||||||||||||||||||
| String remoteAddr = split(remote)[0]; | ||||||||||||||||||||
| String localPort = split(local)[1]; | ||||||||||||||||||||
| String remoteAddr = split(remote).getKey(); | ||||||||||||||||||||
| String localPort = split(local).getValue(); | ||||||||||||||||||||
|
Comment on lines
+53
to
+54
|
||||||||||||||||||||
| String remoteAddr = split(remote).getKey(); | |
| String localPort = split(local).getValue(); | |
| KeyValueTokenizer.KeyValue remoteKeyValue = split(remote); | |
| KeyValueTokenizer.KeyValue localKeyValue = split(local); | |
| if (remoteKeyValue == null || localKeyValue == null) { | |
| throw new IllegalArgumentException("Invalid remote or local address: remote=" + remote + ", local=" + local); | |
| } | |
| String remoteAddr = remoteKeyValue.getKey(); | |
| String localPort = localKeyValue.getValue(); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| package com.navercorp.pinpoint.common.util; | ||
|
|
||
| import org.junit.jupiter.api.Assertions; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
|
|
||
| class KeyValueTokenizerTest { | ||
|
|
||
| @Test | ||
| void tokenize() { | ||
| KeyValueTokenizer.KeyValue keyValue = KeyValueTokenizer.tokenize("key=value", "="); | ||
| assertEquals("key", keyValue.getKey()); | ||
| assertEquals("value", keyValue.getValue()); | ||
| } | ||
|
|
||
| @Test | ||
| void tokenize2() { | ||
| KeyValueTokenizer.KeyValue keyValue = KeyValueTokenizer.tokenize("key==value", "=="); | ||
| assertEquals("key", keyValue.getKey()); | ||
| assertEquals("value", keyValue.getValue()); | ||
| } | ||
|
|
||
| @Test | ||
| void tokenize_emptyValue() { | ||
| KeyValueTokenizer.KeyValue keyValue = KeyValueTokenizer.tokenize("key=", "="); | ||
| assertEquals("key", keyValue.getKey()); | ||
| assertEquals("", keyValue.getValue()); | ||
| } | ||
|
|
||
| @Test | ||
| void tokenize_emptyKeyValue() { | ||
| KeyValueTokenizer.KeyValue keyValue = KeyValueTokenizer.tokenize("=", "="); | ||
| assertEquals("", keyValue.getKey()); | ||
| assertEquals("", keyValue.getValue()); | ||
| } | ||
|
|
||
| @Test | ||
| void tokenize_empty() { | ||
| KeyValueTokenizer.KeyValue keyValue = KeyValueTokenizer.tokenize("", "="); | ||
| Assertions.assertNull(keyValue); | ||
| } | ||
| } | ||
|
Comment on lines
+8
to
+43
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,71 @@ | ||||||||||||||||||||||||||
| package com.navercorp.pinpoint.common.util; | ||||||||||||||||||||||||||
| import java.util.Objects; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| public class KeyValueTokenizer { | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| public static final TokenFactory<KeyValue> KEY_VALUE_FACTORY = new TokenFactory<KeyValue>() { | ||||||||||||||||||||||||||
| public KeyValue accept(String key, String value) { | ||||||||||||||||||||||||||
| return new KeyValue(key, value); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| public static final TokenFactory<KeyValue> KEY_VALUE_TRIM_FACTORY = new TokenFactory<KeyValue>() { | ||||||||||||||||||||||||||
| public KeyValue accept(String key, String value) { | ||||||||||||||||||||||||||
| return new KeyValue(key.trim(), value.trim()); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| public static KeyValue tokenize(String text, String delimiter) { | ||||||||||||||||||||||||||
| return tokenize(text, delimiter, KEY_VALUE_FACTORY); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||
| * Tokenizes the given text into a key and value using the specified delimiter and token factory. | ||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||
| * @param text the text to tokenize | ||||||||||||||||||||||||||
| * @param delimiter the delimiter separating key and value | ||||||||||||||||||||||||||
| * @param factory the factory used to create the token | ||||||||||||||||||||||||||
| * @param <T> the type of token to return | ||||||||||||||||||||||||||
| * @return the parsed token, or {@code null} if the delimiter is not found in the text | ||||||||||||||||||||||||||
|
Comment on lines
+25
to
+29
|
||||||||||||||||||||||||||
| * @param text the text to tokenize | |
| * @param delimiter the delimiter separating key and value | |
| * @param factory the factory used to create the token | |
| * @param <T> the type of token to return | |
| * @return the parsed token, or {@code null} if the delimiter is not found in the text | |
| * @param text the text to tokenize; must not be {@code null} | |
| * @param delimiter the delimiter separating key and value | |
| * @param factory the factory used to create the token | |
| * @param <T> the type of token to return | |
| * @return the parsed token, or {@code null} if the delimiter is not found in the text; callers should | |
| * check for a {@code null} return value | |
| * @throws NullPointerException if {@code text} is {@code null} |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,6 +19,7 @@ | |||||||||
| import com.google.common.primitives.Ints; | ||||||||||
| import com.navercorp.pinpoint.common.server.util.StringPrecondition; | ||||||||||
| import com.navercorp.pinpoint.common.timeseries.window.TimePrecision; | ||||||||||
| import com.navercorp.pinpoint.common.util.KeyValueTokenizer; | ||||||||||
| import com.navercorp.pinpoint.common.util.StringUtils; | ||||||||||
| import com.navercorp.pinpoint.metric.web.util.QueryParameter; | ||||||||||
|
|
||||||||||
|
|
@@ -197,8 +198,8 @@ public Builder addAllFilters(Collection<String> strings) { | |||||||||
| return self(); | ||||||||||
| } | ||||||||||
| for (String string : strings) { | ||||||||||
| String[] tag = string.split(":", 2); | ||||||||||
| filterByAttributes.put(tag[0], tag[1]); | ||||||||||
| KeyValueTokenizer.KeyValue keyValue = KeyValueTokenizer.tokenize(string, ":"); | ||||||||||
| filterByAttributes.put(keyValue.getKey(), keyValue.getValue()); | ||||||||||
|
||||||||||
| filterByAttributes.put(keyValue.getKey(), keyValue.getValue()); | |
| if (keyValue != null) { | |
| filterByAttributes.put(keyValue.getKey(), keyValue.getValue()); | |
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -18,6 +18,7 @@ | |||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| import com.navercorp.pinpoint.common.util.CollectionUtils; | ||||||||||||||
| import com.navercorp.pinpoint.common.util.KeyValueTokenizer; | ||||||||||||||
| import com.navercorp.pinpoint.metric.common.model.Tag; | ||||||||||||||
| import org.apache.commons.lang3.StringUtils; | ||||||||||||||
|
|
||||||||||||||
|
|
@@ -33,7 +34,14 @@ | |||||||||||||
| public class TagUtils { | ||||||||||||||
|
|
||||||||||||||
| private static final Pattern MULTI_VALUE_FIELD_PATTERN = Pattern.compile("[\\[\\]\"]"); | ||||||||||||||
| private static final Pattern JSON_TAG_STRING_PATTERN = Pattern.compile("[{}\"]"); | ||||||||||||||
| private static final String JSON_TAG_STRING = "{}\""; | ||||||||||||||
|
|
||||||||||||||
| public static final KeyValueTokenizer.TokenFactory<Tag> TAG_FACTORY = new KeyValueTokenizer.TokenFactory<>() { | ||||||||||||||
| @Override | ||||||||||||||
| public Tag accept(String key, String value) { | ||||||||||||||
| return new Tag(key, value); | ||||||||||||||
| } | ||||||||||||||
| }; | ||||||||||||||
|
|
||||||||||||||
| private TagUtils() { | ||||||||||||||
| } | ||||||||||||||
|
|
@@ -69,16 +77,7 @@ public static List<Tag> parseTags(String tagStrings) { | |||||||||||||
|
|
||||||||||||||
| public static Tag parseTag(String tagString) { | ||||||||||||||
| Objects.requireNonNull(tagString, "tagString"); | ||||||||||||||
|
|
||||||||||||||
| String[] tag = StringUtils.split(tagString, ":", 2); | ||||||||||||||
|
|
||||||||||||||
| if (tag.length == 1) { | ||||||||||||||
| return new Tag(tag[0], ""); | ||||||||||||||
| } else if (tag.length == 2) { | ||||||||||||||
| return new Tag(tag[0], tag[1]); | ||||||||||||||
| } else { | ||||||||||||||
| throw new IllegalArgumentException("tagString:" + tagString); | ||||||||||||||
| } | ||||||||||||||
| return KeyValueTokenizer.tokenize(tagString, ":", TAG_FACTORY); | ||||||||||||||
|
||||||||||||||
| return KeyValueTokenizer.tokenize(tagString, ":", TAG_FACTORY); | |
| Tag tag = KeyValueTokenizer.tokenize(tagString, ":", TAG_FACTORY); | |
| if (tag == null) { | |
| throw new IllegalArgumentException("Invalid tag format, expected 'key:value' but was: " + tagString); | |
| } | |
| return tag; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,6 +41,14 @@ public void parseTagTest() { | |
| Assertions.assertEquals(tagList, result); | ||
| } | ||
|
|
||
| @Test | ||
| public void parseTagTest_emptyValue() { | ||
| Tag tag = TagUtils.parseTag("A:"); | ||
|
|
||
| Assertions.assertEquals("A", tag.getName()); | ||
| Assertions.assertEquals("", tag.getValue()); | ||
| } | ||
|
Comment on lines
+44
to
+50
|
||
|
|
||
| @Test | ||
| public void parseTagsListTest() { | ||
| List<Tag> tagList = List.of( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing null check after calling KeyValueTokenizer.tokenize(). The tokenize method returns null when the delimiter is not found in the text. Without a null check, this will throw a NullPointerException when calling getKey() or getValue() on a null result. Add a null check similar to the one used in RedisKVPubChannelProvider at line 40.