[Extensions] Add Regex validator by mloufra · Pull Request #6221 · opensearch-project/OpenSearch

mloufra · 2023-02-07T16:49:27Z

Description

Add Regex Validator implement Writeable and Validator to make Validator writeable for custom setting.

Issues Resolved

Fixes opensearch-project/opensearch-sdk-java#350

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff
Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

github-actions · 2023-02-07T17:11:25Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/10785/
CommitID: 46b3151
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?

dbwiddis

Great job!

A few minor tweaks needed, and a usability suggestion from the original issue.

dbwiddis · 2023-02-07T17:49:27Z

server/src/main/java/org/opensearch/common/settings/Setting.java

+    // RegexValidator
+    public static class RegexValidator implements Writeable, Validator<String> {
+        private Pattern pattern;
+        private String value = "forbidden";


Don't hardcode this iniitialization. I don't think you use it anywhere, so it can just be deleted.

dbwiddis · 2023-02-07T17:49:53Z

server/src/main/java/org/opensearch/common/settings/Setting.java

        return properties != null && Arrays.asList(properties).contains(Property.Filtered);
    }

+    // RegexValidator


A more descriptive class javadoc would be helpful! :)

dbwiddis · 2023-02-07T17:51:01Z

server/src/main/java/org/opensearch/common/settings/Setting.java

+
+        @Override
+        public void writeTo(StreamOutput out) throws IOException {
+            out.writeString(pattern.pattern());   


This is good. You don't need to store the original string because you can extract it with pattern().

dbwiddis · 2023-02-07T17:53:22Z

server/src/main/java/org/opensearch/common/settings/Setting.java

+        @Override
+        public void validate(String value) {
+            if(pattern.matcher(value).matches()){
+                throw new IllegalArgumentException("custom setting contains forbidden");


The message here should be "Custom setting matches regex [" + pattern.pattern() + "]".

See this comment for a suggestion to add an (optional) second String in an overloaded constructor, which if not null, will use that message here instead of this one.

dbwiddis · 2023-02-07T17:55:05Z

server/src/main/java/org/opensearch/common/settings/WriteableSetting.java

        EnumSet<Property> propSet = in.readEnumSet(Property.class);
        // Put it all in a setting object
-        this.setting = createSetting(type, key, defaultValue, fallback, propSet.toArray(Property[]::new));
+        this.setting = createSetting(type, key, defaultValue, validator, fallback, propSet.toArray(Property[]::new));


OK for now, when your parser PR is merged you'll have to rebase to update this.

github-actions · 2023-02-08T16:54:46Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/10847/
CommitID: 99e74a8
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 · 2023-02-08T17:47:39Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/10850/
CommitID: f1db1ff
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 · 2023-02-08T19:05:25Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/10859/
CommitID: 0c7ce04
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 · 2023-02-08T20:28:59Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/10865/
CommitID: 3066123
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 · 2023-02-08T22:50:21Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/10882/
CommitID: f5058ca

Signed-off-by: Frank Lou <mloufra@amazon.com>

github-actions · 2023-03-08T23:32:37Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/12136/
CommitID: c3daa44

codecov-commenter · 2023-03-08T23:36:55Z

Codecov Report

Merging #6221 (8e67ed4) into main (8c0e5d0) will increase coverage by 0.02%.
The diff coverage is 75.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff              @@
##               main    #6221      +/-   ##
============================================
+ Coverage     70.77%   70.80%   +0.02%     
- Complexity    59125    59144      +19     
============================================
  Files          4804     4804              
  Lines        283098   283121      +23     
  Branches      40813    40816       +3     
============================================
+ Hits         200372   200470      +98     
+ Misses        66269    66183      -86     
- Partials      16457    16468      +11

Impacted Files	Coverage Δ
...g/opensearch/common/settings/WriteableSetting.java	`69.58% <50.00%> (-2.00%)`	⬇️
...n/java/org/opensearch/common/settings/Setting.java	`86.32% <100.00%> (+0.19%)`	⬆️

... and 457 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: Frank Lou <mloufra@amazon.com>

github-actions · 2023-03-09T00:10:20Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/12141/
CommitID: 28cda4f

dbwiddis

Some final javadoc fixes and this will be GTG

dbwiddis · 2023-03-09T17:09:14Z

server/src/main/java/org/opensearch/common/settings/Setting.java

+    public static class RegexValidator implements Writeable, Validator<String> {
+        private Pattern pattern;
+
+        public RegexValidator(String regex) {


You added the javadoc to the streaminput constructor below, not this one... this is the one developers will use. The StreamInput constructor will be used by the transport service sending these between OpenSearch and SDK.

Also you need the @param javadoc tag for the parameters.

Signed-off-by: Frank Lou <mloufra@amazon.com>

github-actions · 2023-03-09T18:18:57Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/12180/
CommitID: 9efb96b

Changes have been addressed

Signed-off-by: Frank Lou <mloufra@amazon.com>

github-actions · 2023-03-10T19:57:57Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/12243/
CommitID: 8e67ed4

opensearch-trigger-bot · 2023-03-10T20:55:27Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-6221-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 b0ab550ecfb80a2942b58169c5c38e500950a8cc
# Push it to GitHub
git push --set-upstream origin backport/backport-6221-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-6221-to-2.x.

* add RegexValidator class Signed-off-by: Frank Lou <mloufra@amazon.com> * add regexValidator Signed-off-by: Frank Lou <mloufra@amazon.com> * addressing comment Signed-off-by: Frank Lou <mloufra@amazon.com> * addressing comment Signed-off-by: Frank Lou <mloufra@amazon.com> * do spotless Signed-off-by: Frank Lou <mloufra@amazon.com> * address comment Signed-off-by: Frank Lou <mloufra@amazon.com> * fix merge conflict Signed-off-by: Frank Lou <mloufra@amazon.com> * address comment Signed-off-by: Frank Lou <mloufra@amazon.com> * address comment Signed-off-by: Frank Lou <mloufra@amazon.com> * address comment Signed-off-by: Frank Lou <mloufra@amazon.com> * address comment Signed-off-by: Frank Lou <mloufra@amazon.com> * address comment Signed-off-by: Frank Lou <mloufra@amazon.com> * address comment Signed-off-by: Frank Lou <mloufra@amazon.com> * address comment Signed-off-by: Frank Lou <mloufra@amazon.com> * address comment Signed-off-by: Frank Lou <mloufra@amazon.com> * fix AssertionError Signed-off-by: Frank Lou <mloufra@amazon.com> * fix AssertionError Signed-off-by: Frank Lou <mloufra@amazon.com> * address comment Signed-off-by: Frank Lou <mloufra@amazon.com> * add javadoc Signed-off-by: Frank Lou <mloufra@amazon.com> * address comment Signed-off-by: Frank Lou <mloufra@amazon.com> * remove changlog Signed-off-by: Frank Lou <mloufra@amazon.com> --------- Signed-off-by: Frank Lou <mloufra@amazon.com> (cherry picked from commit b0ab550) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* add RegexValidator class * add regexValidator * addressing comment * addressing comment * do spotless * address comment * fix merge conflict * address comment * address comment * address comment * address comment * address comment * address comment * address comment * address comment * fix AssertionError * fix AssertionError * address comment * add javadoc * address comment * remove changlog --------- (cherry picked from commit b0ab550) Signed-off-by: Frank Lou <mloufra@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* add RegexValidator class Signed-off-by: Frank Lou <mloufra@amazon.com> * add regexValidator Signed-off-by: Frank Lou <mloufra@amazon.com> * addressing comment Signed-off-by: Frank Lou <mloufra@amazon.com> * addressing comment Signed-off-by: Frank Lou <mloufra@amazon.com> * do spotless Signed-off-by: Frank Lou <mloufra@amazon.com> * address comment Signed-off-by: Frank Lou <mloufra@amazon.com> * fix merge conflict Signed-off-by: Frank Lou <mloufra@amazon.com> * address comment Signed-off-by: Frank Lou <mloufra@amazon.com> * address comment Signed-off-by: Frank Lou <mloufra@amazon.com> * address comment Signed-off-by: Frank Lou <mloufra@amazon.com> * address comment Signed-off-by: Frank Lou <mloufra@amazon.com> * address comment Signed-off-by: Frank Lou <mloufra@amazon.com> * address comment Signed-off-by: Frank Lou <mloufra@amazon.com> * address comment Signed-off-by: Frank Lou <mloufra@amazon.com> * address comment Signed-off-by: Frank Lou <mloufra@amazon.com> * fix AssertionError Signed-off-by: Frank Lou <mloufra@amazon.com> * fix AssertionError Signed-off-by: Frank Lou <mloufra@amazon.com> * address comment Signed-off-by: Frank Lou <mloufra@amazon.com> * add javadoc Signed-off-by: Frank Lou <mloufra@amazon.com> * address comment Signed-off-by: Frank Lou <mloufra@amazon.com> * remove changlog Signed-off-by: Frank Lou <mloufra@amazon.com> --------- Signed-off-by: Frank Lou <mloufra@amazon.com> Signed-off-by: Mingshi Liu <mingshl@amazon.com>

dbwiddis requested changes Feb 7, 2023

View reviewed changes

mloufra force-pushed the sample-validator branch from 0c7ce04 to 3066123 Compare February 8, 2023 20:00

mloufra force-pushed the sample-validator branch from 3066123 to f5058ca Compare February 8, 2023 22:18

mloufra requested a review from dbwiddis February 8, 2023 22:25

mloufra force-pushed the sample-validator branch from f5058ca to 0eddf2e Compare February 8, 2023 22:52

mloufra marked this pull request as ready for review February 8, 2023 22:52

mloufra requested review from Bukhtawar, CEHENKLE, Rishikesh1159, adnapibar, anasalkouz, andrross, dblock, gbbafna, kartg, kotwanikunal, mch2, nknize, owaiskazi19, reta, ryanbogan, saratvemulapalli, setiah and shwetathareja as code owners February 8, 2023 22:52

mloufra added 3 commits March 8, 2023 22:34

fix AssertionError

a285a94

Signed-off-by: Frank Lou <mloufra@amazon.com>

fix AssertionError

3474922

Signed-off-by: Frank Lou <mloufra@amazon.com>

address comment

c3daa44

Signed-off-by: Frank Lou <mloufra@amazon.com>

mloufra force-pushed the sample-validator branch from 4288cc9 to c3daa44 Compare March 8, 2023 23:03

add javadoc

28cda4f

Signed-off-by: Frank Lou <mloufra@amazon.com>

mloufra requested a review from dbwiddis March 9, 2023 00:10

dbwiddis requested changes Mar 9, 2023

View reviewed changes

address comment

9efb96b

Signed-off-by: Frank Lou <mloufra@amazon.com>

mloufra requested a review from dbwiddis March 9, 2023 17:49

dbwiddis approved these changes Mar 9, 2023

View reviewed changes

mloufra requested a review from dblock March 9, 2023 19:41

remove changlog

8e67ed4

Signed-off-by: Frank Lou <mloufra@amazon.com>

ryanbogan added skip-changelog backport 2.x Backport to 2.x branch labels Mar 10, 2023

ryanbogan merged commit b0ab550 into opensearch-project:main Mar 10, 2023

mloufra deleted the sample-validator branch March 10, 2023 20:54

ryanbogan added backport 2.x Backport to 2.x branch and removed backport 2.x Backport to 2.x branch labels Mar 10, 2023

opensearch-trigger-bot bot mentioned this pull request Mar 10, 2023

[Backport 2.x] [Extensions] Add Regex validator #6620

Merged

ryanbogan mentioned this pull request Mar 10, 2023

[AUTOCUT] Gradle Check Failure on push to main #6621

Closed

Conversation

mloufra commented Feb 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues Resolved

Check List

Uh oh!

github-actions bot commented Feb 7, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

dbwiddis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 8, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Feb 8, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Feb 8, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Feb 8, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Feb 8, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Mar 8, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

codecov-commenter commented Mar 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Mar 9, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

dbwiddis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 9, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Mar 10, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

opensearch-trigger-bot bot commented Mar 10, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mloufra commented Feb 7, 2023 •

edited

Loading

codecov-commenter commented Mar 8, 2023 •

edited

Loading