Unattaching interceptors

[alexanderchr]

It must be possible to unattach interceptors from a client. I see two ways to allow that:

1. Keep some data about each transformer in the transformers array, to allow finding and removing a transformer without calling it.
2. Move transformers out of the request builder and handle them seperately. They were just moved there though, so i assume there is a good reason for them to be there.

I think option 1 is better, because then it will also be possible to remove transformers that modify the same property:

client.configure(x => {
  x.asGet(); // unnessecary to call
  x.asDelete();
}

What do you say?

[Mordred]

Why you want to unattach interceptor?

If you need to run interceptor only for some requests, don't add them during configure:

client.createRequest(url).asGet().withInterceptor(interceptor).send();

Interceptor will be used only for current request.

[alexanderchr]

I can think of at least a few instances where i'd like to keep an interceptor attached for a while and then replace it or disable it entirely.

The same problems arise when configuring a client to use headers. Some token authorization systems hands out a new auth token each request. This means that after a few requests a long chain of message.headers.add(key, value) will be called, while only one is really necessary. An option would be to use request interceptors, but right now it's not possible to inject headers that way.

I'd imagine something like this if this if option 1 were implemented:

transformers = [ { type: 'interceptor',  identifier: interceptorInstance, helperFunction: helperFunc1   }, { type: 'header', identifier: 'Authorization-Token', helperFunction: helperFunc2  } ]

It's quire a step up in complexity though, so I'm not sure if it's really worth it. A better option might be to use a map.

[Mordred]

Did you try make your own service? E.g:

@inject(HttpClient, MyInterceptor)
class ClientWithAuth {

  constructor(client, someInterceptor) {
    this.client = client;
    this.interceptor = someInterceptor;
  }

  prepare(url) {
    return this.client.createRequest().withInterceptor(this.someInterceptor);
  }

  get(url, params) {
    return this.prepare(url).asGet().withParams(params).send();
  }

  // same for other methods, if you need
}

Then in the VM, inject this service instead of HttpClient.

I think that this is more cleaner then modifying transformers.

[alexanderchr]

Yes, that would of course work.

I think you are right that modyfing transformers is not very clean. But I also believe that the current implementation where configuring the same header more than once leads to it being set multiple times during the request is even more unclean.

If we are keeping transformer configuration permanent some restrictions has to be placed on it. If a user tries to set the same header twice using configure, an error should be thrown. Otherwise some of us are going to use configure to set new authorization tokens with every request, and will find the requests getting inexplicably slower and slower.

[migajek]

While I agree that the possibility of removing interceptors might not be very clean, I think it's the only way for decoupled, DRY, minimum-effort pluggable solutions.

I'd like to have a component which would reflect the http requests state (kind of loading indicator). To make the component integration minimum-effort, it cannot relay on forcing users to use custom wrappers/subclasses (like ClientWithAuth above) Obviously I could register life-long interceptor once plugin is configured and use Event aggregation to propagate the state to the UI - but I feel like it's overcomplicating the simple problem

[bryanrsmith]

Sounds good to me. Anyone care to work on a PR?

[alexanderchr]

I'll try to get to it later today

[EisenbergEffect]

@alexanderchr Any progress on the pr? It needs to get in in the next couple of days to be considered for the beta.

[Alexander-Taran]

stale from october 2015

[Alexander-Taran]

can be closed