rockcarver / frodo-lib

A library to manage ForgeRock Identity Cloud tenants, ForgeOps deployments, and classic deployments.
MIT License
9 stars 18 forks source link

state (storage) reform #36

Closed adam-cyclones closed 1 year ago

adam-cyclones commented 2 years ago

linked to rockcarver/frodo-lib#37

There is a well-intentioned false getter and setter paradigm used in the storage of the application but it just adds complexity

Not just opinion

  1. consider the below file SessionStorage.js

The main problem I can see here is that getters and setters are supposed to promote immutability/encapsulation by controlling what's private and public. This concept is considerably easier to achieve in a language such as Java.

In JavaScript, although now it is possible to do something similar with Symbol we still exposed the property raw which allows direct modification of the object which suggests that the approach is flawed and at some point, raw was exposed to allow direct modification, the final reason for getters and setters is to give some expectation of properties that change and their names, defining them, but In my example bellow I use JS doc to tell the editor what properties are expected, I also generate docs from this, I finally define true immutability using defineProperty

const _sessionStorage = {}

export default {
    session: {
        setItem: (key, value) => _sessionStorage[key] = value,
        getItem: (key) => _sessionStorage[key],
        removeItem: (key) => delete _sessionStorage[key],
        raw: _sessionStorage,
        setUsername: (value) => _sessionStorage["username"] = value,
        getUsername: () => _sessionStorage["username"],
        setPassword: (value) => _sessionStorage["password"] = value,
        getPassword: () => _sessionStorage["password"],
        setTenant: (value) => _sessionStorage["tenant"] = value,
        getTenant: () => _sessionStorage["tenant"],
        setDeploymentType: (value) => _sessionStorage["deploymentType"] = value,
        getDeploymentType: () => _sessionStorage["deploymentType"],
        setRealm: (value) => _sessionStorage["realm"] = value,
        getRealm: () => _sessionStorage["realm"],
        setCookieName: (value) => _sessionStorage["cookieName"] = value,
        getCookieName: () => _sessionStorage["cookieName"],
        setCookieValue: (value) => _sessionStorage["cookieValue"] = value,
        getCookieValue: () => _sessionStorage["cookieValue"],
        setBearerToken: (value) => _sessionStorage["bearerToken"] = value,
        getBearerToken: () => _sessionStorage["bearerToken"],
        setLogApiKey: (value) => _sessionStorage["logApiKey"] = value,
        getLogApiKey: () => _sessionStorage["logApiKey"],
        setLogApiSecret: (value) => _sessionStorage["logApiSecret"] = value,
        getLogApiSecret: () => _sessionStorage["logApiSecret"],
        setAmVersion: (value) => _sessionStorage["amVersion"] = value,
        getAmVersion: () => _sessionStorage["amVersion"],
        setFrodoVersion: (value) => _sessionStorage["frodoVersion"] = value,
        getFrodoVersion: () => _sessionStorage["frodoVersion"],
        setAllowInsecureConnection: (value) => _sessionStorage["insecure"] = value,
        getAllowInsecureConnection: () => _sessionStorage["insecure"],
    }
}

vs

  1. JS doc and plain old JavaScript
    
    /**
    * @typedef {Object} GlobalState
    * @property {boolean} allowInsecureConnection `true` enable HTTP.\
    `{ allowInsecureConnection: false }` disable http and enable HTTPS.
    * @property {string} amVersion which version of ForgeRock Access Management (AM) is in use.
    * @property {string} bearerToken The bearer Token, a predominant type of access token used with OAuth 2.0.
    * @property {string} cookieName After auth you will have a cookie with a name and value, this is the name.
    * @property {string} cookieValue After auth you will have a cookie with a name and value, this is the value.
    * @property {"classic"|"cloud"|"forgeops"} deploymentType choose the context of the target environment
    - `classic`  A classic Access Management-only deployment with custom layout and configuration.
    - `cloud`    A ForgeRock Identity Cloud environment.
    - `forgeops` A ForgeOps CDK or CDM deployment.
    * @property {string} frodoVersion The version of lib-frodo, this cannot be changed.
    * @property {string} host What is the url of the tenant?
    * @property {string} logApiKey You provide a generated logging API key from the tenent.
    * @property {string} logApiSecret You provide a generated logging API secret from the tenent.
    * @property {string} password The password of the admin user.
    * @property {string} realm which realm do the frodo commands affect?
    * @property {string} username The username of the admin user.
    */

/**

Object.defineProperty('frodoVersion', globalState, { value: packageJSON.version, writable: false, configurable: false, });

export default globalState;

vscheuber commented 1 year ago

State has been reformed into an instantiable and fully typed construct. Marking as complete.