474568 - Integrate RecordParser into Resource and Collection

Status: RESOLVED DUPLICATE of bug 485573

(Cloud Services :: General, defect)

Description

[Anant Narayanan [:anant]]

For Fennec, records are parsed out and returned as fetched from a full dump as returned by the server to save on local memory and bandwidth. This functionality should be integrated into Collection.

Comment 1

[Anant Narayanan [:anant]]

Attachment: Patch to expose RecordParser to Collection

This patch creates two different listeners for a Resource. simpleChannelListener simply appends data to local memory as it arrives from the server. recordChannelListener however stores only as much data as is required in order to parse out a single record. Once enough data for a single record is received, _collectionCB() is called with the record contents as a parameter and local data deleted.

Once this patch is approved, the next step would be to actually set the _collectionCB property in Collection.js and handle incoming records appropriately.

Comment 2

[Dan Mills [:thunder]]

Comment on attachment 357936 [details] [diff] [review]
Patch to expose RecordParser to Collection

>-    let listener = new ChannelListener(self.cb, this._onProgress, this._log);
>+    if (this._collectionCB) {
>+      let listener = new recordChannelListener(self.cb,
>+                                                this._onProgress, this);
>+    } else {
>+      let listener = new simpleChannelListener(self.cb,
>+                                                this._onProgress, this);
>+    }

Instead of this, make it so Resources can instantiate any listener:

let listener = new this.listenerObj(...);

and have callers / subclasses set listenerObj to whatever listener they want.

>+function simpleChannelListener(onComplete, onProgress, resource) {
>   this._onComplete = onComplete;
>   this._onProgress = onProgress;
>-  this._log = logger;
>+  this._Resource = resource;

nit: lowercase variables: this._resource

>+    this._Resource._log.debug(channel.requestMethod + " request for " +
>       channel.URI.spec);

Why not move the log statement out of the listener and into Resource._request?  That way you don't need to access Resource's _log property.

>-    this._data = '';
>+    this._Resource._data = '';

nit: use .data, not the private ._data

I think the easier thing to do here is to have only one listener, and just add the record parsing code into Collection itself.  Otherwise you need an event for the collection, plus have the collection access the listener so it can call getNextRecord().

So if you just change the listener to be like simpleChannelListener, then Collection (actually, probably the iterator) can parse records out of .data.

It's not quite as clean in some ways, specially if we ever want to use the record parsing code somewhere else, but that seems unlikely to me--we'd probably subclass Collection at that point.