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: Bigtable Authorized Views - Allow checkAndMutate and ReadModifyWriteRow calls on Authorized Views #1464

Open
wants to merge 43 commits into
base: bigtable-authorized-views
Choose a base branch
from

Conversation

danieljbruce
Copy link
Contributor

@danieljbruce danieljbruce commented Aug 28, 2024

Summary:

This PR introduces the architecture required to service checkAndMutate and readModifyWriteRow requests for Authorized views.

Background:

this other PR has already been merged towards the effort of supporting data API calls for Authorized Views.

This current PR demonstrates the shape of the architecture that is required for Authorized View requests that correspond to the checkAndMutate and readModifyWriteRow calls currently serviced by calling the Row class methods. There may be clerical mistakes, but those will be flushed out by writing unit tests and integration tests.

Bigtable - RMWR and CAM support drawio

Changes:

Row Data Utils:

  • Code from increment, filter and createRules in the Row class is moved into new methods in row-data-utils. To support authorized view calls this has to be done. You see, increment, filter and createRules on the Row class make use of a private data property in that Row class. For this reason, to support increment, filter and createRules for authorized views we cannot simply create increment, filter and createRules methods on the TabularApiSurface class because then data would have to be passed into them and we do not want data to be part of the method signature. Instead, we move increment, filter and createRules functionality from Row to methods in a shared row-data-utils class. These methods in the row-data-utils class will now make requests for both tables and authorized views.

Authorized View:

  • increment, filter and createRules methods are added to the new AuthorizedView class for making checkAndMutate and readModifyWriteRow calls for an authorized view. These methods should not be exposed on TabularApiSurface because then they will be exposed on Table, but this functionality is already available for tables on the increment, filter and createRules methods on Row.
  • The createRules, filter and increment methods on the AuthorizedView class will have the same signature as the methods with the same name on the Row class except they must also be provided with a row id that the request will be made for.
  • The AuthorizedView class will store row data for authorized views just like the Row class has a data property that stores data for that row. This rowData variable in AuthorizedView stores data for each row it makes a request on behalf of.

Row:

  • Existing method bodies are moved to RowDataUtils and replaced with one line of code that delegates the call.

Table:

  • A view method is added which will be used for creating authorized views
  • A constructor is added matching the constructor that was in Table before. This is needed now that the TabularApiSurface constructor is protected.

TabularApiSurface:

  • The constructor is now protected since the class isn't meant to be instantiated directly
  • A viewName member is added and will be undefined for tables, but provided for authorized views

test/Row.ts

  • Since the code has been moved then different files need to be mocked in proxyquire and sinon so these mocks are adjusted.

Next Steps:

  1. Write some tests that use the new increment, createRules and filter methods now available on AuthorizedView. Make sure that these methods pass the proper parameters along and that they target an authorized view.
  2. Write a PR for supporting authorized views calls for all methods of the TabularApiSurface class.

danieljbruce and others added 27 commits August 20, 2024 10:02
…apis/nodejs-bigtable into bigtable-authorized-views-move-row-methods

# Conflicts:
#	src/row.ts
#	src/table.ts
#	src/tabular-api-service.ts
#	src/utils/table.ts
#	test/table.ts
@danieljbruce danieljbruce requested review from a team as code owners August 28, 2024 19:05
@product-auto-label product-auto-label bot added the size: l Pull request size is large. label Aug 28, 2024
@product-auto-label product-auto-label bot added the api: bigtable Issues related to the googleapis/nodejs-bigtable API. label Aug 28, 2024
@danieljbruce danieljbruce changed the title feat: Expose functionality from Row to be available on the TabularApiSurface too feat: Bigtable Authorized Views - Expose functionality from Row to be available on the TabularApiSurface too Aug 28, 2024
@danieljbruce danieljbruce changed the title feat: Bigtable Authorized Views - Expose functionality from Row to be available on the TabularApiSurface too feat: Bigtable Authorized Views - Expose functionality from Row to be available on AuthorizedViews too Aug 29, 2024
@danieljbruce danieljbruce changed the title feat: Bigtable Authorized Views - Expose functionality from Row to be available on AuthorizedViews too feat: Bigtable Authorized Views - Allow checkAndMutate and ReadModifyWriteRow calls on Authorized Views Aug 29, 2024
Copy link

@bhshkh bhshkh left a comment

Choose a reason for hiding this comment

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

Well organized code. Thanks for adding the detailed description regarding the changes in the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigtable Issues related to the googleapis/nodejs-bigtable API. size: l Pull request size is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants