Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Return service from findCredentials invocation #290

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rcjpisani
Copy link

@rcjpisani rcjpisani commented Jul 16, 2020

Issue or RFC Endorsed by Atom's Maintainers

#144

Description of the Change

These changes add the service property to the result of the findCredentials method, introduced by #85.

Alternate Designs

Other alternative solutions would be the use of std::array instead of std::tuple for the definition of Credentials in credentials.h seeing as the values used up until this point are homogenous. I decided to stick with Tuple since it's a generalization of the previously used std::pair. I'm open to tweaking the code as needed, pending review, to highlight any potential issues. C++ is not a language I'm well versed in.

Possible Drawbacks

Possible side effects are a slight increase in memory usage in order to support the return of the service property.

Verification Process

  • The program successfully compiled and test suite passed on both Linux (Linux 5.7.8-arch1-1 #1 SMP PREEMPT Thu, 09 Jul 2020 16:34:01 +0000 x86_64 GNU/Linux) and Mac (Darwin 19.5.0 Darwin Kernel Version 19.5.0: Tue May 26 20:41:44 PDT 2020; root:xnu-6153.121.2~2/RELEASE_X86_64 x86_64). Windows testing pending as I do not have access to a working Windows machine.

Result of running test suite on Linux:

$ npm test

> [email protected] test /home/robert/Dev/node-keytar
> npm run lint && npm build . && mocha --require babel-core/register spec/


> [email protected] lint /home/robert/Dev/node-keytar
> npm run cpplint


> [email protected] cpplint /home/robert/Dev/node-keytar
> node-cpplint --filters legal-copyright,build-include,build-namespaces src/*.cc


> [email protected] install /home/robert/Dev/node-keytar
> prebuild-install || node-gyp rebuild

make: Entering directory '/home/robert/Dev/node-keytar/build'
  CXX(target) Release/obj.target/keytar/src/async.o
  CXX(target) Release/obj.target/keytar/src/main.o
  CXX(target) Release/obj.target/keytar/src/keytar_posix.o
  SOLINK_MODULE(target) Release/obj.target/keytar.node
  COPY Release/keytar.node
make: Leaving directory '/home/robert/Dev/node-keytar/build'


  keytar
    setPassword/getPassword(service, account)
      ✓ sets and yields the password for the service and account
      ✓ yields null when the password was not found
      error handling
        setPassword
          ✓ handles when an object is provided for service
          ✓ handles when an object is provided for username
          ✓ handles when an object is provided for password
        getPassword
          ✓ handles when an object is provided for service
          ✓ handles when an object is provided for username
      Unicode support
        ✓ handles unicode strings everywhere
    deletePassword(service, account)
      ✓ yields true when the password was deleted
      ✓ yields false when the password didn't exist
      error handling
        ✓ handles when an object is provided for service
        ✓ handles when an object is provided for username
    findPassword(service)
      ✓ yields a password for the service
      ✓ yields null when no password can be found
      ✓ handles when an object is provided for service
    findCredentials(service)
      ✓ yields an array of the credentials
      ✓ returns an empty array when no credentials are found
      ✓ handles when an object is provided for service
      Unicode support
        ✓ handles unicode strings everywhere


  19 passing (150ms)

Result of running test suite on Mac:

$ npm test

> [email protected] test /Users/robert.pisani/Dev/node-keytar
> npm run lint && npm build . && mocha --require babel-core/register spec/


> [email protected] lint /Users/robert.pisani/Dev/node-keytar
> npm run cpplint


> [email protected] cpplint /Users/robert.pisani/Dev/node-keytar
> node-cpplint --filters legal-copyright,build-include,build-namespaces src/*.cc

 ✓ src/async.cc
 ✓ src/keytar_mac.cc
 ✓ src/keytar_posix.cc
 ✓ src/keytar_win.cc
 ✓ src/main.cc

> [email protected] install /Users/robert.pisani/Dev/node-keytar
> prebuild-install || node-gyp rebuild

  CXX(target) Release/obj.target/keytar/src/async.o
  CXX(target) Release/obj.target/keytar/src/main.o
  CXX(target) Release/obj.target/keytar/src/keytar_mac.o
  SOLINK_MODULE(target) Release/keytar.node


  keytar
    setPassword/getPassword(service, account)
      ✓ sets and yields the password for the service and account (53ms)
      ✓ yields null when the password was not found
      error handling
        setPassword
          ✓ handles when an object is provided for service
          ✓ handles when an object is provided for username
          ✓ handles when an object is provided for password
        getPassword
          ✓ handles when an object is provided for service
          ✓ handles when an object is provided for username
      Unicode support
        ✓ handles unicode strings everywhere
    deletePassword(service, account)
      ✓ yields true when the password was deleted
      ✓ yields false when the password didn't exist
      error handling
        ✓ handles when an object is provided for service
        ✓ handles when an object is provided for username
    findPassword(service)
      ✓ yields a password for the service (47ms)
      ✓ yields null when no password can be found
      ✓ handles when an object is provided for service
    findCredentials(service)
      ✓ yields an array of the credentials (65ms)
      ✓ returns an empty array when no credentials are found
      ✓ handles when an object is provided for service
      Unicode support
        ✓ handles unicode strings everywhere


  19 passing (367ms)

Release Notes

  • Return service property from findCredentials method

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant