On Wednesday 29 April 2009 10:39:02 bo-le wrote:
> 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?

Yes:
- Revert continueWithNewMetadata and the changes to the constructor and to the 
section that starts fetching multi-level metadata.
- Deal with the new metadata in exactly the same way as  
MultiLevelMetadataCallback.onSuccess() does: remove the old metadata, 
substitute the new and call back in to wrapHandleMetadata().
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 835 bytes
Desc: This is a digitally signed message part.
URL: 
<https://emu.freenetproject.org/pipermail/devl/attachments/20090429/5b0381d2/attachment.pgp>

Reply via email to