Required and Repeated fields

[rambleraptor]

• Add support for repeated fields on OpenAPI + proto
• Add support for required fields on proto (they're already implemented on OpenAPI)

[toumorokoshi]

Nice! main concern is in the naming of "repeated".

[toumorokoshi]

I think really this is an array of strings - It'd be nice if we expressed it that way instead of "repeated" - that's really a proto colloquialism and not a term someone would use to describe this data type in any other context. I'd say it's either "Array" or "List".

Json describes things as :

{
  "type": "array",
  "items": {
    "type": "number"
  }
}

Expressing a tuple as

{
  "type": "array",
  "prefixItems": [
    { "type": "number" },
    { "type": "string" },
    { "enum": ["Street", "Avenue", "Boulevard"] },
    { "enum": ["NW", "NE", "SW", "SE"] }
  ]
}

Combining both into the "array" type feels weird to me, so I think a separation between list and tuple would make sense.

I think you would have to do something like:

type: ARRAY
element_type: STRING

WDYT?

[rambleraptor]

Yeah, I'm fine with this. As we move forward with defining objects inline, we'll probably have to change the logic here a bit.

I'm using a oneof to define both a primitive field and an object field. Any ideas to clean that up?

[toumorokoshi]

Oof I see what you're getting at - having that oneof is not great.

What do you think about bool is_array?

I know that's still "repeated" in terms of the schema, but I'd prefer to at least call the concept "is_array" instead.

The other option I can think of is to use oneof with a primitive enum, or an array type:

oneof types { // oneofs aren't even mentioned in textproto, so choose a name that won't collide with anything.
    Type type;
    ArrayType array;
}

Then I guess the declaration might look like:

type: bool 

In the case of a primitive and

array:
    item_type: bool

for an array.

[rambleraptor]

I think I'd like to find something that more comprehensively fits in with having full objects as well.

What about this? It's pretty much the same thing you're suggesting in your oneof, but I'm adding objects as well to show how that would look.

message Property {
    oneof types {
        Type type;
        ObjectType object_type;
        ArrayType array_type;
    }
}

enum Type {
   STRING;
   DOUBLE;
   ....
}

message ObjectType {
    oneof details {
        repeated Property properties;
        string message_name;
    }
}

message ArrayType {
    oneof array_details {
        Type type;
        ObjectType object_type;
    }
}

[rambleraptor]

I'll hold off on these changes until #22 goes in, since that might change some of the internal details.

[toumorokoshi]

is this field still needed?

[rambleraptor]

This particular field is necessary to define arrays of objects. It'll look way cleaner with the new definition we have above.

[toumorokoshi]

I think one field that may no longer be needed, but otherwise LGTM!