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.

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

Bug/12065 refac glob matching #12225

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 31 additions & 26 deletions libs/common/src/main/java/org/opensearch/common/Glob.java
Original file line number Diff line number Diff line change
Expand Up @@ -49,37 +49,42 @@ public class Glob {
* @return whether the String matches the given pattern
*/
public static boolean globMatch(String pattern, String str) {

if (pattern == null || str == null) {
return false;
}
int firstIndex = pattern.indexOf('*');
if (firstIndex == -1) {
return pattern.equals(str);
}
if (firstIndex == 0) {
if (pattern.length() == 1) {
return true;
}
int nextIndex = pattern.indexOf('*', firstIndex + 1);
if (nextIndex == -1) {
return str.endsWith(pattern.substring(1));
} else if (nextIndex == 1) {
// Double wildcard "**" - skipping the first "*"
return globMatch(pattern.substring(1), str);
}
String part = pattern.substring(1, nextIndex);
int partIndex = str.indexOf(part);
while (partIndex != -1) {
if (globMatch(pattern.substring(nextIndex), str.substring(partIndex + part.length()))) {
return true;
}
partIndex = str.indexOf(part, partIndex + 1);

int stringIndex = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this logic any different then Regex.simpleMatchWithNormalizedStrings. If they are identical then can they be written and tested in a common place?

int patternIndex = 0;
int wildcardIndex = -1;
int wildcardStringIndex = -1;

while (stringIndex < str.length()) {
// pattern and string match
if (patternIndex < pattern.length() && str.charAt(stringIndex) == pattern.charAt(patternIndex)) {
stringIndex++;
patternIndex++;
} else if (patternIndex < pattern.length() && pattern.charAt(patternIndex) == '*') {
// wildcard found
wildcardIndex = patternIndex;
patternIndex++;
wildcardStringIndex = stringIndex;
} else if (wildcardIndex != -1) {
// last pattern pointer was a wildcard
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you a comment on what this block does? My understanding is that there was not a match, so you reset pointers from last wildcard and restart the search

patternIndex = wildcardIndex + 1;
wildcardStringIndex++;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should wildCardIndex be set to -1 here? After one match, we can forever fall this if block right?

stringIndex = wildcardStringIndex;
} else {
// characters do not match
return false;
}
return false;
}
return (str.length() >= firstIndex
&& pattern.substring(0, firstIndex).equals(str.substring(0, firstIndex))
&& globMatch(pattern.substring(firstIndex), str.substring(firstIndex)));

while (patternIndex < pattern.length() && pattern.charAt(patternIndex) == '*') {
patternIndex++;
}

return patternIndex == pattern.length();
}

}
88 changes: 88 additions & 0 deletions libs/common/src/test/java/org/opensearch/common/GlobTests.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.common;

import org.opensearch.test.OpenSearchTestCase;

public class GlobTests extends OpenSearchTestCase {

public void testGlobMatchNoWildcard() {
assertTrue(Glob.globMatch("test", "test"));
assertFalse(Glob.globMatch("test", "testing"));
}

public void testGlobMatchWildcardAtBeginning() {
assertTrue(Glob.globMatch("*test", "thisisatest"));
assertFalse(Glob.globMatch("*test", "thisisatesting"));
}

public void testGlobMatchWildcardAtEnd() {
assertTrue(Glob.globMatch("test*", "testthisisa"));
assertFalse(Glob.globMatch("test*", "atestthisis"));
}

public void testGlobMatchWildcardAtMiddle() {
assertTrue(Glob.globMatch("test*ing", "testthisisaing"));
assertFalse(Glob.globMatch("test*ing", "testthisisa"));
}

public void testGlobMatchMultipleWildcards() {
assertTrue(Glob.globMatch("*test*", "thisisatesting"));
robinf95 marked this conversation as resolved.
Show resolved Hide resolved
assertFalse(Glob.globMatch("*test*", "thisisatesing"));
assertTrue(Glob.globMatch("*test*test", "thisisatestingtest"));
assertFalse(Glob.globMatch("*test*test", "thisisatesting"));
}

public void testGlobalMatchDoubleWildcard() {
assertTrue(Glob.globMatch("**test", "thisisatest"));
assertFalse(Glob.globMatch("**test", "thisisatesting"));
assertTrue(Glob.globMatch("test**", "testthisisa"));
assertFalse(Glob.globMatch("test**", "atestthisis"));
assertTrue(Glob.globMatch("**test**", "thisisatesting"));
assertFalse(Glob.globMatch("**test**", "thisisatesing"));
assertTrue(Glob.globMatch("**test**test", "thisisatestingtest"));
assertFalse(Glob.globMatch("**test**test", "thisisatesting"));
}

public void testGlobMatchMultipleCharactersWithSingleWildcard() {
assertTrue(Glob.globMatch("a*b", "acb"));
assertTrue(Glob.globMatch("f*o", "foo"));
assertTrue(Glob.globMatch("f*oo", "foo"));
assertTrue(Glob.globMatch("a*b", "aab"));
assertTrue(Glob.globMatch("a*b", "aaab"));
assertFalse(Glob.globMatch("a*b", "ac"));
}

public void testGlobMatchWildcardWithEmptyString() {
assertTrue(Glob.globMatch("*", ""));
assertTrue(Glob.globMatch("a*", "a"));
assertFalse(Glob.globMatch("a*", ""));
}

public void testGlobMatchMultipleWildcardsWithMultipleCharacters() {
assertTrue(Glob.globMatch("a*b*c", "abc"));
assertTrue(Glob.globMatch("a*b*c", "axxxbxbc"));
assertTrue(Glob.globMatch("a*b*c", "aabc"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test for match("best*test", "bestbuy")

assertTrue(Glob.globMatch("a*b*c", "abac"));
assertFalse(Glob.globMatch("a*b*c", "abca"));
assertFalse(Glob.globMatch("a*b*c", "ac"));
}

public void testGlobMatchNullPattern() {
assertFalse(Glob.globMatch(null, "test"));
}

public void testGlobMatchNullString() {
assertFalse(Glob.globMatch("test", null));
}

public void testGlobMatchNullPatternAndString() {
assertFalse(Glob.globMatch(null, null));
}
}
Loading