Dhruti90 / google-gdata1111

Automatically exported from code.google.com/p/google-gdata
0 stars 0 forks source link

Using StreamSendAsync to for async upload does not return the resulting feed/entry #293

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. use service.StreamSendAsync(BaseUri, _stream, GDataRequestType.Insert, 
contentType, title, this);
2. handle the AsyncOperationCompleted event
3. the AsyncOperationCompletedEventArgs does not contain the Feed/Entry nor 
Stream that was received from the operation.

What is the expected output? What do you see instead?
I was expecting to get at least the stream, but since the StreamSendAsync 
is using an AsyncSendData, the AsyncOperationCompletedEventArgs does not 
retrieve it.

Please use labels and text to provide additional information.
I think it should be handled in a similar manner to the 
QueryFeedAync/QueryStreamAync methods (though the naming can be 
problematic) - 
1. adding a bool ParseFeed to the AsyncSendData class.
2. create an overload to StreamSendAsync that gets a bool value indicating 
whether we should parse or not.
3. get the returned value from StreamSend in AsyncStreamSendWorker, and 
parse it into a Feed/Entry depending on the above value.

I will make a patch along these lines - but should I parse into a Feed or 
an Entry? Will all async stream sends result in a single entry, or should I 
not count on that and return a full feed?

Another question - I noticed in the AsyncQueryWorker the stream is being 
parsed into a feed inline, instead of using the CreateAndParseFeed method. 
Is there any special reason for that, or should I change that as well?

Original issue reported on code.google.com by ATGard...@gmail.com on 25 Nov 2009 at 9:32

GoogleCodeExporter commented 9 years ago
As StreamSendAsync can be used to submit a batch request, it should parse into 
a feed, not an entry.

That CreateAndParseFeed is not used looks like a typical case of "i searched 
the whole service.cs file for that 
usage pattern, but missed the one asyncservice.cs" to me.

Original comment by fman...@gmail.com on 25 Nov 2009 at 10:22

GoogleCodeExporter commented 9 years ago
I created a patch for this.
- Moved ParseFeed up in the AsyncData class.
- Chained all the constructors in the AsyncData hierarchy to remove code 
duplication.
- Added StreamSendFeedAsync and StreamSendStreamAsync that call the private 
StreamSendAsync with ParseFeed value
- Created a HandleResponseStream method, that gets the stream, and decides 
whether to copy it into a memory stream, 
or parse it into a feed. Puts the result in the AsyncData.
- Changed the AsyncStreamSendWorker and איק AsyncQueryWorker to use the 
above HandleResponseStream.
- Created CopyResponseToMemory just to create the memory stream from the 
resulting stream (not really needed, as 
eventually it is only being called from one location)

Original comment by ATGard...@gmail.com on 25 Nov 2009 at 10:52

Attachments:

GoogleCodeExporter commented 9 years ago
Oopsy - didn't return the stream in case the StreamSendStreamAsync is called.
Need to change in asyncservice.cs line #86 - move the line out of the "if" 
statement.

Will that be ok - returning the stream whether it is an AsyncQueryData or 
AsyncSendData?

Original comment by ATGard...@gmail.com on 25 Nov 2009 at 11:02

GoogleCodeExporter commented 9 years ago
Maybe even simply

--------------------
uri = data.UriToUse;
feedObject = data.Feed;
stream = data.DataStream;
AsyncSendData sData = data as AsyncSendData;
if (sData != null)
{
    entryObject = sData.Entry;
}
--------------------

The feed or stream can be null, all depending on how the response was handled.
Or maybe some code paths will result in both of them being not null?

Original comment by ATGard...@gmail.com on 25 Nov 2009 at 11:06

GoogleCodeExporter commented 9 years ago
You should not return the stream if you have a feed - this will only increase 
likelyhood of memory leaks, and if 
the feed was created, most likely the stream is useless (as not unwindable).

Original comment by fman...@gmail.com on 25 Nov 2009 at 2:25

GoogleCodeExporter commented 9 years ago
I changed the HandleResponseStream to 

-----------------------
            if (data.ParseFeed)
            {
                Tracing.TraceCall("Using Atom always.... ");
                data.Feed = CreateAndParseFeed(responseStream, data.UriToUse);
                data.DataStream = null;
            }
            else
            {
                data.DataStream = CopyResponseToMemory(data, responseStream, contentLength);
            }
-----------------------

that way the data contains either a stream, or a feed.

Original comment by ATGard...@gmail.com on 25 Nov 2009 at 2:43

GoogleCodeExporter commented 9 years ago
Did you put those changes into the patch?

Frank Mantek

Original comment by fman...@gmail.com on 30 Nov 2009 at 10:24

GoogleCodeExporter commented 9 years ago
I will put that in the patch on Sunday, if you'd like.

Original comment by ATGard...@gmail.com on 3 Dec 2009 at 6:20

GoogleCodeExporter commented 9 years ago
Here is my fixed patch, with the HandleResponseStream and the AsyncData 
construtor.

Original comment by ATGard...@gmail.com on 6 Dec 2009 at 7:36

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks for the work. Applied, tested, and checked in.

Frank

Original comment by fman...@gmail.com on 8 Dec 2009 at 1:43

GoogleCodeExporter commented 9 years ago
This issue was closed by revision r963.

Original comment by fman...@gmail.com on 8 Dec 2009 at 1:43