Skip to content

Commit

Permalink
OAK-10768: query-spi: deprecate use of slf4j.event.Level in QueryInde… (
Browse files Browse the repository at this point in the history
#1426)

done
  • Loading branch information
mbaedke committed May 10, 2024
1 parent 86befaf commit dc6dc4e
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -555,25 +555,25 @@ private void logAdditionalMessages() {
for (SelectorImpl s : selectors) {
if (s.getExecutionPlan() != null &&
s.getExecutionPlan().getIndexPlan() != null) {
s.getExecutionPlan().getIndexPlan().getAdditionalMessages().forEach((level, list) -> {
s.getExecutionPlan().getIndexPlan().getAdditionalLogMessages().forEach((level, list) -> {
switch (level) {
case TRACE: for (String msg : list) {
case "TRACE": for (String msg : list) {
LOG.trace(msg);
}
break;
case DEBUG: for (String msg : list) {
case "DEBUG": for (String msg : list) {
LOG.debug(msg);
}
break;
case INFO: for (String msg : list) {
break;
case "INFO": for (String msg : list) {
LOG.info(msg);
}
break;
case WARN: for (String msg : list) {
break;
case "WARN": for (String msg : list) {
LOG.warn(msg);
}
break;
case ERROR: for (String msg : list) {
case "ERROR": for (String msg : list) {
LOG.error(msg);
}
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,11 @@
import org.apache.jackrabbit.guava.common.collect.Maps;
import org.apache.jackrabbit.oak.api.Type;
import org.apache.jackrabbit.oak.spi.state.NodeState;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.event.Level;

import static java.util.stream.Collectors.toMap;
import static org.apache.jackrabbit.oak.spi.query.Filter.PropertyRestriction;

/**
Expand Down Expand Up @@ -221,6 +224,8 @@ List<IndexPlan> getPlans(Filter filter, List<OrderEntry> sortOrder,
@ProviderType
interface IndexPlan extends Cloneable{

Logger LOG = LoggerFactory.getLogger(QueryIndex.IndexPlan.class);

/**
* The cost to execute the query once. The returned value should
* approximately match the number of disk read operations plus the
Expand Down Expand Up @@ -361,17 +366,32 @@ default boolean logWarningForPathFilterMismatch() {
* This method can be used for communicating any messages which should be logged if this plan is selected for execution.
* The messages are returned as a map whose key indicates log level and value is a list of messages against that
* log level.
* @deprecated use {@link #getAdditionalLogMessages()} instead
* @return map containing log messages.
*/
@Deprecated(forRemoval = true)
default Map<Level, List<String>> getAdditionalMessages() {
return Collections.emptyMap();
LOG.warn("use of deprecated API - this method is going to be removed in future Oak releases - see OAK-10768 for details");
return getAdditionalLogMessages().entrySet().stream().collect(toMap(entry -> Level.valueOf(entry.getKey()), Map.Entry::getValue));
}

/**
* This method can be used for communicating any messages which should be logged if this plan is selected for execution.
* The messages are returned as a map whose key indicates log level and value is a list of messages against that
* log level.
* @return map containing log messages.
*/
default Map<String, List<String>> getAdditionalLogMessages() { return Collections.emptyMap(); }

/**
* A builder for index plans.
*/
class Builder {

private static Logger LOG = LoggerFactory.getLogger(QueryIndex.IndexPlan.Builder.class);

protected double costPerExecution = 1.0;
protected double costPerEntry = 1.0;
protected long estimatedEntryCount = 1000000;
Expand All @@ -388,7 +408,7 @@ class Builder {
protected String planName;
protected boolean deprecated;
protected boolean logWarningForPathFilterMismatch;
protected final Map<Level, List<String>> additionalMessages = new HashMap<>();
protected final Map<String, List<String>> additionalMessages = new HashMap<>();

public Builder setCostPerExecution(double costPerExecution) {
this.costPerExecution = costPerExecution;
Expand All @@ -415,7 +435,33 @@ public Builder setDelayed(boolean isDelayed) {
return this;
}

/**
* @deprecated use {@link #addAdditionalMessage(String level, String s)} instead
* */
@Deprecated(forRemoval = true)
public Builder addAdditionalMessage(Level level, String s) {
LOG.warn("use of deprecated API - this method is going to be removed in future Oak releases - see OAK-10768 for details");
this.additionalMessages.compute(level.name(), (k,v) -> {
if (v == null) {
v = new ArrayList<>();
}
v.add(s);
return v;
});
return this;
}

public Builder addAdditionalMessage(String level, String s) {
switch (level) {
case "TRACE":
case "DEBUG":
case "INFO":
case "WARN":
case "ERROR":
break;
default:
throw new IllegalArgumentException("unsupported log level: " + level);
}
this.additionalMessages.compute(level, (k,v) -> {
if (v == null) {
v = new ArrayList<>();
Expand Down Expand Up @@ -517,7 +563,7 @@ public IndexPlan build() {
private final boolean deprecated =
Builder.this.deprecated;
private final boolean logWarningForPathFilterMismatch = Builder.this.logWarningForPathFilterMismatch;
private final Map<Level, List<String>> additionalMessages = Builder.this.additionalMessages;
private final Map<String, List<String>> additionalMessages = Builder.this.additionalMessages;

private String getAdditionalMessageString() {
return additionalMessages.entrySet().stream()
Expand Down Expand Up @@ -661,10 +707,9 @@ public boolean logWarningForPathFilterMismatch() {
}

@Override
public Map<Level, List<String>> getAdditionalMessages() {
public Map<String, List<String>> getAdditionalLogMessages() {
return additionalMessages;
}

};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
/**
* This package contains oak query index related classes.
*/
@Version("1.8.0")
@Version("2.0.0")
package org.apache.jackrabbit.oak.spi.query;

import org.osgi.annotation.versioning.Version;
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@
import org.jetbrains.annotations.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.event.Level;

import static org.apache.jackrabbit.guava.common.collect.Lists.newArrayList;
import static org.apache.jackrabbit.guava.common.collect.Lists.newArrayListWithCapacity;
Expand Down Expand Up @@ -307,13 +306,13 @@ private IndexPlan.Builder getPlanBuilder() {

if (queryFilterPattern != null) {
if (ft != null && !queryFilterPattern.matcher(ft.toString()).find()) {
plan.addAdditionalMessage(Level.WARN, "Potentially improper use of index " + definition.getIndexPath() + " with queryFilterRegex "
plan.addAdditionalMessage("WARN", "Potentially improper use of index " + definition.getIndexPath() + " with queryFilterRegex "
+ queryFilterPattern + " to search for value '" + ft + "'");
}
for (PropertyRestriction pr : filter.getPropertyRestrictions()) {
// Ignore properties beginning with ";" like :indexTag / :indexName etx
if (!pr.propertyName.startsWith(":") && !queryFilterPattern.matcher(pr.toString()).find()) {
plan.addAdditionalMessage(Level.WARN, "Potentially improper use of index " + definition.getIndexPath() + " with queryFilterRegex "
plan.addAdditionalMessage("WARN", "Potentially improper use of index " + definition.getIndexPath() + " with queryFilterRegex "
+ queryFilterPattern + " to search for value '" + pr + "'");
}
}
Expand Down

0 comments on commit dc6dc4e

Please sign in to comment.