AbstractSession.storeCookie() support for run-time defined cookie names.

[Tulsagrammer]

Commit b1a0fad introduced a breaking change to AbstractSession.storeCookie() for APIs and applications generating cookie names that can't be fully known at build time. The current code assumes the "=" character separating cookie name and cookie value immediately follows the value stored in tokenType. This assumption breaks down when the server generates a cookie name starting with a fixed character string followed by an additional character string generated at run time.

This proposed change continues to support static cookie names as well as dynamically generated cookie names. Imperative's test:unit was run successfully against this change.

Dynamically generated cookie names might be used by APIs and servers that can listen for connections on multiple ports, where the port has some significance to the server. If that server used a fixed character string for the cookie name, it might have difficulty distinguishing the source of a returned cookie. In our case, we have a server that generates cookie names using the string "EJESWEB_" followed by the port number on which the request was accepted.

[codecov]

Codecov Report

Merging #431 into master will decrease coverage by 0.31%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #431      +/-   ##
==========================================
- Coverage   82.12%   81.81%   -0.32%     
==========================================
  Files         157      157              
  Lines        7754     7616     -138     
  Branches     1459     1375      -84     
==========================================
- Hits         6368     6231     -137     
+ Misses       1382     1381       -1     
  Partials        4        4              

Impacted Files	Coverage Δ
packages/rest/src/session/AbstractSession.ts	96.90% <100.00%> (+0.10%)	⬆️
...ages/rest/src/session/ConnectionPropsForSessCfg.ts	97.87% <0.00%> (-1.15%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c3766bd...5a7dd4d. Read the comment docs.

[ghost]

Thanks for your contribution!

Please add a unit test that verifies dynamic token type support to prevent a regression in the future. The test can be added in packages/rest/__tests__/session/Session.test.ts.

[zFernand0]

LGTM!

[zFernand0]

Just need to address the merge conflict, and this PR should be good to go! 😋

[Tulsagrammer]

@zFernand0: I'm sorry to bug you, but I'm lost. I'm still getting "Conflicting files" on CHANGELOG.md. I just don't see a conflict: I'm just adding a change comment for my storeCookie above the existing 4.7.5 change entry. (The most recent change entry on master.) Any suggestions or advice on what I'm doing wrong?

[ChrisBoehmCA]

If you're using an editor like VSCode, you should be able to try to pull master into your branch and resolve the conflict graphically. https://code.visualstudio.com/docs/editor/versioncontrol (Section Merge conflicts). The git conflict is probably something simple but I haven't pulled your branch to see. Also I think you are supposed to use a "Recent Changes" header in the changelog in pull requests and the version is assigned later after the PR is merged. Fernando can confirm

If you don't have a git "remote" set up to point to this original repository you can do something like this:

git remote add zowe https://github.com/zowe/imperative.git
git pull zowe master

[Tulsagrammer]

@zFernand0: the changelog conflict has been addressed. I notice that two checks appear to have failed. Looking at the details of "Imperative CI / test (10.x, windows-latest)" it appears that the issues causing the test to fail are not related to the files in my pull request. In particular, "packages/rest/tests/session/Session.test.ts" has a PASS. As for "continuous-integration/jenkins/pr-merge", the other failing test, I have no idea what's making that test fail.

[Tulsagrammer]

@zFernand0: as a follow-up to my previous message, it appears that "continuous-integration/jenkins/pr-merge" is failing on issues associated with integration testing. More specifically, "tests/src/packages/profiles/CliProfileManager.credentials.spec.ts" appears to be having problems with secure credential storage: "Couldn't create item: The secret was transferred or encrypted in an invalid way." A second issue seems to be associated with tests of CredentialManagerFactory.ts.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
0 Code Smells

88.9% Coverage
0.0% Duplication

The version of Java (1.8.0_212) you have used to run this analysis is deprecated and we will stop accepting it from October 2020. Please update to at least Java 11.
Read more here

[ghost]

@Tulsagrammer Re-ran the GitHub and Jenkins builds, both failures were transient and the builds passed the second time. The "codecov/project" check is still failing but can be ignored, as it often shows inaccurate reports and "codecov/patch" shows that the PR has sufficient coverage.

[ghost]

LGTM, thanks for addressing the conflict 🙂