chore!: improve decoding of required fields

[EvanHahn]

A required field, confusingly, should probably be marked optional in the protobuf. That way, we can throw errors when decoding. See #252 for more details.

This change:

• Updates .proto files to mark all required fields with our custom required tag
• Updates .proto files to mark most required fields as optional, so we can throw errors when decoding
• Throw errors when decoding if a field is missing

I tested this end-to-end.

Closes #252.

[gmaclennan]

I have a question about enums: without optional then if the field is not present, it will be decoded to unspecified. With optional we kind of have two "unspecified" states of the enum. Because we have already gone to the effort of adding unspecified everywhere, I think maybe we could follow a practice of enum fields never being optional? And require their presence base on unspecified.

[gmaclennan]

I realise there is confusing behaviour here: you could write data with an empty string for a required field and it will type check, and it will not complain when writing to the DB, you will just never see that data record again...

[EvanHahn]

I have a question about enums: without optional then if the field is not present, it will be decoded to unspecified. With optional we kind of have two "unspecified" states of the enum. Because we have already gone to the effort of adding unspecified everywhere, I think maybe we could follow a practice of enum fields never being optional? And require their presence base on unspecified.

Fixed the one violation of this in 931f473. (Manually checked every occurrence of git grep -E "optional [A-Z]" proto/.)

I realise there is confusing behaviour here: you could write data with an empty string for a required field and it will type check, and it will not complain when writing to the DB, you will just never see that data record again...

Good point. Gave all required strings a minLength in 0bd5ab4. (Required a small tweak to the way we do codegen because of a bug in Ajv.)

[EvanHahn]

This causes breakages in @comapeo/core which I'll look at tomorrow.

[gmaclennan]

Did an initial review of this because I'm not sure when I can get back to it later.

I think the barrier for a "required" field at the protobuf level should be a bit higher. I think it should be "the app can't function without this field", because the action we take is fairly "drastic" which is to ignore the record altogether. There are a few fields I have highlighted that I think could be made optional. I think also the config metadata should also maybe be all optional - I feel like in general metadata should be a "optional if you have it" field.

VersionId should have index as required, and decode code should check that. I think we we require it on the JSON side.

There are a few follow up things I think which are not essential for MVP, such as checking docIds are not zero-length, and I think we should also look at distinguishing "write" vs "read" types for the app-side, mainly for enums - you should not be able to write an enum which is "unspecified" or "unrecognized", but the read type shoudl have them.

[EvanHahn]

I think I got this working, but I'm going to test it end-to-end to make sure things still work.

[EvanHahn]

Running into issues getting this to work in @comapeo/core and beyond. I'll look more on Monday.

[EvanHahn]

This PR was getting a little too large for me, so I'm starting to split it into smaller ones:

• chore: getVersionId should throw on invalid input #257
• chore: make CoreOwnership core IDs required #258

[EvanHahn]

Gregor and I paired on this so I'm going to merge.