-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/212 canonical rdf #220
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
Conversation
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.
Why remove the standard format checking here ?
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.
Maybe incorporate all your work into validateIRI into / replacing isStandardIRI ?
| throw new ParsingErrorException("Undeclared prefix: " + prefix); | ||
| } | ||
|
|
||
| return resolveIRIAgainstBase(raw); |
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.
Minor: Put back the return in inline form here
| public String extractAndUnescapeIRI(String text) { | ||
| String iri = text.substring(1, text.length() - 1); | ||
| return unescapeIRI(iri); | ||
| iri = unescapeIRI(iri); |
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.
Document the fact that validateIRI will make this function throw an exception
| * @param c the character to validate | ||
| * @return true if the character is forbidden in IRIs | ||
| */ | ||
| private boolean isInvalidIRICharacter(char c) { |
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.
Should be put in the IRIUtils class
| * @param iri the IRI string to validate (after escape sequences have been processed) | ||
| * @throws ParsingErrorException if the IRI contains forbidden characters | ||
| */ | ||
| private void validateIRI(String iri) throws ParsingErrorException { |
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.
Make a boolean returning version or make it replace isStandardURI ?
|
|
||
| assertNotNull(this.valueFactory.createIRI(correctIRI)); | ||
| assertThrows(IncorrectFormatException.class, () -> this.valueFactory.createIRI(incorrectIRI)); | ||
| // assertThrows(IncorrectFormatException.class, () -> this.valueFactory.createIRI(incorrectIRI)); |
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.
Delete, do not comment
| assertEquals("test", coreseIRI2.getLocalName()); | ||
| } | ||
|
|
||
| @Test |
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.
There should still be a test for an incorrect IRI given to the constructor
6d39da7 to
951c346
Compare
|
951c346 to
b4179c1
Compare
|
b4179c1 to
61f9211
Compare
|
61f9211 to
c6a90ca
Compare
c6a90ca to
910f540
Compare
|
remiceres
left a comment
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.
Looks good to me, thanks for the work! I left a few comments, mostly based on SonarQube findings.
| Matcher matcher = matchWithTimeout(IRI_PATTERN, iri); | ||
| if (matcher == null || !matcher.matches()) { | ||
| return ""; | ||
| return iri.endsWith("#") ? iri : (iri.contains("#") ? iri.substring(0, iri.lastIndexOf("#") + 1) : iri); |
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.
Extract this nested ternary operation into an independent statement
| * @return A canonical N-Quad string with placeholder substitution. | ||
| */ | ||
| public static String quadToNQuad(Statement quad, String blankNodeToReplace, String replacement) { | ||
| StringBuilder sb = new StringBuilder(); |
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.
Convert comments in english
| .toList(); | ||
|
|
||
| return replaced.stream() | ||
| List<Statement> sorted = replaced.stream() |
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.
Immediately return this expression instead of assigning it to the temporary variable "sorted".
| String iri = text.substring(1, text.length() - 1); | ||
| return unescapeIRI(iri); | ||
| iri = unescapeIRI(iri); | ||
| return validateIRI(iri) ? iri : iri; |
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.
This conditional operation returns the same value whether the condition is "true" or "false"
| } | ||
|
|
||
| return resolveIRIAgainstBase(raw); | ||
| String result = resolveIRIAgainstBase(raw); |
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.
Immediately return this expression instead of assigning it to the temporary variable "result".
| case '^': // U+005E - circumflex | ||
| case '`': // U+0060 - grave accent | ||
| case '|': // U+007C - pipe | ||
| case '"': // U+0022 - quotation mark |
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.
Merge the previous cases into this one using comma-separated label
https://next.sonarqube.com/sonarqube/coding_rules?open=java%3AS6208&rule_key=java%3AS6208
|
eba2902 to
174e581
Compare
|
No description provided.