Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core-api): add createIsJwsGeneralTypeGuard, createAjvTypeGuard<T> #3494

Conversation

petermetz
Copy link
Contributor

  1. createAjvTypeGuard() is the lower level utility which can be used
    to construct the more convenient, higher level type predicates/type guards
    such as createIsJwsGeneralTypeGuard() which uses createAjvTypeGuard
    under the hood.
  2. This commit is also meant to be establishing a larger, more generic
    pattern of us being able to create type guards out of the Open API specs
    in a convenient way instead of having to write the validation code by hand.

An example usage of the new createAjvTypeGuard() utility is the
createIsJwsGeneralTypeGuard() function itself.

An example usage of the new createIsJwsGeneralTypeGuard() can be found
in packages/cactus-plugin-consortium-manual/src/main/typescript/plugin-consortium-manual.ts

The code documentation contains examples as well for maximum discoverabilty
and I'll also include it here:

import { JWSGeneral } from "@hyperledger/cactus-core-api";
import { createIsJwsGeneralTypeGuard } from "@hyperledger/cactus-core-api";

export class PluginConsortiumManual {
  private readonly isJwsGeneral: (x: unknown) => x is JWSGeneral;

  constructor() {
    // Creating the type-guard function is relatively costly due to the Ajv schema
    // compilation that needs to happen as part of it so it is good practice to
    // cache the type-guard function as much as possible, for examle by adding it
    // as a class member on a long-lived object such as a plugin instance which is
    // expected to match the life-cycle of the API server NodeJS process itself.
    // The specific anti-pattern would be to create a new type-guard function
    // for each request received by a plugin as this would affect performance
    // negatively.
    this.isJwsGeneral = createIsJwsGeneralTypeGuard();
  }

  public async getNodeJws(): Promise<JWSGeneral> {
    // rest of the implementation that produces a JWS ...
    const jws = await joseGeneralSign.sign();

    if (!this.isJwsGeneral(jws)) {
      throw new TypeError("Jose GeneralSign.sign() gave non-JWSGeneral type");
    }
    return jws;
  }
}

Relevant discussion took place here:
#3471 (comment)

Signed-off-by: Peter Somogyvari peter.somogyvari@accenture.com

Pull Request Requirements

  • Rebased onto upstream/main branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.
  • Have git sign off at the end of commit message to avoid being marked red. You can add -s flag when using git commit command. You may refer to this link for more information.
  • Follow the Commit Linting specification. You may refer to this link for more information.

Character Limit

  • Pull Request Title and Commit Subject must not exceed 72 characters (including spaces and special characters).
  • Commit Message per line must not exceed 80 characters (including spaces and special characters).

A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.

@petermetz
Copy link
Contributor Author

cc: @RafaelAPB @eduv09 Please take a look at how the casting to JWSGeneral can be eliminated once this PR gets merged (look at the diff for the file packages/cactus-plugin-consortium-manual/src/main/typescript/plugin-consortium-manual.ts in this PR to see what I mean exactly)

@petermetz petermetz force-pushed the feat-core-api-openapi-schema-type-guards branch from 7a662d8 to 535f087 Compare August 27, 2024 06:48
Copy link
Contributor

@jagpreetsinghsasan jagpreetsinghsasan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

1. createAjvTypeGuard<T>() is the lower level utility which can be used
to construct the more convenient, higher level type predicates/type guards
such as createIsJwsGeneralTypeGuard() which uses createAjvTypeGuard<JwsGeneral>
under the hood.
2. This commit is also meant to be establishing a larger, more generic
pattern of us being able to create type guards out of the Open API specs
in a convenient way instead of having to write the validation code by hand.

An example usage of the new createAjvTypeGuard<T>() utility is the
createIsJwsGeneralTypeGuard() function itself.

An example usage of the new createIsJwsGeneralTypeGuard() can be found
in packages/cactus-plugin-consortium-manual/src/main/typescript/plugin-consortium-manual.ts

The code documentation contains examples as well for maximum discoverabilty
and I'll also include it here:

```typescript
import { JWSGeneral } from "@hyperledger/cactus-core-api";
import { createIsJwsGeneralTypeGuard } from "@hyperledger/cactus-core-api";

export class PluginConsortiumManual {
  private readonly isJwsGeneral: (x: unknown) => x is JWSGeneral;

  constructor() {
    // Creating the type-guard function is relatively costly due to the Ajv schema
    // compilation that needs to happen as part of it so it is good practice to
    // cache the type-guard function as much as possible, for examle by adding it
    // as a class member on a long-lived object such as a plugin instance which is
    // expected to match the life-cycle of the API server NodeJS process itself.
    // The specific anti-pattern would be to create a new type-guard function
    // for each request received by a plugin as this would affect performance
    // negatively.
    this.isJwsGeneral = createIsJwsGeneralTypeGuard();
  }

  public async getNodeJws(): Promise<JWSGeneral> {
    // rest of the implementation that produces a JWS ...
    const jws = await joseGeneralSign.sign();

    if (!this.isJwsGeneral(jws)) {
      throw new TypeError("Jose GeneralSign.sign() gave non-JWSGeneral type");
    }
    return jws;
  }
}
```

Relevant discussion took place here:
hyperledger-cacti#3471 (comment)

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
@petermetz petermetz force-pushed the feat-core-api-openapi-schema-type-guards branch from 535f087 to 56b442e Compare August 30, 2024 17:18
Copy link
Contributor

@RafaelAPB RafaelAPB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cast LGTM, thanks @petermetz.

@petermetz petermetz merged commit 957da7c into hyperledger-cacti:main Sep 2, 2024
143 of 146 checks passed
@petermetz petermetz deleted the feat-core-api-openapi-schema-type-guards branch September 4, 2024 23:10
@petermetz
Copy link
Contributor Author

The cast LGTM, thanks @petermetz.

@RafaelAPB You got it! Any other casting/error handling related stuff you need help with just let me know! It is important for production deployments to have these nailed down and I want to help as much as I can!

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

Successfully merging this pull request may close these issues.

3 participants