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

Add new ParseFile download api #113

Merged

Conversation

wangmengyan95
Copy link
Contributor

Support fetch a ParseFile as File pointer or InputStream.
File related new apis:

  1. public File getFile() throws ParseException
  2. public Task<File> getFileInBackground(final ProgressCallback progressCallback)
  3. public Task<File> getFileInBackground()
  4. public void getFileInBackground(GetFileCallback fileCallback, final ProgressCallback progressCallback)
  5. public void getFileInBackground(GetFileCallback fileCallback)

DataStream related new apis:

  1. public File getDataStream() throws ParseException
  2. public Task<File> getDataStreamInBackground(final ProgressCallback progressCallback)
  3. public Task<File> getDataStreamInBackground()
  4. public void getDataStreamInBackground(GetDataStreamCallback fileCallback, final ProgressCallback progressCallback)
  5. public void getDataStreamInBackground(GetDataStreamCallback fileCallback)

@wangmengyan95 wangmengyan95 force-pushed the wangmengyan.t8120706_add_download_file_stream_api branch from e25570a to 9d6163b Compare September 9, 2015 00:44
@wangmengyan95 wangmengyan95 added this to the 1.10.2 milestone Sep 10, 2015
@grantland grantland modified the milestones: 1.10.2, 1.10.3 Sep 10, 2015
*/
@Override
public void done(File file, ParseException e);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: newline

@grantland
Copy link
Contributor

Some nits and a thought Q

Do you think the getDataStream APIs are worthwhile when we have getFile APIs?

I think we should update the get(Data|File|DataStream) API documentation to match iOS's since it's a bit more descriptive:

getData:

Synchronously gets the data from cache if available or fetches its contents from the network.

getDataStream:

This method is like getData but avoids ever holding the entire PFFile contents in memory at once.

@wangmengyan95
Copy link
Contributor Author

For getDataStream, this method is not like iOS which does not wait download to cache file finish but open a stream from the cache file directly. In Android, it just wait download finish and read the data from cache file for developers. So I do not hold strong opinions for either sides.

/**
* Synchronously gets the file pointer from cache if available or fetches its content from the
* network. You probably want to use {@link #getFileInBackground()} instead unless you're already
* in a background thread.
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add the following:

<strong>Note: </strong> The {@link File} location may change without notice and should not be stored to be accessed later.

/cc @nlutsenko @richardjrossiii

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@grantland grantland assigned wangmengyan95 and unassigned grantland Oct 1, 2015
@wangmengyan95 wangmengyan95 force-pushed the wangmengyan.t8120706_add_download_file_stream_api branch from d6c28b1 to 4181900 Compare October 5, 2015 17:24
@grantland
Copy link
Contributor

LGTM, feel free to squash and merge!

@wangmengyan95 wangmengyan95 force-pushed the wangmengyan.t8120706_add_download_file_stream_api branch from 4181900 to 831fbbe Compare October 6, 2015 23:45
wangmengyan95 added a commit that referenced this pull request Oct 6, 2015
…ownload_file_stream_api

Add new ParseFile download api
@wangmengyan95 wangmengyan95 merged commit 522ac13 into master Oct 6, 2015
@wangmengyan95 wangmengyan95 deleted the wangmengyan.t8120706_add_download_file_stream_api branch October 6, 2015 23:49
@grantland grantland modified the milestone: 1.11.0 Nov 11, 2015
@facebook-github-bot
Copy link

@wangmengyan95 updated the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants