Change defaultHeaders

[aayusharyan]

I was checking FlowJS Compatability with S3. I realized there is one thing we need to fix. 👇

There are few headers which are being sent by default. However, if we don't want them, we have to change it at the code level... Ideally, it should be at the opts level configurable.

I would suggest two things.

1. Make headers a function instead of array|function.
2. Check if headers function is defined, then not use the default getHeaders method.

This way, if the we want, then we can change the default header with ease. Suggestion 1 can be ignored and we can keep headers as array|function. But I want to make it strict that if headers is defined, then default getHeaders should not be called.

What do you think @AidasK ?

[AidasK]

https://github.com/flowjs/flow.js/blob/master/dist/flow.js#L1521

are you sure current implementation is not suitable?

[aayusharyan]

Ohhhhh, my bad. I confused two things.

My concern is getParams function. It is used to send default data. I don't want to send the default query Params, and there is no way to stop that. Technically, changeRawDataBeforeSend can be used to remove the query params, but that would require knowledge of blob and will make implementation more complicated for doing a thing as simple as stopping default Params.

I want to check query. If that is defined, then use that instead of getParams... And any which way it can be a function so it can be used to get chunk/file information and use that.

[AidasK]

Yes, we don't need this line: https://github.com/flowjs/flow.js/blob/master/dist/flow.js#L1496

Maybe

var query = evalOpts(this.flowObj.opts.query, this.fileObj, this, isTest);
      query = extend(query || {}, this.getParams());

should be written as (just a quick example, maybe you can write it better)

    if (typeof his.flowObj.opts.query === "function") {
var query = evalOpts(this.flowObj.opts.query, this.fileObj, this, isTest, this.getParams());
else {
query = this.flowObj.opts.query.
}

ofc you would need to create a pull request, document it well and release as 3.0.0 release

[aayusharyan]

Yeah, I was thinking same. I'll do that. 👍

[drzraf]

I had the exact same problem with signed temporary URL of Swift. My workaround to live-patch getParams() for this specific chunk, before it get called by later in prepareXhrRequest

query: (fileObj, chunk, isTest) => {
    chunk.getParams = e => {};
}

Suitable as safe/advised practice?

[aayusharyan]

@drzraf This is fine. You can check the documentation as well for customising query.

query: Extra parameters to include in the multipart POST with data. This can be an object or a function. If a function, it will be passed a FlowFile, a FlowChunk object and a isTest boolean (Default: {})

[drzraf]

Yes, indeed. I wondered, because it normally returns an object later merged with this.getParams(). Given this expectations, profiting from the function call to override another object's method sounded... hacky.

[aayusharyan]

@drzraf If it's a function, it is evaluated here. BTW, what do you suggest?

[drzraf]

The above construct in https://github.com/flowjs/flow.js/issues/283#issuecomment-576592340 is good. (But keep this.getParams(); is query is not a function to maintain backward compatibility)

[AidasK]

or we could just add config option such as this.flowObj.opts.overwiriteQuery. If it is true, we are not going merge/extend existing result.