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

Improve MultiValueTable #829

Merged
merged 25 commits into from
Nov 30, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
a877b23
improve MultiValueTable
venfernand Feb 12, 2023
573cee3
Merge branch 'next' into fred-multi-value-table-improvement
venfernand Apr 21, 2023
00f9ef2
restore previous public API of freenet/support/MultiValueTable.java
venfernand Oct 8, 2023
0525735
Merge branch 'next' into fred-multi-value-table-improvement
venfernand Oct 8, 2023
ab7e17c
fix code indentation in freenet/support/MultiValueTable.java
venfernand Oct 8, 2023
a9aae2d
Merge branch 'next' into fred-multi-value-table-improvement
venfernand Oct 20, 2023
0a32278
Fix comment in ToadletContextImpl.sendReplyHeaders()
venfernand Oct 20, 2023
7db50fd
Expand star imports in ToadletContextImpl
venfernand Oct 20, 2023
3bfbc44
Move assignment of FProxyFetchTracker.fetchers into the place where v…
venfernand Oct 20, 2023
c6f8d7f
Remove unused import from FProxyFetchTracker
venfernand Oct 20, 2023
3b5523d
Improve method FProxyFetchTracker.getFetchInProgress()
venfernand Oct 20, 2023
2eef2a0
Remove the MultiValueTable.removeAndGet() method
venfernand Oct 20, 2023
7f8ef91
Return a Collection from MultiValueTable.values()
venfernand Oct 20, 2023
2d85389
Make MultiValueTable.putAll() to accept sub-types
venfernand Oct 20, 2023
5573a5f
Change deprecation note in MultiValueTable.getArray() method Javadoc
venfernand Oct 21, 2023
c48a27b
Replace CopyOnWriteArrayList with unmodifiable list in MultiValueTable
venfernand Oct 20, 2023
98e1cf7
Improve immutability of values returned from MultiValueTable and docu…
venfernand Oct 21, 2023
7fed29f
Add deprecation notes for MultiValueTable.get(key) method
venfernand Oct 21, 2023
2217f1f
Restore previous behavior of MultiValueTable.getArray() method
venfernand Oct 21, 2023
f7d34e3
Restore previous behavior of MultiValueTable.getSync() and return a V…
venfernand Oct 22, 2023
117dad4
Remove redundant null check from RequestStatusCache.getShadowBucket()
venfernand Oct 22, 2023
e7ff0e4
Remove redundant null check from FProxyFetchTracker.getFetchInProgress()
venfernand Oct 22, 2023
b79a2a0
Merge branch 'next' into fred-multi-value-table-improvement
ArneBab Nov 30, 2024
36cd30b
Fix merge error
ArneBab Nov 30, 2024
643cbee
Fix merge error
ArneBab Nov 30, 2024
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
48 changes: 31 additions & 17 deletions src/freenet/clients/fcp/RequestStatusCache.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import freenet.client.ClientMetadata;
import freenet.client.FetchException.FetchExceptionMode;
Expand All @@ -28,16 +29,16 @@ public class RequestStatusCache {

private final ArrayList<RequestStatus> downloads;
private final ArrayList<RequestStatus> uploads;
private final HashMap<String, RequestStatus> requestsByIdentifier;
private final MultiValueTable<FreenetURI, RequestStatus> downloadsByURI;
private final Map<String, RequestStatus> requestsByIdentifier;
private final MultiValueTable<FreenetURI, DownloadRequestStatus> downloadsByURI;
private final MultiValueTable<FreenetURI, RequestStatus> uploadsByFinalURI;

RequestStatusCache() {
downloads = new ArrayList<RequestStatus>();
uploads = new ArrayList<RequestStatus>();
requestsByIdentifier = new HashMap<String, RequestStatus>();
downloadsByURI = new MultiValueTable<FreenetURI, RequestStatus>();
uploadsByFinalURI = new MultiValueTable<FreenetURI, RequestStatus>();
downloads = new ArrayList<>();
uploads = new ArrayList<>();
requestsByIdentifier = new HashMap<>();
downloadsByURI = new MultiValueTable<>();
uploadsByFinalURI = new MultiValueTable<>();
}

synchronized void addDownload(DownloadRequestStatus status) {
Expand Down Expand Up @@ -112,7 +113,7 @@ synchronized void removeByIdentifier(String identifier) {
downloads.remove(status);
FreenetURI uri = status.getURI();
assert(uri != null);
downloadsByURI.removeElement(uri, status);
downloadsByURI.removeElement(uri, (DownloadRequestStatus) status);
Bombe marked this conversation as resolved.
Show resolved Hide resolved
} else if(status instanceof UploadRequestStatus) {
uploads.remove(status);
FreenetURI uri = ((UploadRequestStatus) status).getFinalURI();
Expand Down Expand Up @@ -192,17 +193,30 @@ public synchronized void updateStarted(String identifier, FreenetURI redirect) {
}

public synchronized CacheFetchResult getShadowBucket(FreenetURI key, boolean noFilter) {
Object[] downloads = downloadsByURI.getArray(key);
if(downloads == null) return null;
for(Object o : downloads) {
DownloadRequestStatus download = (DownloadRequestStatus) o;
List<DownloadRequestStatus> downloads = downloadsByURI.getAllAsList(key);
if (downloads == null) {
return null;
}
Copy link
Contributor

@bertm bertm Oct 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unreachable return, getAllAsList never returns null

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

for (DownloadRequestStatus download : downloads) {
Bucket data = download.getDataShadow();
if(data == null) continue;
if(data.size() == 0) continue;
if(noFilter && download.filterData) continue;
if (data == null) {
continue;
}
if (data.size() == 0) {
continue;
}
if (noFilter && download.filterData) {
continue;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice change! (we need this at many places — but only where we actually change stuff to avoid breaking existing pull requests we cannot merge yet)

// FIXME it probably *is* worth the effort to allow this when it is overridden on the fetcher, since the user changed the type???
if(download.overriddenDataType) continue;
return new CacheFetchResult(new ClientMetadata(download.getMIMEType()), new NoFreeBucket(data), download.filterData);
if (download.overriddenDataType) {
continue;
}
return new CacheFetchResult(
new ClientMetadata(download.getMIMEType()),
new NoFreeBucket(data),
download.filterData
);
}
return null;
}
Expand Down
8 changes: 4 additions & 4 deletions src/freenet/clients/http/ConfigToadlet.java
Original file line number Diff line number Diff line change
Expand Up @@ -273,12 +273,12 @@ public void handleMethodPOST(URI uri, HTTPRequest request,
}
String params = paramsBuilder.toString();
if (directorySelector) {
MultiValueTable<String, String> headers = new MultiValueTable<String, String>(
1);
// params ends in &. Download directory browser starts in default
// download directory.
headers.put("Location", directoryBrowserPath + params + "path="
+ core.getDownloadsDir().getAbsolutePath());
MultiValueTable<String, String> headers = MultiValueTable.from(
"Location",
directoryBrowserPath + params + "path=" + core.getDownloadsDir().getAbsolutePath()
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

much nicer — thank you!

ctx.sendReplyHeaders(302, "Found", headers, null, 0);
return;
}
Expand Down
7 changes: 4 additions & 3 deletions src/freenet/clients/http/ConnectionsToadlet.java
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,10 @@ public void handleMethodGET(URI uri, final HTTPRequest request, ToadletContext c
if(path.endsWith("myref.fref")) {
SimpleFieldSet fs = getNoderef();
String noderefString = fs.toOrderedStringWithBase64();
MultiValueTable<String, String> extraHeaders = new MultiValueTable<String, String>();
// Force download to disk
extraHeaders.put("Content-Disposition", "attachment; filename=myref.fref");
MultiValueTable<String, String> extraHeaders = MultiValueTable.from(
// Force download to disk
"Content-Disposition", "attachment; filename=myref.fref"
);
writeReply(ctx, 200, "application/x-freenet-reference", "OK", extraHeaders, noderefString);
return;
}
Expand Down
24 changes: 12 additions & 12 deletions src/freenet/clients/http/ContentFilterToadlet.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public class ContentFilterToadlet extends Toadlet implements LinkEnabledCallback
/**
* What to do the the output from the content filter.
*/
public static enum ResultHandling {
public enum ResultHandling {
DISPLAY,
SAVE
}
Expand Down Expand Up @@ -90,17 +90,17 @@ public void handleMethodPOST(URI uri, final HTTPRequest request, final ToadletCo
FilterOperation filterOperation = getFilterOperation(request);
ResultHandling resultHandling = getResultHandling(request);
String mimeType = request.getPartAsStringFailsafe("mime-type", 100);
MultiValueTable<String, String> responseHeaders = new MultiValueTable<String, String>();
responseHeaders.put("Location", LocalFileFilterToadlet.PATH
+ "?filter-operation=" + filterOperation
+ "&result-handling=" + resultHandling
+ "&mime-type=" + mimeType);
String location = LocalFileFilterToadlet.PATH
+ "?filter-operation=" + filterOperation
+ "&result-handling=" + resultHandling
+ "&mime-type=" + mimeType;
MultiValueTable<String, String> responseHeaders = MultiValueTable.from("Location", location);
ctx.sendReplyHeaders(302, "Found", responseHeaders, null, 0);
} catch (BadRequestException e) {
String invalidPart = e.getInvalidRequestPart();
if (invalidPart == "filter-operation") {
if ("filter-operation".equals(invalidPart)) {
Bombe marked this conversation as resolved.
Show resolved Hide resolved
writeBadRequestError(l10n("errorMustSpecifyFilterOperationTitle"), l10n("errorMustSpecifyFilterOperation"), ctx, true);
} else if (invalidPart == "result-handling") {
} else if ("result-handling".equals(invalidPart)) {
writeBadRequestError(l10n("errorMustSpecifyResultHandlingTitle"), l10n("errorMustSpecifyResultHandling"), ctx, true);
} else {
writeBadRequestError(l10n("errorBadRequestTitle"), l10n("errorBadRequest"), ctx, true);
Expand Down Expand Up @@ -243,11 +243,11 @@ private void handleFilterRequest(HTTPRequest request, ToadletContext ctx, NodeCl
}
} catch (BadRequestException e) {
String invalidPart = e.getInvalidRequestPart();
if (invalidPart == "filter-operation") {
if ("filter-operation".equals(invalidPart)) {
writeBadRequestError(l10n("errorMustSpecifyFilterOperationTitle"), l10n("errorMustSpecifyFilterOperation"), ctx, true);
} else if (invalidPart == "result-handling") {
} else if ("result-handling".equals(invalidPart)) {
writeBadRequestError(l10n("errorMustSpecifyResultHandlingTitle"), l10n("errorMustSpecifyResultHandling"), ctx, true);
} else if (invalidPart == "filename") {
} else if ("filename".equals(invalidPart)) {
writeBadRequestError(l10n("errorNoFileSelectedTitle"), l10n("errorNoFileSelected"), ctx, true);
} else {
writeBadRequestError(l10n("errorBadRequestTitle"), l10n("errorBadRequest"), ctx, true);
Expand Down Expand Up @@ -309,7 +309,7 @@ private void handleFilter(Bucket data, String mimeType, FilterOperation operatio
ctx.sendReplyHeaders(200, "OK", null, resultMimeType, resultBucket.size());
ctx.writeData(resultBucket);
} else if (resultHandling == ResultHandling.SAVE) {
MultiValueTable<String, String> headers = new MultiValueTable<String, String>();
MultiValueTable<String, String> headers = new MultiValueTable<>();
headers.put("Content-Disposition", "attachment; filename=\"" + resultFilename + '"');
headers.put("Cache-Control", "private");
headers.put("Content-Transfer-Encoding", "binary");
Expand Down
10 changes: 5 additions & 5 deletions src/freenet/clients/http/DarknetConnectionsToadlet.java
Original file line number Diff line number Diff line change
Expand Up @@ -455,8 +455,7 @@ protected void handleAltPost(URI uri, HTTPRequest request, ToadletContext ctx, b
}

private void redirectHere(ToadletContext ctx) throws ToadletContextClosedException, IOException {
MultiValueTable<String, String> headers = new MultiValueTable<String, String>();
headers.put("Location", "/friends/");
MultiValueTable<String, String> headers = MultiValueTable.from("Location", "/friends/");
ctx.sendReplyHeaders(302, "Found", headers, null, 0);
}

Expand Down Expand Up @@ -512,9 +511,10 @@ private boolean tryHandlePeerNoderef(URI uri, HTTPRequest request,
if(fs == null) return false;
String filename = FileUtil.sanitizeFileNameWithExtras(peernode_name+".fref", "\" ");
String content = fs.toString();
MultiValueTable<String, String> extraHeaders = new MultiValueTable<String, String>();
// Force download to disk
extraHeaders.put("Content-Disposition", "attachment; filename="+filename);
MultiValueTable<String, String> extraHeaders = MultiValueTable.from(
// Force download to disk
"Content-Disposition", "attachment; filename="+filename
);
this.writeReply(ctx, 200, "application/x-freenet-reference", "OK", extraHeaders, content);
return true;
} else return false;
Expand Down
6 changes: 2 additions & 4 deletions src/freenet/clients/http/ExternalLinkToadlet.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,23 +34,21 @@ public String path() {

public void handleMethodPOST(URI uri, HTTPRequest request, ToadletContext ctx) throws ToadletContextClosedException, IOException {
String url = request.getPartAsStringFailsafe(magicHTTPEscapeString, MAX_URL_LENGTH);
MultiValueTable<String, String> headers = new MultiValueTable<String, String>();
//If the user clicked cancel, or the URL is not defined, return to the main page.
//TODO: This will mean the beginning of the first time wizard if it's still in progress.
//TODO: Is it worth it to fix that?
if (request.getPartAsStringFailsafe("Go", 32).isEmpty() || url.isEmpty()) {
url = WelcomeToadlet.PATH;
}
headers.put("Location", url);
MultiValueTable<String, String> headers = MultiValueTable.from("Location", url);
ctx.sendReplyHeaders(302, "Found", headers, null, 0);
}

public void handleMethodGET(URI uri, HTTPRequest request, ToadletContext ctx) throws ToadletContextClosedException, IOException {

//Unexpected: a URL should have been specified.
if (request.getParam(magicHTTPEscapeString).isEmpty()) {
MultiValueTable<String, String> headers = new MultiValueTable<String, String>();
headers.put("Location", WelcomeToadlet.PATH);
MultiValueTable<String, String> headers = MultiValueTable.from("Location", WelcomeToadlet.PATH);
ctx.sendReplyHeaders(302, "Found", headers, null, 0);
return;
}
Expand Down
92 changes: 52 additions & 40 deletions src/freenet/clients/http/FProxyFetchTracker.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package freenet.clients.http;

import java.util.ArrayList;
import java.util.Enumeration;
import java.util.List;
import java.util.stream.Collectors;

import freenet.client.FetchContext;
import freenet.client.FetchException;
Expand All @@ -28,7 +28,7 @@ public void shouldUpdate() {
});
}

final MultiValueTable<FreenetURI, FProxyFetchInProgress> fetchers;
private final MultiValueTable<FreenetURI, FProxyFetchInProgress> fetchers = new MultiValueTable<>();
final ClientContext context;
private long fetchIdentifiers;
private final FetchContext fctx;
Expand All @@ -37,7 +37,6 @@ public void shouldUpdate() {
private boolean requeue;

public FProxyFetchTracker(ClientContext context, FetchContext fctx, RequestClient rc) {
fetchers = new MultiValueTable<FreenetURI, FProxyFetchInProgress>();
this.context = context;
this.fctx = fctx;
this.rc = rc;
Expand Down Expand Up @@ -84,26 +83,44 @@ public FProxyFetchWaiter makeWaiterForFetchInProgress(FreenetURI key,long maxSiz
}
return null;
}

/** Gets an FProxyFetchInProgress identified by the URI and the maxsize. If no such FetchInProgress exists, then returns null.
* @param key - The URI of the fetch

/**
* Gets an {@link FProxyFetchInProgress} identified by the URI and having provided max size.
* If optional fetch context parameter is specified,
* then fetch context in {@link FProxyFetchInProgress} is compared to provided fetch context.
* If no such FetchInProgress exists, then returns {@code null}.
*
* @param key - The URI of the fetch
* @param maxSize - The maxSize of the fetch
* @param fctx TODO
* @return The FetchInProgress if found, null otherwise*/
public FProxyFetchInProgress getFetchInProgress(FreenetURI key, long maxSize, FetchContext fctx){
* @param fctx - Optional {@link FetchContext} with fetch parameters
* @return The FetchInProgress if found, {@code null} otherwise
*/
public FProxyFetchInProgress getFetchInProgress(FreenetURI key, long maxSize, FetchContext fctx) {
synchronized (fetchers) {
Object[] check = fetchers.getArray(key);
if(check != null) {
for(int i=0;i<check.length;i++) {
FProxyFetchInProgress progress = (FProxyFetchInProgress) check[i];
if((progress.maxSize == maxSize && progress.notFinishedOrFatallyFinished())
|| progress.hasData()){
if(logMINOR) Logger.minor(this, "Found "+progress);
if(fctx != null && !progress.fetchContextEquivalent(fctx)) continue;
if(logMINOR) Logger.minor(this, "Using "+progress);
return progress;
} else
if(logMINOR) Logger.minor(this, "Skipping "+progress);
List<FProxyFetchInProgress> fetchList = fetchers.getAllAsList(key);
if (fetchList == null) {
return null;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unreachable return; fetchList is never null.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

for (FProxyFetchInProgress fetch : fetchList) {
if ((fetch.maxSize == maxSize && fetch.notFinishedOrFatallyFinished())
|| fetch.hasData()
) {
if (logMINOR) {
Logger.minor(this, "Found " + fetch);
}
if (fctx != null && !fetch.fetchContextEquivalent(fctx)) {
if (logMINOR) {
Logger.minor(this, "Fetch context does not match. Skipping " + fetch);
}
continue;
}
if (logMINOR) {
Logger.minor(this, "Using " + fetch);
}
return fetch;
}
if (logMINOR) {
Logger.minor(this, "Skipping " + fetch);
}
}
}
Expand All @@ -124,8 +141,10 @@ public void queueCancel(FProxyFetchInProgress progress) {

@Override
public void run() {
if(logMINOR) Logger.minor(this, "Removing old FProxyFetchInProgress's");
ArrayList<FProxyFetchInProgress> toRemove = null;
if (logMINOR) {
Logger.minor(this, "Removing old FProxyFetchInProgress's");
}
List<FProxyFetchInProgress> toRemove;
boolean needRequeue = false;
synchronized(fetchers) {
if(requeue) {
Expand All @@ -135,34 +154,27 @@ public void run() {
queuedJob = false;
}
// Horrible hack, FIXME
Enumeration<FreenetURI> e = fetchers.keys();
while(e.hasMoreElements()) {
FreenetURI uri = e.nextElement();
// Really horrible hack, FIXME
for(FProxyFetchInProgress f : fetchers.iterateAll(uri)) {
// FIXME remove on the fly, although cancel must wait
if(f.canCancel()) {
if(toRemove == null) toRemove = new ArrayList<FProxyFetchInProgress>();
toRemove.add(f);
}
}
}
if(toRemove != null)
toRemove = fetchers.values().stream()
// FIXME remove on the fly, although cancel must wait
.filter(FProxyFetchInProgress::canCancel)
.collect(Collectors.toList());

for(FProxyFetchInProgress r : toRemove) {
if(logMINOR){
Logger.minor(this,"Removed fetchinprogress:"+r);
}
fetchers.removeElement(r.uri, r);
}
}
if(toRemove != null)
for(FProxyFetchInProgress r : toRemove) {
if(logMINOR)
if (logMINOR) {
Logger.minor(this, "Cancelling for "+r);
}
r.finishCancel();
}
if(needRequeue)
if(needRequeue) {
context.ticker.queueTimedJob(this, FProxyFetchInProgress.LIFETIME);
}
}

public int makeRandomElementID() {
Expand Down
Loading
Loading