You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by GitBox <gi...@apache.org> on 2022/05/31 12:50:00 UTC
[GitHub] [sling-org-apache-sling-jcr-resource] anchela opened a new pull request, #25: SLING-11361 : Sonor findings (excluding complexity and nullable/notnull annotations)
anchela opened a new pull request, #25:
URL: https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/25
@cziegeler , as discussed i am spitting the original PR into smaller change sets. this one addressing a bunch of sonar findings that don't require heavy refactoring. i would appreciate if you had time to review it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-resource] joerghoh commented on a diff in pull request #25: SLING-11361 : Sonor findings (excluding complexity and nullable/notnull annotations)
Posted by GitBox <gi...@apache.org>.
joerghoh commented on code in PR #25:
URL: https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/25#discussion_r885704591
##########
src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/BasicQueryLanguageProvider.java:
##########
@@ -98,85 +99,91 @@ public Iterator<ValueMap> queryResources(final ResolveContext<JcrProviderState>
try {
final QueryResult result = JcrResourceUtil.query(ctx.getProviderState().getSession(), query, queryLanguage);
- final String[] colNames = result.getColumnNames();
- final RowIterator rows = result.getRows();
+ return new ValueMapIterator(result.getColumnNames(), result.getRows());
+ } catch (final javax.jcr.query.InvalidQueryException iqe) {
+ throw new QuerySyntaxException(iqe.getMessage(), query, language, iqe);
+ } catch (final RepositoryException re) {
+ throw new SlingException(re.getMessage(), re);
+ }
+ }
- return new Iterator<ValueMap>() {
Review Comment:
And here the diff gets really confusing ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-resource] sonarcloud[bot] commented on pull request #25: SLING-11361 : Sonor findings (excluding complexity and nullable/notnull annotations)
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #25:
URL: https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/25#issuecomment-1142099571
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=25)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=25&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=25&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=25&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=25&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=25&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=25&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=25&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=25&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=25&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=25&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=25&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=25&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=25&metric=new_coverage&view=list) [48.6% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=25&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=25&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=25&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-resource] anchela commented on a diff in pull request #25: SLING-11361 : Sonor findings (excluding complexity and nullable/notnull annotations)
Posted by GitBox <gi...@apache.org>.
anchela commented on code in PR #25:
URL: https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/25#discussion_r885723119
##########
src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/BasicQueryLanguageProvider.java:
##########
@@ -98,85 +99,91 @@ public Iterator<ValueMap> queryResources(final ResolveContext<JcrProviderState>
try {
final QueryResult result = JcrResourceUtil.query(ctx.getProviderState().getSession(), query, queryLanguage);
- final String[] colNames = result.getColumnNames();
- final RowIterator rows = result.getRows();
+ return new ValueMapIterator(result.getColumnNames(), result.getRows());
+ } catch (final javax.jcr.query.InvalidQueryException iqe) {
+ throw new QuerySyntaxException(iqe.getMessage(), query, language, iqe);
+ } catch (final RepositoryException re) {
+ throw new SlingException(re.getMessage(), re);
+ }
+ }
- return new Iterator<ValueMap>() {
Review Comment:
maybe that helps :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-resource] anchela commented on a diff in pull request #25: SLING-11361 : Sonor findings (excluding complexity and nullable/notnull annotations)
Posted by GitBox <gi...@apache.org>.
anchela commented on code in PR #25:
URL: https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/25#discussion_r885722435
##########
src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/BasicQueryLanguageProvider.java:
##########
@@ -98,85 +99,91 @@ public Iterator<ValueMap> queryResources(final ResolveContext<JcrProviderState>
try {
final QueryResult result = JcrResourceUtil.query(ctx.getProviderState().getSession(), query, queryLanguage);
- final String[] colNames = result.getColumnNames();
- final RowIterator rows = result.getRows();
+ return new ValueMapIterator(result.getColumnNames(), result.getRows());
+ } catch (final javax.jcr.query.InvalidQueryException iqe) {
+ throw new QuerySyntaxException(iqe.getMessage(), query, language, iqe);
+ } catch (final RepositoryException re) {
+ throw new SlingException(re.getMessage(), re);
+ }
+ }
- return new Iterator<ValueMap>() {
Review Comment:
yeah... because the original code is imho extra hard to read:
original:
```
@Override
public Iterator<ValueMap> queryResources(final @NotNull ResolveContext<JcrProviderState> ctx,
final String query,
final String language) {
final String queryLanguage = ArrayUtils.contains(getSupportedLanguages(ctx), language) ? language : DEFAULT_QUERY_LANGUAGE;
try {
final QueryResult result = JcrResourceUtil.query(ctx.getProviderState().getSession(), query, queryLanguage);
final String[] colNames = result.getColumnNames();
final RowIterator rows = result.getRows();
return new Iterator<ValueMap>() {
private ValueMap next;
{
next = seek();
}
@Override
public boolean hasNext() {
return next != null;
}
@Override
public ValueMap next() {
if (next == null) {
throw new NoSuchElementException();
}
final ValueMap result = next;
next = seek();
return result;
}
private ValueMap seek() {
ValueMap result = null;
while (result == null && rows.hasNext()) {
try {
final Row jcrRow = rows.nextRow();
final String resourcePath = jcrRow.getPath();
if (resourcePath != null && providerContext.getExcludedPaths().matches(resourcePath) == null) {
final Map<String, Object> row = new HashMap<>();
boolean didPath = false;
boolean didScore = false;
final Value[] values = jcrRow.getValues();
for (int i = 0; i < values.length; i++) {
Value v = values[i];
if (v != null) {
String colName = colNames[i];
row.put(colName,
JcrResourceUtil.toJavaObject(values[i]));
if (colName.equals(QUERY_COLUMN_PATH)) {
didPath = true;
row.put(colName, JcrResourceUtil.toJavaObject(values[i]).toString());
}
if (colName.equals(QUERY_COLUMN_SCORE)) {
didScore = true;
}
}
}
if (!didPath) {
row.put(QUERY_COLUMN_PATH, jcrRow.getPath());
}
if (!didScore) {
row.put(QUERY_COLUMN_SCORE, jcrRow.getScore());
}
result = new ValueMapDecorator(row);
}
} catch (final RepositoryException re) {
logger.error("queryResources$next: Problem accessing row values", re);
}
}
return result;
}
@Override
public void remove() {
throw new UnsupportedOperationException("remove");
}
};
} catch (final javax.jcr.query.InvalidQueryException iqe) {
throw new QuerySyntaxException(iqe.getMessage(), query, language, iqe);
} catch (final RepositoryException re) {
throw new SlingException(re.getMessage(), re);
}
}
```
new code:
```
@Override
public Iterator<ValueMap> queryResources(final ResolveContext<JcrProviderState> ctx,
final String query,
final String language) {
final String queryLanguage = ArrayUtils.contains(getSupportedLanguages(ctx), language) ? language : DEFAULT_QUERY_LANGUAGE;
try {
final QueryResult result = JcrResourceUtil.query(ctx.getProviderState().getSession(), query, queryLanguage);
return new ValueMapIterator(result.getColumnNames(), result.getRows());
} catch (final javax.jcr.query.InvalidQueryException iqe) {
throw new QuerySyntaxException(iqe.getMessage(), query, language, iqe);
} catch (final RepositoryException re) {
throw new SlingException(re.getMessage(), re);
}
}
private class ValueMapIterator implements Iterator<ValueMap> {
private final String[] colNames;
private final RowIterator rows;
private ValueMap next;
private ValueMapIterator(@NotNull String[] colNames, @NotNull RowIterator rows) {
this.colNames = colNames;
this.rows = rows;
next = seek();
}
@Override
public boolean hasNext() {
return next != null;
}
@Override
public ValueMap next() {
if ( next == null ) {
throw new NoSuchElementException();
}
final ValueMap result = next;
next = seek();
return result;
}
private ValueMap seek() {
ValueMap result = null;
while (result == null && rows.hasNext()) {
try {
final Row jcrRow = rows.nextRow();
final String resourcePath = jcrRow.getPath();
if (resourcePath != null && providerContext.getExcludedPaths().matches(resourcePath) == null) {
result = new ValueMapDecorator(getRow(jcrRow));
}
} catch (final RepositoryException re) {
logger.error("queryResources$next: Problem accessing row values", re);
}
}
return result;
}
private Map<String, Object> getRow(@NotNull Row jcrRow) throws RepositoryException {
final Map<String, Object> row = new HashMap<>();
boolean didPath = false;
boolean didScore = false;
final Value[] values = jcrRow.getValues();
for (int i = 0; i < values.length; i++) {
Value v = values[i];
if (v != null) {
String colName = colNames[i];
row.put(colName, JcrResourceUtil.toJavaObject(values[i]));
if (colName.equals(QUERY_COLUMN_PATH)) {
didPath = true;
row.put(colName, JcrResourceUtil.toJavaObject(values[i]).toString());
}
if (colName.equals(QUERY_COLUMN_SCORE)) {
didScore = true;
}
}
}
if (!didPath) {
row.put(QUERY_COLUMN_PATH, jcrRow.getPath());
}
if (!didScore) {
row.put(QUERY_COLUMN_SCORE, jcrRow.getScore());
}
return row;
}
@Override
public void remove() {
throw new UnsupportedOperationException("remove");
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-resource] anchela merged pull request #25: SLING-11361 : Sonor findings (excluding complexity and nullable/notnull annotations)
Posted by GitBox <gi...@apache.org>.
anchela merged PR #25:
URL: https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/25
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-resource] joerghoh commented on a diff in pull request #25: SLING-11361 : Sonor findings (excluding complexity and nullable/notnull annotations)
Posted by GitBox <gi...@apache.org>.
joerghoh commented on code in PR #25:
URL: https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/25#discussion_r885725327
##########
src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/BasicQueryLanguageProvider.java:
##########
@@ -98,85 +99,91 @@ public Iterator<ValueMap> queryResources(final ResolveContext<JcrProviderState>
try {
final QueryResult result = JcrResourceUtil.query(ctx.getProviderState().getSession(), query, queryLanguage);
- final String[] colNames = result.getColumnNames();
- final RowIterator rows = result.getRows();
+ return new ValueMapIterator(result.getColumnNames(), result.getRows());
+ } catch (final javax.jcr.query.InvalidQueryException iqe) {
+ throw new QuerySyntaxException(iqe.getMessage(), query, language, iqe);
+ } catch (final RepositoryException re) {
+ throw new SlingException(re.getMessage(), re);
+ }
+ }
- return new Iterator<ValueMap>() {
Review Comment:
yes, definitely. It had a lengthy inner class ... *brrrr*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org