FirebaseExtended / protobuf-rules-gen

This is an experimental protoc plugin that generates Firebase Rules for Cloud Firestore based on Google's Protocol Buffer format. This allows you to easily validate your data in a platform independent manner.
Apache License 2.0
197 stars 13 forks source link

typed references and subcollections #25

Open ribrdb opened 5 years ago

ribrdb commented 5 years ago

I've started working on a protobuf code generator to create typescript models. One feature I need is to be able to specify the type of a reference or subcollection. I'm thinking something like this:

message Foo {...}
message Bar {
  Foo fooref = 1 [(protofire.options).reference = true];
  map<string, Foo> foocol = 2 [(protofire.options).subcollection = true];
}

This would generate something like:

interface Bar {
  fooref: Reference<Foo>
  foocol: Collection<Foo>

Would this be something you'd be interested in supporting, or does it make more sense for us to have our own rules generator?

rockwotj commented 5 years ago

Sorry can you explain what you mean by the difference between subcollection and reference?

Why is subcollection a map type?

There is always an validation options on a field if you need something application specific.

See: https://github.com/FirebaseExtended/protobuf-rules-gen/blob/master/testdata/test6.proto and https://github.com/FirebaseExtended/protobuf-rules-gen/blob/master/testdata/test6.rules

ribrdb commented 5 years ago

Sorry, let me try to explain this better. Reference fields store a Firestore reference, but also hold type information for the code generator. As far as the rules generator is concerned, this is the same as a string field with the reference option. Sub-collections don't actually represent a field, they should be ignored by the rules generator. They instruct the code generator to create methods for accessing a sub collection. For example if b is a reference to document Bar/abc:

// Get a typed reference to collection Bar/abc/foocol
const bfoos: Collection<Foo> = b.foocol();
// Access a reference property from a snapshot
const snap = await b.get();
const foo?: Reference<Foo> = s.fooref;

And this is the rules I'd want generated:

function isBarMessage(resource) {
  return resource.keys().hasAll([]) &&
          ((!resource.keys().hasAny(['fooref'])) || (resource.fooref is reference)));
}

I don't think the validation option helps. I believe it lets me add to the generate rules, but I need to modify or omit part of what's generated.

ribrdb commented 5 years ago

Also I may use a repeated field instead of a map. A map is closer to how Collections work, but since it's going to be handled specially by the code generator I don't think it matters.

rockwotj commented 5 years ago

Oh I see, so you want is to be able to ignore fields from rules?

rockwotj commented 5 years ago

Does the option extra_properties help here? Or no because you need the proto interface to have the property?

ribrdb commented 5 years ago

I don't think extra_properties helps, but an option to ignore a field would.

ribrdb commented 5 years ago

I came up with a workaround here, although it seems pretty hacky. I wrote another code generator that wraps //firebase_rules_generator:firebase_rules_options and //firebase_rules_generator:generator. It modifies the CodeGeneratorRequest to convert from (protofire.field) options to (google.firebase.rules.firebase_rules_field) options, then runs the RulesGenerator.

I had a couple of ideas for a cleaner way to implement it:

  1. Add some virtual methods to RulesGenerator, that I can override in a subclass. Right now I would need something like ShouldIgnoreField() and IsReferenceField()
  2. Directly support the protofire options in RulesGenerator. Right now it's pretty simple:
    
    syntax = "proto2";
    package protofire;

import "google/protobuf/descriptor.proto";

message ProtofireFieldOptions { // protobuf-rules-gen should ignore fields with this option optional bool collection = 1; // This is pretty much the same as (google.firebase.rules.firebase_rules_field).reference_type // except that it can apply to a string or a message field. optional bool ref = 2; }

extend google.protobuf.FieldOptions { optional ProtofireFieldOptions field = 1044; }



What do you think? Would you be ok with any of these solutions? Or do you have any other ideas?
rockwotj commented 5 years ago

I think for ref we can extend the current reference type (that is a message) to work for message types as well.

As for collection I think if we add an option to ignore things that's fine - I don't think I want to support these exact options as they are.

I'm also totally fine to accept some PRs to refactor the rules generator to have some virtual methods and be pluggable. I sadly don't have time to work on that at the moment right now 😞