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

Support contact uris #394

Closed
sjudd opened this issue Apr 5, 2015 · 13 comments · Fixed by #1322
Closed

Support contact uris #394

sjudd opened this issue Apr 5, 2015 · 13 comments · Fixed by #1322
Labels
Milestone

Comments

@sjudd
Copy link
Collaborator

sjudd commented Apr 5, 2015

http://stackoverflow.com/questions/29438701/how-to-load-a-uri-with-content-prefix-using-glide-android

@R4md4c
Copy link

R4md4c commented Apr 6, 2016

I'm wondering, What do I need to know about Glide in order to be able to add this feature to Glide? In other words is there some sort of an example to start reading from that could enrich my knowledge about Glide?

@TWiStErRob
Copy link
Collaborator

PR or for yourself?

@R4md4c
Copy link

R4md4c commented Apr 6, 2016

Yes, a PR, I'd like to send a PR.

@TWiStErRob
Copy link
Collaborator

Cool :) 3.8.0 or v4?
(I'm going out now, I'll give you details later)
Check out https://github.com/TWiStErRob/glide-support/ in the meantime.

@R4md4c
Copy link

R4md4c commented Apr 6, 2016

I guess 3.8.0 will be good, as this feature would reach many users who could be afraid of migrating to v4 when it gets released.

@TWiStErRob
Copy link
Collaborator

@R4md4c Thank you very much for coming with a request like this, it's awesome!

To start dev: first do Build (probably worth doing it on your fork). Then if it works in command line, then Development.

This feature needs to be integrated into the leftmost arrow in this flow

Once your env is up take a look at StringLoader, UriLoader and its descenants. I think you'll need modify UriLoader.getLocalUriFetcher or StreamLocalUriFetcher.loadResource (latter looks more reusable) to check the authority (Uri.getHost()) and then load the best available version with ContactsContract.Contacts.openContactPhotoInputStream or similar (you likely need to re-implement it for compatibility with older platforms). Use the latest available API with SDK_INT conditions if it's worth it. Similarly for the descriptor version.

A thing to look out for may be that the following needs to continue to work after this new feature is introduced (on API 14+):

Glide.with(...)
    .load(Uri.withAppendedPath(contactUri, Contacts.Photo.DISPLAY_PHOTO)

because the authority matches, but it doesn't need to be pushed through openContactPhotoInputStream for it to work (it should work with current impl).

Useful resource for debugging: https://github.com/bumptech/glide/wiki/Debugging-and-Error-Handling.

@TWiStErRob TWiStErRob added this to the 3.8.0 milestone Apr 6, 2016
@R4md4c
Copy link

R4md4c commented Apr 6, 2016

Thanks for the super helpful introduction!

I'll try to do what I could, and I think I can do it, but I might ask some questions especially when writing tests.

The branch that I should work on is 3.0, right?

@TWiStErRob
Copy link
Collaborator

Yep.

@R4md4c
Copy link

R4md4c commented Apr 6, 2016

If I wanted to test loading a contact photo what is the best way to test? Do I create an application for that? or Do I use Robolectric?

I don't know if Robolectric support testing a contact Content Uri or not, Is there any test class that might have a similar use case?

@TWiStErRob
Copy link
Collaborator

I think you should use Roboelectric and mock the ContentResolver to return a MatrixCursor with the right columns; for visual test/debug I guess you can modify one of the sample apps. @sjudd probably can tell you more about this.

@R4md4c
Copy link

R4md4c commented Apr 6, 2016

For the time being I hacked an existing project to make sure that loading contacts works.
I've had a look on how Picasso handling this feature and took the code from it, (I don't know if it was permitted to take code from Picasso or not).

It now supports these Uri variations.

/** A lookup uri (e.g. content://com.android.contacts/contacts/lookup/3570i61d948d30808e537) */
    private static final int ID_LOOKUP = 1;
    /** A contact thumbnail uri (e.g. content://com.android.contacts/contacts/38/photo) */
    private static final int ID_THUMBNAIL = 2;
    /** A contact uri (e.g. content://com.android.contacts/contacts/38) */
    private static final int ID_CONTACT = 3;
    /**
     * A contact display photo (high resolution) uri
     * (e.g. content://com.android.contacts/5/display_photo)
     */
    private static final int ID_DISPLAY_PHOTO = 4;

@sjudd
Copy link
Collaborator Author

sjudd commented Apr 7, 2016

Robolectric requires more or less mocking out the entire contacts provider. You can use it, but probably a better idea is to just create a small sample app. It can be as simple as just loading the contact photo from one particular contact, or just loading all of the contacts into a list.

R4md4c added a commit to R4md4c/glide that referenced this issue Apr 7, 2016
R4md4c added a commit to R4md4c/glide that referenced this issue Apr 7, 2016
R4md4c added a commit to R4md4c/glide that referenced this issue Apr 7, 2016
R4md4c added a commit to R4md4c/glide that referenced this issue Apr 8, 2016
R4md4c added a commit to R4md4c/glide that referenced this issue Apr 8, 2016
R4md4c added a commit to R4md4c/glide that referenced this issue Apr 9, 2016
sjudd added a commit that referenced this issue Apr 18, 2016
@TWiStErRob
Copy link
Collaborator

Closing since #1119 is merged.

TWiStErRob added a commit to TWiStErRob/glide that referenced this issue Jul 10, 2016
(minor cosmetic changes were applied and more examples added)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants