adobe / acc-js-sdk

A JavaScript SDK for Adobe Campaign Classic
Apache License 2.0
21 stars 22 forks source link

[NEO-78835]: fix extraHttpHeaders issue #109

Closed pankajkrk8 closed 2 months ago

pankajkrk8 commented 3 months ago

Description

The extraHttpHeaders passed to the connection parameters which is used to initialise the client, is not present in the XtkJobInterface's function and FileUploader APIs

const connectionParameters = sdk.ConnectionParameters.ofBearerToken(targetDomain, imsToken, {extraHttpHeaders: {testHeader: 'headerValue'}});
const clientSdk = await sdk.init(connectionParameters).catch(() => false);
await clientSdk.logon();

const job = await clientSdk.jobInterface({
  xtkschema: "nms:dataModel",
  method: "write",
  jobId: name,
  args: dataModelJSON,
});

// This API call does not have the extraHttpHeaders in the headers.
await job.submitSoapCall();

Also, the submit and execute APIs does not have the extraHttpHeaders in the header. Although, all the other methods like getStatus, getResult, cancel, pause, waitJobCancelled and hasWarning already support the extraHttpHeaders as they are being handled by the Proxy clientHandler.

Similarly, we have this issue in the fileUploader APIs.

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

pankajkrk8 commented 2 months ago

I see in the PR that you have modified a couple of tests to pass connectionParameters: { _options: {}} }. Could you explain why? Normally unit tests serve as regression tests, so any changes in the tests is a sign that there could be a regression.

Also, I see don't see any new tests regarding this PR, could you add some? We'd like to see tests which asserts that the extra headers are correctly passed now, i.e. those tests should fail with the previous code and succeed with the new code

The connectionParameter is something which will always be present in the client object. But the client mocking in XtkJob test cases did not have it, so I added it. If we use the MockClient, then we do not have to add this required parameter.

Also, I have added the test cases for extra headers.