Am Mittwoch, 29. April 2009 03:21:00 schrieb Matthew Toseland:
> On Monday 20 April 2009 22:21:12 saces at freenetproject.org wrote:
> > Author: saces
> > Date: 2009-04-20 21:21:11 +0000 (Mon, 20 Apr 2009)
> > New Revision: 27121
> >
> > Modified:
> >    trunk/freenet/src/freenet/client/async/SingleFileFetcher.java
> > Log:
> > teach the SFI how to fetch the new metadata format
>
> (ARCHIVE_METADATA_REDIRECT)
>
> > needs review
>
> As requested ... this commit has problems...
>
> > Modified: trunk/freenet/src/freenet/client/async/SingleFileFetcher.java
> > ===================================================================
> > ---
> > trunk/freenet/src/freenet/client/async/SingleFileFetcher.java       
> > 2009-04-20
>
> 21:16:19 UTC (rev 27120)
>
> > +++
> > trunk/freenet/src/freenet/client/async/SingleFileFetcher.java       
> > 2009-04-20
>
> 21:21:11 UTC (rev 27121)
>
> > @@ -114,7 +114,7 @@
> >     /** Copy constructor, modifies a few given fields, don't call
> > schedule(). * Used for things like slave fetchers for MultiLevelMetadata,
> > therefore
>
> does not remember returnBucket,
>
> >      * metaStrings etc. */
> > -   public SingleFileFetcher(SingleFileFetcher fetcher, boolean persistent,
>
> boolean deleteFetchContext, Metadata newMeta, GetCompletionCallback
> callback, FetchContext ctx2, ObjectContainer container, ClientContext
> context) throws FetchException {
>
> > +   public SingleFileFetcher(SingleFileFetcher fetcher, boolean persistent,
>
> boolean deleteFetchContext, Metadata newMeta, ArrayList<String>
> metaStrings2, GetCompletionCallback callback, FetchContext ctx2,
> ObjectContainer container, ClientContext context) throws FetchException {
>
> >             // Don't add a block, we have already fetched the data, we are 
> > just
>
> handling the metadata in a different fetcher.
>
> >             super(persistent ? fetcher.key.cloneKey() : fetcher.key,
>
> fetcher.maxRetries, ctx2, fetcher.parent, callback, false, true,
> fetcher.token, container, context, deleteFetchContext);
>
> >             if(logMINOR) Logger.minor(this, "Creating SingleFileFetcher
>
> for "+fetcher.key+" meta="+fetcher.metaStrings.toString(), new
> Exception("debug"));
>
> > @@ -128,7 +128,7 @@
> >             this.archiveMetadata = null;
> >             this.clientMetadata = (fetcher.clientMetadata != null ?
>
> fetcher.clientMetadata.clone() : new ClientMetadata());
>
> >             this.metadata = newMeta;
> > -           this.metaStrings = new ArrayList<String>();
> > +           this.metaStrings = metaStrings2;
>
> This should be copied unless you can guarantee that the caller will always
> copy it unless nonpersistent.
>
> >             this.addedMetaStrings = 0;
> >             this.recursionLevel = fetcher.recursionLevel + 1;
> >             if(recursionLevel > ctx.maxRecursionLevel)
> > @@ -471,6 +471,83 @@
> >                             }
> >                             metadataBucket.free();
> >                             continue;
> > +                   } else if(metadata.isArchiveMetadataRedirect()) {
> > +                           if(logMINOR) Logger.minor(this, "Is 
> > archive-metadata");
> > +                           // Fetch it from the archive
> > +                           if(ah == null)
> > +                                   throw new 
> > FetchException(FetchException.UNKNOWN_METADATA, "Archive
>
> redirect not in an archive manifest");
>
> > +                           String filename = 
> > metadata.getArchiveInternalName();
> > +                           if(logMINOR) Logger.minor(this, "Fetching 
> > "+filename);
> > +                           Bucket dataBucket = ah.get(filename, actx, 
> > context.archiveManager);
> > +                           if(dataBucket != null) {
> > +                                   if(logMINOR) Logger.minor(this, 
> > "Returning data");
> > +                                   final Metadata newMetadata;
> > +                                   try {
> > +
> > +                                           newMetadata = 
> > Metadata.construct(dataBucket);
> > +                                           dataBucket.free();
> > +                                   } catch (IOException e) {
> > +                                           throw new 
> > FetchException(FetchException.BUCKET_ERROR);
> > +                                   }
> > +                                   continueWithNewMetadata(newMetadata, 
> > this.rcb, container, context);
> > +                                   return;
> > +                           } else {
> > +                                   if(logMINOR) Logger.minor(this, 
> > "Fetching archive
>
> (thisKey="+thisKey+ ')');
>
> > +                                   // Metadata cannot contain pointers to 
> > files which don't exist.
> > +                                   // We enforce this in ArchiveHandler.
> > +                                   // Therefore, the archive needs to be 
> > fetched.
> > +                                   final boolean persistent = 
> > this.persistent;
> > +                                   fetchArchive(true, archiveMetadata, 
> > filename, new
>
> ArchiveExtractCallback() {
>
> > +                                           public void gotBucket(Bucket 
> > data, ObjectContainer container,
>
> ClientContext context) {
>
> > +                                                   if(persistent)
> > +                                                           
> > container.activate(SingleFileFetcher.this, 1);
> > +                                                   if(logMINOR) 
> > Logger.minor(this, "Returning data");
> > +                                                   final Metadata 
> > newMetadata;
> > +                                                   try {
> > +
> > +                                                           newMetadata = 
> > Metadata.construct(data);
> > +                                                           
> > continueWithNewMetadata(newMetadata, SingleFileFetcher.this.rcb,
>
> container, context);
>
> Here we are using the metadata we have just fetched as new metadata.
> ...
>
> > @@ -576,18 +653,7 @@
> >                             if(logMINOR) Logger.minor(this, "Is multi-level 
> > metadata");
> >                             // Fetch on a second SingleFileFetcher, like 
> > with archives.
> >                             metadata.setSimpleRedirect();
> > -                           final SingleFileFetcher f = new 
> > SingleFileFetcher(this, persistent,
>
> false, metadata, new MultiLevelMetadataCallback(), ctx, container,
> context);
>
> > -                           // Clear our own metadata so it can be garbage 
> > collected, it will be
>
> replaced by whatever is fetched.
>
> > -                           // The new fetcher has our metadata so we don't 
> > need to
>
> removeMetadata().
>
> > -                           this.metadata = null;
> > -                           if(persistent) container.store(this);
> > -                           if(persistent) container.store(f);
> > -
> > -                           // We must transition to the sub-fetcher so 
> > that if the request is
>
> cancelled, it will get deleted.
>
> > -                           parent.onTransition(this, f, container);
> > -
> > -                           f.wrapHandleMetadata(true, container, context);
> > -                           if(persistent) container.deactivate(f, 1);
> > +                           continueWithNewMetadata(metadata, new 
> > MultiLevelMetadataCallback(),
>
> container, context);
>
> Here we are using the CURRENT metadata to fetch new metadata in order to
> unpack that metadata and use that. These are *two different operations*.
> The latter is fine, but the former is broken. What you want to do is simply
> remove the old metadata (if persistent) and set metadata to the new
> metadata, and go back around the loop.

i got the ARCHIVE_METADATA_REDIRECT basicly working, 
but get somewhat working does not allways include i have really understand the 
code i changed. :(

is it better to revert the continueWithNewMetadata(..) and include the code 
directly?
>
> >                             return;
> >                     } else if(metadata.isSingleFileRedirect()) {
> >                             if(logMINOR) Logger.minor(this, "Is single-file 
> > redirect");
> > @@ -768,6 +834,22 @@
> >                     }
> >             }
> >     }
> > +
> > +   private void continueWithNewMetadata(Metadata newMetadata,
>
> GetCompletionCallback callback, ObjectContainer container, ClientContext
> context) throws FetchException {
>
> > +           final SingleFileFetcher f = new SingleFileFetcher(this, 
> > persistent,
>
> false, newMetadata, metaStrings, callback, ctx, container, context);
>
> Why do you want to pass in metaStrings? For multi-level metadata we do not
> want to pass it in. For the other case, you don't want to create a new
> SingleFileFetcher.
>
> > +           // Clear our own metadata so it can be garbage collected, it 
> > will be
>
> replaced by whatever is fetched.
>
> > +           // The new fetcher has our metadata so we don't need to
> > removeMetadata(). +         metadata = null;
> > +           if(persistent) container.store(this);
> > +           if(persistent) container.store(f);
> > +
> > +           // We must transition to the sub-fetcher so that if the request 
> > is
>
> cancelled, it will get deleted.
>
> > +           parent.onTransition(this, f, container);
> > +
> > +           f.wrapHandleMetadata(true, container, context);
> > +           if(persistent) container.deactivate(f, 1);
> > +           return;
> > +   }
> >
> >     private String removeMetaString() {
> >             String name = metaStrings.remove(0);
> > @@ -792,7 +874,7 @@
> >             Metadata newMeta = (Metadata) meta.clone();
> >             newMeta.setSimpleRedirect();
> >             final SingleFileFetcher f;
> > -           f = new SingleFileFetcher(this, persistent, true, newMeta, new
>
> ArchiveFetcherCallback(forData, element, callback), new FetchContext(ctx,
> FetchContext.SET_RETURN_ARCHIVES, true, null), container, context);
>
> > +           f = new SingleFileFetcher(this, persistent, true, newMeta, new
>
> ArrayList<String>(), new ArchiveFetcherCallback(forData, element,
> callback), new FetchContext(ctx, FetchContext.SET_RETURN_ARCHIVES, true,
> null), container, context);
>
> This, like the change to SingleFileFetcher's constructor, is unnecessary.
>
> >             if(persistent) container.store(f);
> >             if(logMINOR) Logger.minor(this, "fetchArchive(): "+f);
> >             // Fetch the archive. The archive fetcher callback will unpack 
> > it, and
>
> either call the element



Reply via email to