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

Use precompiled regular expressions to validate the segments of a UID #4064

Merged
merged 1 commit into from Jan 24, 2024

Conversation

joerg1985
Copy link
Contributor

Description

Avoid to compile the SEGMENT_PATTERN for each validated segment of a UID.

Signed-off-by: Jörg Sautter joerg.sautter@gmx.net

@joerg1985 joerg1985 requested a review from a team as a code owner January 24, 2024 16:40
@J-N-K
Copy link
Member

J-N-K commented Jan 24, 2024

Thanks! This looks like a nice improvement. It seems the pattern is also used at other places. I would suggest to make SEGMENT_PATTERN private instead of public and add a new method

public static boolean isValid(@Nullable String str) {
    return str != null && SEGMENT_PATTERN.matcher(str).matches();
}

You can then change the line in MetadataSelectorMatcher to

return result.stream().filter(AbstractUID::isValid).collect(Collectors.toSet());

@joerg1985
Copy link
Contributor Author

@J-N-K the PR has been updated, as suggested. Thank you for the fast feedback / review!

@J-N-K
Copy link
Member

J-N-K commented Jan 24, 2024

Can you fix spotless:

Error:  Failed to execute goal com.diffplug.spotless:spotless-maven-plugin:2.38.0:check (codestyle_check) on project org.openhab.core.io.rest.core: The following files had format violations:
Error:      src/main/java/org/openhab/core/io/rest/core/internal/item/MetadataSelectorMatcher.java
Error:          @@ -78,8 +78,7 @@
Error:           ············result.addAll(metadataNamespaces);
Error:           
Error:           ············//·filter·all·name·spaces·which·do·not·match·the·UID·segment·pattern·(this·will·be·the·regex·tokens):
Error:          -············return·result.stream().filter(AbstractUID::isValid)
Error:          -····················.collect(Collectors.toSet());
Error:          +············return·result.stream().filter(AbstractUID::isValid).collect(Collectors.toSet());
Error:           ········}
Error:           ····}
Error:           }
Error:  Run 'mvn spotless:apply' to fix these violations.
Error:  -> [Help 1]
Error:  
Error:  To see the full stack trace of the errors, re-run Maven with the -e switch.
Error:  Re-run Maven using the -X switch to enable full debug logging.
Error:  
Error:  For more information about the errors and possible solutions, please read the following articles:
Error:  [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
Error:  
Error:  After correcting the problems, you can resume the build with the command
Error:    mvn <args> -rf :org.openhab.core.io.rest.core
Error: Process completed with exit code 1.

Signed-off-by: Jörg Sautter <joerg.sautter@gmx.net>
@joerg1985
Copy link
Contributor Author

@J-N-K Sorry for this, my local spotless is somehow broken.

mvn install will end in this error for me:

[ERROR] /pom.xml:[0]
Missing pom.xml file.
[WARNING] OH-INF.config.addons.xml:[0]
Unused configuration reference with uri - system:addons
[ERROR] null/pom.xml:[0]
Missing /project/version in the pom.xml file.
[ERROR] null/pom.xml:[0]
Missing /project/artifactId in the pom.xml file.

Copy link
Member

@J-N-K J-N-K left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@J-N-K J-N-K merged commit 7ea603c into openhab:main Jan 24, 2024
3 checks passed
@J-N-K J-N-K added the enhancement An enhancement or new feature of the Core label Jan 24, 2024
@J-N-K J-N-K added this to the 4.2 milestone Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature of the Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants