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

Proposal: Builder Pattern Alternatives #32

Closed
KwilLuke opened this issue Oct 4, 2023 · 4 comments · Fixed by #37
Closed

Proposal: Builder Pattern Alternatives #32

KwilLuke opened this issue Oct 4, 2023 · 4 comments · Fixed by #37
Assignees
Labels
enhancement New feature or request

Comments

@KwilLuke
Copy link
Contributor

KwilLuke commented Oct 4, 2023

Below is a proposed approach for how the public APIs in Kwil-JS can look to move away from the Builder Patterns and more closely resemble what is available in packages like EtherJS and Viem.

1. Create KwilSigner Class

Create a Kwil Signer Class that is used to signing transactions and call requests:

const mySigner = new KwilSigner(
     publicKey: string, 
     signer: EthersSigner | (msg: Uint8Array) => Uint8Array, 
     signatureType?: string
)

signatureType is required to be specified if it does not receive an EtherJS signer.

2. Create four new methods on the main kwil class

Currently, the entry point for KwilJS is with the WebKwil or NodeKwil class:

e.g.

const kwil = new WebKwil({
   kwilProvider: "https://provider.kwil.com",
   chainId: "your_chain_id",
});

Action Operations - kwil.call() and kwil.execute()

For action-related operations (state-changing and view actions), there will be two methods:

  • kwil.call(actionPayload: ActionInterface, signer?: KwilSigner) - Executes view actions. Signer only required if the action has must_sign.
  • kwil.execute(actionPayload: ActionInterface, signer: KwilSigner) - Executes mutative actions. Signer is required.

TheActionInterface for both methods will look like this:

interface ActionInterface {
     dbid: string,
     action: string,
     inputs?: Array<objects> | Array<ActionInput>,
     description?: string
}

Database Operations - kwil.deploy() and kwil.drop()

For deploying and dropping databases, there will be two methods:

  • kwil.deploy(deployPayload: DeployInterface, signer: KwilSigner) - Used to deploy a database.
  • kwil.drop(dropPayload: DropInterface, signer: KwilSigner) - Used to drop a database.

With the following respective interfaces:

interface DeployInterface {
     schema: object<CompiledKuneiform>,
     description?: string
}

interface DropInterface {
     dbid: string,
     description?: string
}
@KwilLuke KwilLuke added the enhancement New feature or request label Oct 4, 2023
@KwilLuke KwilLuke self-assigned this Oct 4, 2023
@Yaiba
Copy link

Yaiba commented Oct 4, 2023

I feel the awkwardness to create the KwilSigner comes from:

  • need public key
  • need to allow developer to use existing Ethersjs signer

Overall it looks good to me, we can try the DX constructing KwilSigner like proposed, if we need to change, it will still works with the rest of the methods.

KwilMachine pushed a commit that referenced this issue Oct 4, 2023
Added KwilSigner class to be used with the new, builder pattern alternative structure.

re #32
KwilMachine pushed a commit that referenced this issue Oct 5, 2023
….drop()

Add the `kwil` `.call()`, `.execute()`, `.deploy()`, and `.drop()` methods, providing an alternative
interface for executing database operations from the original builders.

re #32
@KwilLuke
Copy link
Contributor Author

KwilLuke commented Oct 5, 2023

@Yaiba,

I have completed a first implementation of what I described in this issue (in commit aca229d).

Here is what the developer experience could look like end to end. In my opinion, the public key thing is quite awkward (when using ethereum / etherjs), but I can't find a way around it. Any thoughts?

const kwil = new NodeKwil({
    kwilProvider: "http://localhost:8080"
})

const provider = new JsonRpcProvider(process.env.ETH_PROVIDER)
const wallet = new Wallet(process.env.PRIVATE_KEY as string, provider)

async function buildSigner() {
    const pk = await Utils.recoverSecp256k1PubKey(wallet);
    return new KwilSigner(pk, wallet);
}

async function main() {
    const signer = await buildSigner();

    // actions
    const actionBody: ActionBody = {
        dbid: 'xd924382720df474c6bb62d26da9aeb10add2ad2835c0b7e4a6336ad8',
        action: 'add_post',
        inputs: [{
            "$id": 69,
            "$user": "Luke",
            "$title": "Test Post",
            "$body": "This is a test post"
        }],
        description: "Add a post"
    }

    // execute
    await kwil.execute(actionBody, signer);

    //call with signer
    await kwil.call(actionBody, signer);

    //call without signer
    await kwil.call(actionBody);

    // deploy database
    const deployBody: DeployBody = {
        schema: compiledKf,
        description: "My first database"
    }

    // deploy
    await kwil.deploy(deployBody, signer);
    
    // drop database
    const dropBody: DropBody = {
        dbid: 'abc123',
        description: "Drop this database"
    }

    // drop
    await kwil.drop(dropBody, signer);
}

cc @brennanjl

@Yaiba
Copy link

Yaiba commented Oct 5, 2023

Can const kwil = new NodeKwil({ kwilProvider: "http://localhost:8080" })
becomes
const kwil = new NodeKwil("http://localhost:8080") ? Feels more direct this way.

@KwilLuke
Copy link
Contributor Author

KwilLuke commented Oct 5, 2023

As of now, no, because there are other configs that can be passed there (all optional).

e.g.

const kwil = new NodeKwil({
     kwilProvider: 'http://localhost:8080',
     timeout: 10000,
     logging: true
});

KwilMachine pushed a commit that referenced this issue Oct 19, 2023
Added KwilSigner class to be used with the new, builder pattern alternative structure.

re #32
KwilMachine pushed a commit that referenced this issue Oct 19, 2023
….drop()

Add the `kwil` `.call()`, `.execute()`, `.deploy()`, and `.drop()` methods, providing an alternative
interface for executing database operations from the original builders.

re #32
KwilMachine pushed a commit that referenced this issue Oct 20, 2023
updated README.md and MIGRATING.md to highlight the new features described in #32 (`kwil.execute()`,
`kwil.call()`, `kwil.deploy()`, and kwil.drop()`).

re #32
KwilMachine pushed a commit to kwilteam/docs that referenced this issue Oct 20, 2023
Add documentation (Kwil-JS and Hello World Tutorial) for the new builder pattern alternatives described here: kwilteam/kwil-js#32
KwilLuke added a commit that referenced this issue Oct 26, 2023
* feat: added KwilSigner class

Added KwilSigner class to be used with the new, builder pattern alternative structure.

re #32

* feat: add the methods to kwil class: .call(), .execute(), .deploy(), .drop()

Add the `kwil` `.call()`, `.execute()`, `.deploy()`, and `.drop()` methods, providing an alternative
interface for executing database operations from the original builders.

re #32

* test(tests/unittests): fix unit tests for api_client and builders

* test(tests): clean up all existing unit tests

* refactor(builder, client, core): add TSDOC comments, type safety

added detailed TSDOC comments and in-line explanations for every builder, and stronger type safety
for internal and external apis

* feat: added KwilSigner class

Added KwilSigner class to be used with the new, builder pattern alternative structure.

re #32

* feat: add the methods to kwil class: .call(), .execute(), .deploy(), .drop()

Add the `kwil` `.call()`, `.execute()`, `.deploy()`, and `.drop()` methods, providing an alternative
interface for executing database operations from the original builders.

re #32

* test(tests/unittests): fix unit tests for api_client and builders

* test(tests): clean up all existing unit tests

* refactor(builder, client, core): add TSDOC comments, type safety

added detailed TSDOC comments and in-line explanations for every builder, and stronger type safety
for internal and external apis

* refactor: improve bytes type safety on payloads

improved the typesafety on the Transaction and Message classes to allow developer to specify which
bytes type should be used (Uint8Array or base64), making it easier to know when Uint8 is being used
(building the payload) versus when base64 is being used (sending over GRPC).

* docs(readme): update readme for new execute and call (re #32)

updated README.md and MIGRATING.md to highlight the new features described in #32 (`kwil.execute()`,
`kwil.call()`, `kwil.deploy()`, and kwil.drop()`).

re #32

* docs(readme): minor typos and docs changes
KwilMachine pushed a commit that referenced this issue Oct 30, 2023
* add deprecation notice for the `kwil.actionBuilder()`, `kwil.dbBuilder()`, `kwil.dropDbBuilder()`
and `kwil.broadcast()` in favor of using `kwil.execute()`, `kwil.deploy()`, `kwil.drop()`, and
`kwil.call()`.

* Cleaned up the Utils namespace to inherit class types.

BREAKING CHANGE: The `kwil.actionBuilder()`, `kwil.dbBuilder()`, `kwil.dropDbBuilder()` and
`kwil.broadcast()` are deprecated in favor of using `kwil.execute()`, `kwil.deploy()`,
`kwil.drop()`, and `kwil.call()`. The deprecated methods will be removed in Q1 2024.

re #32
KwilLuke added a commit that referenced this issue Oct 30, 2023
* add deprecation notice for the `kwil.actionBuilder()`, `kwil.dbBuilder()`, `kwil.dropDbBuilder()`
and `kwil.broadcast()` in favor of using `kwil.execute()`, `kwil.deploy()`, `kwil.drop()`, and
`kwil.call()`.

* Cleaned up the Utils namespace to inherit class types.

BREAKING CHANGE: The `kwil.actionBuilder()`, `kwil.dbBuilder()`, `kwil.dropDbBuilder()` and
`kwil.broadcast()` are deprecated in favor of using `kwil.execute()`, `kwil.deploy()`,
`kwil.drop()`, and `kwil.call()`. The deprecated methods will be removed in Q1 2024.

re #32
KwilLuke added a commit to kwilteam/docs that referenced this issue Oct 30, 2023
* update kwil-js for builder pattern alternatives

Add documentation (Kwil-JS and Hello World Tutorial) for the new builder pattern alternatives described here: kwilteam/kwil-js#32

* implement brennan comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants