Bug/12065 refac glob matching #12225

robinf95 · 2024-02-07T08:32:56Z

Description

Refactored Glob.globMatch by using iterative approach

Related Issues

Resolves #12065

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
Commits are signed per the DCO using --signoff
Commit changes are listed out in CHANGELOG.md file (See: Changelog)
Public documentation issue/PR created

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Robin Friedmann <[email protected]>

github-actions · 2024-02-07T08:37:15Z

❌ Gradle check result for fa59c6c: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2024-02-07T08:53:19Z

Compatibility status:

Checks if related components are compatible with change 5b6ee3f

Incompatible components

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/flow-framework.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/performance-analyzer.git]

deshsidd · 2024-02-07T23:13:19Z

Build still failing please check and resubmit.

libs/common/src/main/java/org/opensearch/common/Glob.java

libs/common/src/test/java/org/opensearch/common/GlobTests.java

Signed-off-by: Robin Friedmann <[email protected]> Refactored Glob.globMatch by using iterative approach; added unit tests (opensearch-project#12065) Signed-off-by: Robin Friedmann <[email protected]>

github-actions · 2024-02-09T07:40:38Z

❌ Gradle check result for 79117b6: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Robin Friedmann <[email protected]>

github-actions · 2024-02-09T09:27:39Z

❌ Gradle check result for 5b6ee3f: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

kkmr · 2024-02-11T16:53:41Z

libs/common/src/main/java/org/opensearch/common/Glob.java

+                patternIndex++;
+                wildcardStringIndex = stringIndex;
+            } else if (wildcardIndex != -1) {
+                // last pattern pointer was a wildcard


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

kkmr · 2024-02-11T16:57:05Z

libs/common/src/test/java/org/opensearch/common/GlobTests.java

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


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

kkmr · 2024-02-12T20:48:57Z

libs/common/src/main/java/org/opensearch/common/Glob.java

+            } else if (wildcardIndex != -1) {
+                // last pattern pointer was a wildcard
+                patternIndex = wildcardIndex + 1;
+                wildcardStringIndex++;


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

opensearch-trigger-bot · 2024-03-14T15:20:00Z

This PR is stalled because it has been open for 30 days with no activity.

cwperks · 2024-03-25T14:59:26Z

libs/common/src/main/java/org/opensearch/common/Glob.java

-                }
-                partIndex = str.indexOf(part, partIndex + 1);
+
+        int stringIndex = 0;


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

opensearch-trigger-bot · 2024-04-25T15:21:01Z

This PR is stalled because it has been open for 30 days with no activity.

kkhatua · 2024-07-30T18:12:30Z

@cwperks
The original issue #12065 seems to be resolved by #13104 .
Can we close this PR as it is not needed?

cwperks · 2024-07-30T18:21:34Z

@kkhatua Yes, this can be closed.

Implement unit tests for Glob.globMatch (opensearch-project#12065)

758a388

Signed-off-by: Robin Friedmann <[email protected]>

github-actions bot added bug Something isn't working good first issue Good for newcomers Search Search query, autocomplete ...etc labels Feb 7, 2024

github-actions bot added good first issue Good for newcomers Search Search query, autocomplete ...etc labels Feb 7, 2024

deshsidd reviewed Feb 7, 2024

View reviewed changes

libs/common/src/main/java/org/opensearch/common/Glob.java Outdated Show resolved Hide resolved

libs/common/src/test/java/org/opensearch/common/GlobTests.java Show resolved Hide resolved

Fix failing test(opensearch-project#12065)

79117b6

Signed-off-by: Robin Friedmann <[email protected]> Refactored Glob.globMatch by using iterative approach; added unit tests (opensearch-project#12065) Signed-off-by: Robin Friedmann <[email protected]>

robinf95 force-pushed the bug/12065-refac-glob-matching branch from fa59c6c to 79117b6 Compare February 9, 2024 07:29

Adjust Glob.globMatch algorithm; Add tests (opensearch-project#12065)

5b6ee3f

Signed-off-by: Robin Friedmann <[email protected]>

robinf95 requested a review from deshsidd February 10, 2024 07:20

kkmr reviewed Feb 11, 2024

View reviewed changes

kkmr reviewed Feb 12, 2024

View reviewed changes

opensearch-trigger-bot bot added the stalled Issues that have stalled label Mar 14, 2024

cwperks reviewed Mar 25, 2024

View reviewed changes

opensearch-trigger-bot bot removed the stalled Issues that have stalled label Mar 26, 2024

opensearch-trigger-bot bot added the stalled Issues that have stalled label Apr 25, 2024

kkhatua closed this Jul 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug/12065 refac glob matching #12225

Bug/12065 refac glob matching #12225

robinf95 commented Feb 7, 2024

github-actions bot commented Feb 7, 2024

github-actions bot commented Feb 7, 2024 •

edited

Loading

deshsidd commented Feb 7, 2024

github-actions bot commented Feb 9, 2024

github-actions bot commented Feb 9, 2024

kkmr Feb 11, 2024

kkmr Feb 11, 2024

kkmr Feb 12, 2024

opensearch-trigger-bot bot commented Mar 14, 2024

cwperks Mar 25, 2024

opensearch-trigger-bot bot commented Apr 25, 2024

kkhatua commented Jul 30, 2024

cwperks commented Jul 30, 2024

Bug/12065 refac glob matching #12225

Bug/12065 refac glob matching #12225

Conversation

robinf95 commented Feb 7, 2024

Description

Related Issues

Check List

github-actions bot commented Feb 7, 2024

github-actions bot commented Feb 7, 2024 • edited Loading

Compatibility status:

Incompatible components

Skipped components

Compatible components

deshsidd commented Feb 7, 2024

github-actions bot commented Feb 9, 2024

github-actions bot commented Feb 9, 2024

kkmr Feb 11, 2024

Choose a reason for hiding this comment

kkmr Feb 11, 2024

Choose a reason for hiding this comment

kkmr Feb 12, 2024

Choose a reason for hiding this comment

opensearch-trigger-bot bot commented Mar 14, 2024

cwperks Mar 25, 2024

Choose a reason for hiding this comment

opensearch-trigger-bot bot commented Apr 25, 2024

kkhatua commented Jul 30, 2024

cwperks commented Jul 30, 2024

github-actions bot commented Feb 7, 2024 •

edited

Loading