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

[PROJECT] New proto-backed Firestore structure #1758

Closed
23 of 24 tasks
jcqli opened this issue Apr 29, 2024 · 4 comments
Closed
23 of 24 tasks

[PROJECT] New proto-backed Firestore structure #1758

jcqli opened this issue Apr 29, 2024 · 4 comments
Assignees
Labels
type: code health Improvements to readability or robustness of codebase

Comments

@jcqli
Copy link

jcqli commented Apr 29, 2024

Parent issue tracking the work needed to introduce protos as our db and Firestore schema.

Additional tasks related to this migration:

@jcqli jcqli added web Angular implementation of Web UI type: code health Improvements to readability or robustness of codebase labels Apr 29, 2024
@gino-m gino-m changed the title [Proposal / Proof of Concept] Protocol Buffers as a Firestore schema Enforce structure of data in Firestore at compile-time May 13, 2024
@gino-m gino-m changed the title Enforce structure of data in Firestore at compile-time [Proposal / Proof of Concept] Protocol Buffers as a Firestore schema May 13, 2024
@gino-m gino-m changed the title [Proposal / Proof of Concept] Protocol Buffers as a Firestore schema Protocol Buffers as a Firestore schema May 13, 2024
@jcqli jcqli changed the title Protocol Buffers as a Firestore schema Protocol Buffers as a Firestore schema on platform May 13, 2024
@jcqli jcqli changed the title Protocol Buffers as a Firestore schema on platform Use Protocol Buffers as a Firestore schema on platform May 13, 2024
@gino-m
Copy link
Collaborator

gino-m commented May 16, 2024

Did some research on various protoc plugins for generating js+ts code from protodef. We need something which makes metadata accessible (esp. field numbers) in order to implement the representation described in #2455. Here are my findings:

protobufjs ✅

The most popular lib I found (16M weekly downloads), originally designed by Google. It can generate js and ts, but to get what we want you need to call it three times - once to generate the js, once to generate to ts bindings, and once to generate JSON with pb metadata.

npm i -D protobufjs protobufjs-cli
npx pbjs -t static-module ../proto/survey.proto -o dist/survey.js
npx pbts dist/survey.js -o dist/survey.d.ts
npx pbjs -t json ../proto/survey.proto -o dist/survey.proto.json

We could load the JSON at runtime to pick out field numbers we need.

Alternatives considered

@protobuf-ts/plugin

146k downloads/week. Generates pure ts code in a single command, and includes field numbers in the message metadata. But given this is a less "standard", less used library, prefer using protobufjs.

ts-proto

OTOO 300k weekly downloads, and doesn't encode the field numbers anywhere we can use at runtime, so we can exclude this one for now.

protoc-gen-ts

It seems the protoc packaged with the one didn't support my M1 Mac.

npm i -D @protobuf-ts/plugin
npx protoc --ts_out dist --proto_path ../proto survey.proto loi.proto submission.proto

It doesn't produce or consume JSON, but it's fairly easy to go from ts object<>JSON anyway, plus we'll prob want more control over the formats.

grpc_tools_node_protoc_ts

Only 100k downloads per week, but uses the standard google grpc-tools. Doesn't seem to embed the set of valid field numbers, though individual fields can be fetched by field number:

npm i -D grpc-tools grpc_tools_node_protoc_ts
npx grpc_tools_node_protoc --js_out=import_style=commonjs,binary:./dist --grpc_out=grpc_js:./dist --plugin=protoc-gen-grpc=./node_modules/.bin/grpc_tools_node_protoc_plugin -I ../proto survey.proto
npx protoc --plugin=protoc-gen-ts=./node_modules/.bin/protoc-gen-ts --ts_out=grpc_js:./dist -I ../proto survey.proto

@gino-m
Copy link
Collaborator

gino-m commented May 16, 2024

Also, carrying forward proposal from google/ground-android#2455:

  • Create lightweight converters in Kotlin and TypeScript which take a Map<Long, Any> and produce protobufs and vice-versa, where keys in Firestore correspond to a protobuf field number, and values primitive types supported by proto.

Example survey data before:

{
  "title": "My survey",
  "description": "An example survey",
   //...
}

Example survey data after:

{
  1: "My survey",
  2: "An example survey",
  ...
}

This has several advantages:

  • Generated code ensures a consistent schema across web, Android, and Cloud Functions.
  • Using field IDs instead of names allows field names can be changes without breaking clients.
  • Storaging values as Firestore values rather than binary allows Firestore query filters to work without duplicating values in "index" fields, and it allows data to be viewed and debugged using standard tools.
  • The implementation is simple, easy to write and debug.
  • Using field numbers instead of names allows this sol'n to work with Android "lite" protobufs in which name bindings aren't available at runtime.
  • Writing directly to generated code

This has a few disadvantages as well:

  • Maintaining backwards compatibility with existing test surveys is very difficult. We would probably want to zap the dbs and start from scratch.
  • Indirection in key names in Firestore; would need to check protobuf def to understand which field is which.

@jcqli
Copy link
Author

jcqli commented Aug 12, 2024

Filed #1966 to track cleanup of old code @gino-m shall we mark this as closed?

@gino-m
Copy link
Collaborator

gino-m commented Aug 12, 2024

Yes, I think we can!

@jcqli jcqli closed this as completed Aug 12, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Ground Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: code health Improvements to readability or robustness of codebase
Projects
Status: Done
Development

No branches or pull requests

3 participants