Refactored storage code + fixed global storage + fixed quickconnect

[sebjulliand]

Changes

This PR fixes the Global storage (used to store the last connection and the cached server settings). It used to use the same BaseStorage object reference as the connection storage. That broke the global storage behavior since it had a storage prefix key assigned once connected (because connecting sets a prefix key on the connection storage...but since the global storage used the same BaseStorage as the connection, it lost its purpose).

Fixing the global storage fixes the "connect to last connected IBM i" action and the cached server settings access.

The quickConnect parameter was also not correctly tested, preventing quick connect from happening when connecting.

How to test this PR

• From a local terminal: Run npm run test to run the test suites
• From a debug session
  • Connect to a system and reload settings
  • Disconnect
  • Check that the system you just connected to is marked as the last connected system (yellow icon)
  • Re-connect normally: it must connect faster than when reloading the settings

Checklist

• have tested my change

[github-actions]

👋 A new build is available for this PR based on efd0a8b.

• Download here.
• Read more about how to test

[github-actions]

👋 A new build is available for this PR based on 7b2d1bc.

• Download here.
• Read more about how to test

[github-actions]

👋 A new build is available for this PR based on f2df2da.

• Download here.
• Read more about how to test

[chrjorgensen]

Why this change? Now changes to the file .env.sample in src\api won't be committed.
Isn't it better to have a positive list of files to ignore, instead of a global ignore, and let it be up to the committer to ensure that no secrets are included?

[chrjorgensen]

It looks like the files .env and .env.sample should not be located in src\api\ and should be moved to the src\api\tests\ subdirectory instead - they are used by the tests framework and nowhere else. Agree?

[sebjulliand]

Excluding all dot files is a common practice; I thought it would be good to enforce it here instead of listing all the unwanted dot files or folder.
Since .env.sample has already been synchronized with Git, any change made to it will actually be committed. But I'll add an exception because, as you suggested, it should be moved elsewhere.

Indeed, moving .env/.env.sample elsewhere sounds like a good idea.

[chrjorgensen]

@sebjulliand Thanks for fixing the storage issue! It's been annoying me for quite a while now, but seemed to complicated for me to try and fix it myself - the TypeScript guru to the rescue! ❤️ 👍

I will give it a spin...

[chrjorgensen]

@sebjulliand I ran the suggested tests, and npm run test ran without issues, but when I ran the extension in debug, I found that

• no connection is set as previous after disconnect, i.e. no >< in yellow in the Connection browser
• the connection does not appear any faster on subsequent connection attempts, so Quick Connect does not seem to work still...

?

[sebjulliand]

Eek, that's weird. If you put a breakpoint in the connection process, anywhere after line 311 in IBMi.ts, what does the quickconnect method returns for you?

[chrjorgensen]

Sorry for the delay... work, you know... 😉

When I put a breakpoint in line 306 in IBMi.ts, I see reloadServerSettings is undefined:

So quickConnect will always return false?

[sebjulliand]

@chrjorgensen it used to always return false because reloadServerSettings was either undefined or true, depending on the action taken to connect.

With my change, quickConnect must return true if reloadServerSettings is false or undefined:

this.config!.quickConnect === true && !options.reloadServerSettings

Try adding a Log point on line 312 with this value:

And then what goes in the Debug Output tab.

[chrjorgensen]

@sebjulliand After adding the log point, I see QuickConnect is false on every connection, even after disconnect and immediate reconnect. My connection has Quick Connect enabled...

[sebjulliand]

I don't get it...if I break after line 312 and run this in the Debug Console:

And it does quick connect...on two different computers.
Well, I'll add the Boolean(...) in quickConnect just to be sure.

[chrjorgensen]

@sebjulliand I found the issue with your changes not working on my laptop - the GitHub extension had not checked out the current changes completely, and my test was run on an older version of your code! 😡

I deleted the branch and checked it out again - and now it all worked, of course! 🎉

I will approve and merge - thank you! 🫶

[sebjulliand]

Great, thank you @chrjorgensen ! I'm relieved that it was just the extension messing around.