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

Updating a requests RequestOptions #2858

Closed
MFlisar opened this issue Feb 1, 2018 · 4 comments
Closed

Updating a requests RequestOptions #2858

MFlisar opened this issue Feb 1, 2018 · 4 comments

Comments

@MFlisar
Copy link

MFlisar commented Feb 1, 2018

Glide Version: 4.5

Integration libraries: none

Device/Android Version: all

Issue details / Repro steps / Use case background:

I would like to have following feature: updating request options.

Why

I manage image loading in a simple class, so it's not a big problem here to solve my issue, but I'm preparing requests and hand them on from function to function. In the end I just start the reqeust with a target. So far so good.

Sometimes I want to manipulate the request based on the position where I called it from. In my case, I have one ModelLoader that executes requests internally to prepare the images it needs. Therefore I would like to be able to update the options in the request (it's immutable I think, making a copy of them is fine for me here). Or to have a possibility to get the current options, create new ones based on them and set the resulting options to my request.

Examples

 RequestBuilder request = ...;

// either this
RequestOptions ro = request.getRequestOptions();
ro.useUnlimitedSourceGeneratorsPool(true);

// or this
RequestOptions ro = request.getCopyOfRequestOptions();
ro.useUnlimitedSourceGeneratorsPool(true);
request.apply(ro);

// or this
RequestOptions ro = new ReqeustOptions().useUnlimitedSourceGeneratorsPool(true);
reqeust.update(ro); // this could merge all set values in ro into the internal RequestOptions

Any reasons to not support this sort of thing in any way?

@sjudd
Copy link
Collaborator

sjudd commented Feb 1, 2018

You can already apply RequestOptions to both RequestBuilders and other RequestOptions. The only caveat is that the apply method may return a new object, which it sounds like you're fine with.

@sjudd sjudd added the question label Feb 1, 2018
@MFlisar
Copy link
Author

MFlisar commented Feb 2, 2018

Here are my final functions every request runs through.

I use a custom target to identify synchronous calls without the need of changing anything in my ImageManager. I have already a lot of functions for Targets/ImageView and I just wanted to reuse them like this.

The functions show what I would like to do - add an option to the options already present in the RequestBuilder. I could adjust my functions to just pass through the RequestOptions or to adjust them bevorehand (what I currently do anyways), I am just curious if I could solve this in another way in the future.

private static void loadIcon(RequestBuilder request, ImageView iv) {
    request.listener(new LoggingListener());
    request.into(iv).waitForLayout();
}

private static void loadIcon(RequestBuilder<Drawable> request, Target<Drawable> target) {
    request.listener(new LoggingListener());
    // check if I find my InstantTarget => if so, this is an 
    if (target instanceof InstantTarget) {
        FutureTarget t = null;
        try {
            if (t.isNestedCall()) {
                // I can't access already existing options here! I don't want to replace them
               // but only add one flag to them and keep all other already existing options
                request.apply(<ORIGINAL options + useUnlimitedSourceGeneratorsPool>);
            }
            t = request.submit(((InstantTarget) target).getWidth(), ((InstantTarget) target).getHeight());
            Drawable d = (Drawable) t.get();
            ((InstantTarget) target).setDrawable(d);
        } catch (InterruptedException e) {
            L.e(e);
        } catch (ExecutionException e) {
            L.e(e);
        } finally {
            if (t != null) {
                clear(t);
            }
        }
    } else {
        request.into(target);
    }
}

@sjudd
Copy link
Collaborator

sjudd commented Feb 2, 2018

apply doesn't replace the previous set of options, it just applies whatever new options you specified. It already works the way you want it to.

@MFlisar
Copy link
Author

MFlisar commented Feb 4, 2018

Thanks, did not realise this

@MFlisar MFlisar closed this as completed Feb 4, 2018
sjudd added a commit to sjudd/glide that referenced this issue Feb 5, 2018
sjudd added a commit to sjudd/glide that referenced this issue Feb 5, 2018
aw691190716 pushed a commit to aw691190716/glide that referenced this issue Feb 8, 2018
sjudd added a commit to sjudd/glide that referenced this issue Mar 6, 2018
-------------
Include the debug aar in release artifacts for Android projects.

We removed the release variant a while ago to speed up the build, which
has the side affect of removing the release aar from artifacts. Since
we expect the debug and release variants to be identical (hence why
we disabled the release variant), it should be safe to just use the
debug aar instead. We will have to specify it explicitly since android’s
rules unsurprisingly only add the release variant by default.

-------------
Bump version to 4.6.0

-------------
Update readme to 4.6.0

Also removes the old v4 dependency from maven deps, I don’t think it’s
necessary.

-------------
Change update_javadocs to use debugJavadocJar instead of release.

We’ve disabled the release variant.

-------------
Bump version to 4.7.0-SNAPSHOT

-------------
Add POM dependencies explicitly.

Fixes bumptech#2863.

-------------
Bump version to 4.6.1

-------------
Update readme to 4.6.1

-------------
Fix param mistake (bumptech#2873)

-------------
Update SimpleTarget javadoc to match v4 API.

-------------
Add javadoc for RequestOptions.apply/RequestBuilder.apply.

Related to bumptech#2858.

-------------
Add support for Uri data uris.

Previously we only supported data uris if they were provided as Strings.

Fixes bumptech#556.

-------------
Make GlideBuilder.build package private.

It shouldn’t have been made visible and can’t
safely be used directly outside of the library.

Fixes bumptech#2885

-------------
Handle notifications in MultiFetcher after cancellation.

We can’t guarantee that every fetcher implementation will strictly avoid
notifying after they’ve been cancelled.

This also improves the behavior in VolleyStreamFetcher so that it attempts to avoid notifying after cancel, although it still doesn’t make
any strict guarantee.

Fixes bumptech#2879

-------------
Re-enable -Werror for java compilation.

Related to bumptech#2886.

-------------
Fix a deprecation warning in DataUriTest.

-------------
gradle 4.5.1

-------------
deprecate fragments

-------------
add @deprecated to javadoc and suppress deprecations in code

-------------
Created by MOE: https://github.com/google/moe

MOE_MIGRATED_REVID=99229725401d5777e059da7b6331134bf73fbcdf

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=185535564
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants