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

Ensure cache hit for parent directory not found on subsequent queries #16753

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package ch.cyberduck.core;

/*
* Copyright (c) 2002-2021 iterate GmbH. All rights reserved.
* Copyright (c) 2002-2025 iterate GmbH. All rights reserved.
* https://cyberduck.io/
*
* This program is free software; you can redistribute it and/or modify
Expand Down Expand Up @@ -55,16 +55,7 @@ public PathAttributes find(final Path file, final ListProgressListener listener)
log.debug("Cached directory listing does not contain {}", file);
throw new NotfoundException(file.getAbsolute());
}
final CachingListProgressListener caching = new CachingListProgressListener(cache);
try {
final PathAttributes attr = delegate.find(file, new ProxyListProgressListener(listener, caching));
caching.cache();
return attr;
}
catch(NotfoundException e) {
caching.cache();
throw e;
}
return delegate.find(file, new CachingListProgressListener(listener, cache));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package ch.cyberduck.core;

/*
* Copyright (c) 2002-2021 iterate GmbH. All rights reserved.
* Copyright (c) 2002-2025 iterate GmbH. All rights reserved.
* https://cyberduck.io/
*
* This program is free software; you can redistribute it and/or modify
Expand Down Expand Up @@ -54,10 +54,7 @@ public boolean find(final Path file, final ListProgressListener listener) throws
log.debug("Cached directory listing does not contain {}", file);
return false;
}
final CachingListProgressListener caching = new CachingListProgressListener(cache);
final boolean found = delegate.find(file, new ProxyListProgressListener(listener, caching));
caching.cache();
return found;
return delegate.find(file, new CachingListProgressListener(listener, cache));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,37 +15,46 @@
* GNU General Public License for more details.
*/

import ch.cyberduck.core.exception.BackgroundException;
import ch.cyberduck.core.exception.NotfoundException;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import java.util.HashMap;
import java.util.Map;
import java.util.Optional;

public class CachingListProgressListener extends DisabledListProgressListener {
public class CachingListProgressListener extends ProxyListProgressListener {
private static final Logger log = LogManager.getLogger(CachingListProgressListener.class);

private final Cache<Path> cache;
private final Map<Path, AttributedList<Path>> contents = new HashMap<>(1);

public CachingListProgressListener(final Cache<Path> cache) {
public CachingListProgressListener(final ListProgressListener proxy, final Cache<Path> cache) {
super(proxy);
this.cache = cache;
}

@Override
public void chunk(final Path directory, final AttributedList<Path> list) {
contents.put(directory, list);
}

/**
* Add enumerated contents to cache
*/
public void cache() {
for(Map.Entry<Path, AttributedList<Path>> entry : contents.entrySet()) {
final AttributedList<Path> list = entry.getValue();
public void finish(final Path directory, final AttributedList<Path> list, final Optional<BackgroundException> e) {
// Add enumerated contents to cache
if(e.isPresent()) {
if(e.get() instanceof NotfoundException) {
// Parent directory not found
log.debug("Cache empty contents for directory {} after failure {}", directory, e.get().toString());
cache.put(directory, AttributedList.emptyList());
}
else {
log.warn("Failure {} caching contents for {}", e.get().toString(), directory);
}
}
else {
if(!(AttributedList.<Path>emptyList() == list)) {
log.debug("Cache directory listing for {}", entry.getKey());
cache.put(entry.getKey(), list);
log.debug("Cache contents for {}", directory);
cache.put(directory, list);
}
else {
log.warn("Skip caching directory listing for {}", directory);
}
}
super.finish(directory, list, e);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import ch.cyberduck.core.ListProgressListener;
import ch.cyberduck.core.ListService;
import ch.cyberduck.core.Path;
import ch.cyberduck.core.ProxyListProgressListener;
import ch.cyberduck.core.Session;
import ch.cyberduck.core.exception.BackgroundException;
import ch.cyberduck.core.exception.ConnectionCanceledException;
Expand All @@ -40,33 +41,23 @@ public AttributedList<Path> search(final Path workdir, final Filter<Path> filter
return session.getFeature(ListService.class).list(workdir, new SearchListProgressListener(filter, listener)).filter(filter);
}

private static final class SearchListProgressListener implements ListProgressListener {
private static final class SearchListProgressListener extends ProxyListProgressListener {
private final Filter<Path> filter;
private final ListProgressListener delegate;

public SearchListProgressListener(final Filter<Path> filter, final ListProgressListener delegate) {
super(delegate);
this.filter = filter;
this.delegate = delegate;
}

@Override
public void chunk(final Path directory, final AttributedList<Path> list) throws ConnectionCanceledException {
delegate.chunk(directory, list.filter(filter));
super.chunk(directory, list.filter(filter));
}

@Override
public void finish(final Path directory, final AttributedList<Path> list, final Optional<BackgroundException> e) {
delegate.finish(directory, list, e);
}

@Override
public ListProgressListener reset() {
public ListProgressListener reset() throws ConnectionCanceledException {
super.reset();
return this;
}

@Override
public void message(final String message) {
delegate.message(message);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import java.util.Optional;

public abstract class ListFilteringFeature {
private static final Logger log = LogManager.getLogger(ListFilteringFeature.class);

Expand All @@ -44,7 +46,16 @@ public ListFilteringFeature(final Session<?> session) {
* @return Null if not found
*/
protected Path search(final Path file, final ListProgressListener listener) throws BackgroundException {
final AttributedList<Path> list = session._getFeature(ListService.class).list(file.getParent(), listener);
final Path directory = file.getParent();
final AttributedList<Path> list;
try {
list = session._getFeature(ListService.class).list(directory, listener);
}
catch(BackgroundException e) {
listener.finish(directory, AttributedList.emptyList(), Optional.of(e));
throw e;
}
listener.finish(directory, list, Optional.empty());
// Try to match path only as the version might have changed in the meantime
final Path found = list.find(new ListFilteringPredicate(session.getCaseSensitivity(), file));
if(null == found) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
*/

import ch.cyberduck.core.Cache;
import ch.cyberduck.core.CachingAttributesFinderFeature;
import ch.cyberduck.core.CachingListProgressListener;
import ch.cyberduck.core.DisabledListProgressListener;
import ch.cyberduck.core.LocaleFactory;
import ch.cyberduck.core.Path;
import ch.cyberduck.core.PathAttributes;
Expand Down Expand Up @@ -44,10 +45,13 @@ public AttributesWorker(final Cache<Path> cache, final Path file) {
@Override
public PathAttributes run(final Session<?> session) throws BackgroundException {
log.debug("Read latest attributes for file {}", file);
final AttributesFinder find = new CachingAttributesFinderFeature(session, cache, session.getFeature(AttributesFinder.class));
final PathAttributes attr = find.find(file);
log.debug("Return {} for file {}", attr, file);
return attr;
return session.getFeature(AttributesFinder.class).find(file, new WorkerCanceledListProgressListener(this,
new CachingListProgressListener(new DisabledListProgressListener(), cache)));
}

@Override
public void cleanup(final PathAttributes result) {
log.debug("Return {} for file {}", result, file);
}

@Override
Expand Down
75 changes: 75 additions & 0 deletions core/src/main/java/ch/cyberduck/core/worker/FindWorker.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package ch.cyberduck.core.worker;

/*
* Copyright (c) 2002-2025 iterate GmbH. All rights reserved.
* https://cyberduck.io/
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*/

import ch.cyberduck.core.Cache;
import ch.cyberduck.core.CachingListProgressListener;
import ch.cyberduck.core.DisabledListProgressListener;
import ch.cyberduck.core.Path;
import ch.cyberduck.core.Session;
import ch.cyberduck.core.exception.BackgroundException;
import ch.cyberduck.core.features.Find;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import java.util.Objects;

public class FindWorker extends Worker<Boolean> {
private static final Logger log = LogManager.getLogger(FindWorker.class.getName());

private final Cache<Path> cache;
private final Path file;

public FindWorker(final Cache<Path> cache, final Path file) {
this.file = file;
this.cache = cache;
}

@Override
public Boolean run(final Session<?> session) throws BackgroundException {
log.debug("Find file {}", file);
return session.getFeature(Find.class).find(file, new WorkerCanceledListProgressListener(this,
new CachingListProgressListener(new DisabledListProgressListener(), cache)));
}

@Override
public void cleanup(final Boolean result) {
log.debug("Return {} for file {}", result, file);
}

@Override
public Boolean initialize() {
return false;
}

@Override
public final boolean equals(final Object o) {
if(this == o) {
return true;
}
if(o == null || getClass() != o.getClass()) {
return false;
}
final FindWorker that = (FindWorker) o;
return Objects.equals(file, that.file);
}

@Override
public int hashCode() {
return Objects.hashCode(file);
}
}
32 changes: 3 additions & 29 deletions core/src/main/java/ch/cyberduck/core/worker/ListWorker.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,13 @@

import ch.cyberduck.core.AttributedList;
import ch.cyberduck.core.Cache;
import ch.cyberduck.core.CachingListProgressListener;
import ch.cyberduck.core.ListProgressListener;
import ch.cyberduck.core.ListService;
import ch.cyberduck.core.LocaleFactory;
import ch.cyberduck.core.Path;
import ch.cyberduck.core.ProxyListProgressListener;
import ch.cyberduck.core.Session;
import ch.cyberduck.core.exception.BackgroundException;
import ch.cyberduck.core.exception.ConnectionCanceledException;
import ch.cyberduck.core.exception.ListCanceledException;

import org.apache.logging.log4j.LogManager;
Expand All @@ -46,7 +45,7 @@ public class ListWorker extends Worker<AttributedList<Path>> {
public ListWorker(final Cache<Path> cache, final Path directory, final ListProgressListener listener) {
this.cache = cache;
this.directory = directory;
this.listener = new ConnectionCancelListProgressListener(this, directory, listener);
this.listener = new WorkerCanceledListProgressListener(this, new CachingListProgressListener(listener, cache));
}

@Override
Expand Down Expand Up @@ -93,11 +92,6 @@ public void cleanup(final AttributedList<Path> list) {
log.debug("Notify listener {} with empty result set for {}", listener, directory);
listener.finish(directory, AttributedList.emptyList(), Optional.empty());
}
else {
log.debug("Cache contents for {}", directory);
// Cache directory listing
cache.put(directory, list);
}
}

@Override
Expand Down Expand Up @@ -133,29 +127,9 @@ public int hashCode() {

@Override
public String toString() {
final StringBuilder sb = new StringBuilder("SessionListWorker{");
final StringBuilder sb = new StringBuilder("ListWorker{");
sb.append("directory=").append(directory);
sb.append('}');
return sb.toString();
}

private static final class ConnectionCancelListProgressListener extends ProxyListProgressListener {
private final Worker worker;
private final Path directory;

public ConnectionCancelListProgressListener(final Worker worker, final Path directory, final ListProgressListener proxy) {
super(proxy);
this.worker = worker;
this.directory = directory;
}

@Override
public void chunk(final Path directory, final AttributedList<Path> list) throws ConnectionCanceledException {
log.info("Retrieved chunk of {} items in {}", list.size(), this.directory);
if(worker.isCanceled()) {
throw new ListCanceledException(list);
}
super.chunk(this.directory, list);
}
}
}
Loading
Loading