thomasmueller commented on code in PR #2181: URL: https://github.com/apache/jackrabbit-oak/pull/2181#discussion_r1995265861
########## oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/pio/Closer.java: ########## @@ -0,0 +1,130 @@ +/* + * 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.commons.pio; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.io.Closeable; +import java.io.IOException; +import java.util.ArrayDeque; +import java.util.Deque; +import java.util.Objects; + +/** + * Convenience utility to close a list of {@link Closeable}s. + * <p> + * Inspired by and replacing Guava's Closer. + */ +public class Closer implements Closeable { + + private Closer() { + // no instances for you + } + + // stack of closeables to close + private final Deque<Closeable> closeables = new ArrayDeque<>(); + + // set by rethrow method + private Throwable rethrow = null; + + /** + * Create instance of Closer. + */ + public static Closer create() { + return new Closer(); + } + + /** + * Add a {@link Closeable} to the list. + * @param closeable {@link Closeable} object to be added + * @return the closeable param + */ + public @Nullable <CLO extends Closeable> CLO register(@Nullable CLO closeable) { + if (closeable != null) { + closeables.add(closeable); + } + return closeable; + } + + /** + * Closes the set of {@link Closeable}s in reverse order. + * <p> + * Swallows all {@link IOException}s except the first that + * was thrown. + * <p> + * If {@link #rethrow} was called before, throw <em>that</em> + * exception instead (wrapped into a {@link RuntimeException} + * when necessary). + */ + public void close() throws IOException { + // keep track of the IOException to throw + IOException toThrow = null; + + // close all in reverse order + while (!closeables.isEmpty()) { + Closeable closeable = closeables.removeLast(); Review Comment: I think we are closing in the wrong order... We should have a unit test for that! ```suggestion Closeable closeable = closeables.removeFirst(); ``` ########## oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/pio/Closer.java: ########## @@ -0,0 +1,130 @@ +/* + * 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.commons.pio; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.io.Closeable; +import java.io.IOException; +import java.util.ArrayDeque; +import java.util.Deque; +import java.util.Objects; + +/** + * Convenience utility to close a list of {@link Closeable}s. + * <p> + * Inspired by and replacing Guava's Closer. + */ +public class Closer implements Closeable { + + private Closer() { + // no instances for you + } + + // stack of closeables to close + private final Deque<Closeable> closeables = new ArrayDeque<>(); Review Comment: I think the default of 16 is too high for our case. What about 3? ```suggestion private final Deque<Closeable> closeables = new ArrayDeque<>(3); ``` -- 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: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
