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

OAK-10768: query-spi: deprecate use of slf4j.event.Level in QueryInde… #1426

Merged
merged 7 commits into from
May 10, 2024
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;
mbaedke marked this conversation as resolved.
Show resolved Hide resolved
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)
mbaedke marked this conversation as resolved.
Show resolved Hide resolved
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) {
mbaedke marked this conversation as resolved.
Show resolved Hide resolved
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")
mbaedke marked this conversation as resolved.
Show resolved Hide resolved
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