rishabhdaim commented on code in PR #708: URL: https://github.com/apache/jackrabbit-oak/pull/708#discussion_r977515877
########## oak-run/src/main/java/org/apache/jackrabbit/oak/plugins/document/check/AsyncDocumentProcessor.java: ########## @@ -0,0 +1,50 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.jackrabbit.oak.plugins.document.check; + +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; + +import org.apache.jackrabbit.oak.plugins.document.NodeDocument; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** + * Base class for {@link DocumentProcessor} implementations that create tasks + * executed by an executor service. + */ +public abstract class AsyncDocumentProcessor implements DocumentProcessor { + + private final ExecutorService executorService; + + protected AsyncDocumentProcessor(ExecutorService executorService) { + this.executorService = executorService; + } + + @Override + public final void processDocument(@NotNull NodeDocument document, + @NotNull BlockingQueue<Result> results) { + Callable<Void> task = createTask(document, results); + if (task != null) { + executorService.submit(task); + } + } + + protected abstract @Nullable Callable<Void> createTask(@NotNull NodeDocument document, Review Comment: IMHO it's better to return `Optional` here and saves its client an additional null check. Since we are returning a `Callable` here, its callers are mostly likely to submit this task to an executor. Optional would make that NPE free and more readable. `createTask(_, _).ifPresent(executorService::submit)`. ########## oak-run/src/main/java/org/apache/jackrabbit/oak/plugins/document/check/AsyncNodeStateProcessor.java: ########## @@ -0,0 +1,140 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.jackrabbit.oak.plugins.document.check; + +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.atomic.AtomicReference; + +import org.apache.jackrabbit.oak.api.PropertyState; +import org.apache.jackrabbit.oak.api.Type; +import org.apache.jackrabbit.oak.plugins.document.DocumentNodeStore; +import org.apache.jackrabbit.oak.plugins.document.NodeDocument; +import org.apache.jackrabbit.oak.plugins.document.Path; +import org.apache.jackrabbit.oak.plugins.document.RevisionVector; +import org.apache.jackrabbit.oak.spi.state.NodeState; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import static org.apache.jackrabbit.oak.spi.state.NodeStateUtils.getNode; + +/** + * A {@link DocumentProcessor} that processes {@link NodeState}s. + */ +public abstract class AsyncNodeStateProcessor extends AsyncDocumentProcessor { + + + protected final DocumentNodeStore ns; + + protected final RevisionVector headRevision; + + protected final NodeState uuidIndex; + + protected AsyncNodeStateProcessor(DocumentNodeStore ns, + RevisionVector headRevision, + ExecutorService executorService) { + super(executorService); + this.ns = ns; + this.headRevision = headRevision; + this.uuidIndex = getNode(ns.getRoot(), "/oak:index/uuid/:index"); + } + + /** + * Decide early whether a {@link NodeDocument} should be processed or not. + * This implementation returns {@code true} if the document not a split + * document, otherwise {@code false}. This method can be overridden by + * subclasses. + * + * @param doc the document to process. + * @return whether the document should be processed or ignored. + */ + protected boolean process(NodeDocument doc) { + return !doc.isSplitDocument(); + } + + /** + * Utility method that resolves he {@code uuid} into a {@link NodeState} + * with the help of the UUID index. + * + * @param uuid the UUID to resolve. + * @param resolvedPath will be set to the resolved path if available. + * @return the {@link NodeState} with the given UUID or {@code null} if it + * cannot be resolved or doesn't exist. + */ + protected final @Nullable NodeState getNodeByUUID(@NotNull String uuid, + @NotNull AtomicReference<String> resolvedPath) { + PropertyState entry = uuidIndex.getChildNode(uuid).getProperty("entry"); + NodeState state = null; + if (entry != null && entry.isArray() && entry.count() > 0) { + String path = entry.getValue(Type.STRING, 0); + resolvedPath.set(path); + state = getNode(ns.getRoot(), path); + if (!state.exists()) { + state = null; + } + } + return state; + } + + @Override + protected final @Nullable Callable<Void> createTask(@NotNull NodeDocument document, + @NotNull BlockingQueue<Result> results) { + if (process(document)) { + return new NodeStateTask(document, results); + } else { + return null; + } + } + + /** + * Responsibility of the subclass to implement the processor logic. This + * method will run as a task with an executor service. + * + * @param path the path of the {@code NodeState} to process. + * @param state the {@code NodeState} or {@code null} if the node does not + * exist at this path. This may happen for nodes that have been + * deleted but not yet garbage collected. + * @return the result of the task or {@code null} if nothing is reported. + */ + protected abstract @Nullable Result runTask(@NotNull Path path, + @Nullable NodeState state); + + protected class NodeStateTask implements Callable<Void> { + + private final NodeDocument document; + + private final BlockingQueue<Result> results; + + public NodeStateTask(@NotNull NodeDocument document, + @NotNull BlockingQueue<Result> results) { + this.document = document; + this.results = results; + } + + @Override + public Void call() throws Exception { + Path path = document.getPath(); + NodeState state = document.getNodeAtRevision(ns, headRevision, null); + Result r = runTask(path, state); Review Comment: ```suggestion runTask(path, state).ifPresent(results::put); return null; ``` ########## oak-run/src/main/java/org/apache/jackrabbit/oak/plugins/document/check/ReferenceCheck.java: ########## @@ -0,0 +1,95 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.jackrabbit.oak.plugins.document.check; + +import java.util.concurrent.ExecutorService; +import java.util.concurrent.atomic.AtomicReference; + +import org.apache.jackrabbit.oak.api.PropertyState; +import org.apache.jackrabbit.oak.api.Type; +import org.apache.jackrabbit.oak.commons.json.JsopBuilder; +import org.apache.jackrabbit.oak.plugins.document.DocumentNodeStore; +import org.apache.jackrabbit.oak.plugins.document.Path; +import org.apache.jackrabbit.oak.plugins.document.RevisionVector; +import org.apache.jackrabbit.oak.spi.state.NodeState; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** + * Checks if {@code jcr:baseVersion} reference properties resolve to a node. + */ +public class ReferenceCheck extends AsyncNodeStateProcessor { + + private final String propertyName; + + ReferenceCheck(String propertyName, + DocumentNodeStore ns, + RevisionVector headRevision, + ExecutorService executorService) { + super(ns, headRevision, executorService); + this.propertyName = propertyName; + } + + @Override + protected @Nullable Result runTask(@NotNull Path path, Review Comment: I would make `runTask` return an Instance of `Optional`, save a potential NPE and make the code more fluent. -- 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-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
