feat(map): Extend Type support

[jonathan-casey]

Changes

This PR extends the current Type support for MapDeclaration Serialisation & Deserialisation to include:

Key Type Support

• Primitives: String, DateTime
• Scalar String, DateTime

Value Type Support

• Primitives: String, Integer, Long, Double, Boolean
• Scalar : String, Integer, Long, Double, Boolean
• Concepts: Identified & Non Identified

Sample

Given the ModelFile:

namespace org.acme.sample

concept Concepts {
    o Appointment appointment optional
    o Rolodex rolodex optional
    o Points points optional
}

scalar Time extends DateTime

concept Person identified by name {
    o String name
}

map Appointment {
    o Time
    o String
}

map Rolodex {
    o String
    o Person
}

map Points {
    o String
    o Long
}

The Serialiser will generate & validate against the following format

<Scalar, String>

{
  $class: 'org.acme.sample.Concepts',
  appointment: {
    '$class': 'org.acme.sample.Appointment',
    '2023-11-28T01:02:03Z': 'BobBirthday',
    '2024-10-28T01:02:03Z': 'AliceAnniversary'
  }
}

<String, Concept>

{
  $class: 'org.acme.sample.Concepts',
  rolodex: {
    $class: 'org.acme.sample.Rolodex',
    'Dublin': {"$class":"org.acme.sample.Person","name":"Bob"},
    'London': {"$class":"org.acme.sample.Person","name":"Alice"}
  }
}

<String, Long>

{
  $class: 'org.acme.sample.Concepts',
  points: {
    $class: 'org.acme.sample.Points',
    Bob: -398741129664271,
    Alice: 8999999125356546
  }
}

AST

Below is a sample of a MapDeclaration entry in the AST:

{
  $class: '[email protected]',
  name: 'MapPermutation1',
  key: {
    $class: '[email protected]'
  },
  value: {
   $class: '[email protected]'
  }
}

In the case of a Key or Value which is an Object, a TypeIdentifier is now present

{
  $class: '[email protected]',
  name: 'MapPermutation1',
  key: {
    $class: '[email protected]',
  },
  value: {
    $class: '[email protected]'
    type: {
        $class: '[email protected]',
        name: 'Person'
    }
  }
}

$Class Identifiers

Key

[email protected]
[email protected]
[email protected]

Value

[email protected]
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]

[DS-AdamMilazzo]

I'd argue that we shouldn't have a $class attribute within the map values. In your string/string map example, say...

{
  $class: 'org.acme.sample.Concepts',
  appointment: {
    '$class': 'org.acme.sample.Appointment',
    '2023-11-28T01:02:03Z': 'BobBirthday',
    '2024-10-28T01:02:03Z': 'AliceAnniversary'
  }
}

... what values could the "$class" key have? It seems like "org.acme.sample.Appointment" (the declared field type) is the only legal value.

[mttrbrts]

I'd argue that we shouldn't have a $class attribute within the map values.
... what values could the "$class" key have? It seems like "org.acme.sample.Appointment" (the declared field type) is the only legal value.

I'm thinking about assignment scenarios. Say we have client code with two map instances of type Map<String, String> that represent a Dictionary and Phonebook respectively.

Without a discriminator in the instance, the client application could assign a Phonebook to a Dictionary (and visa-versa) and the parent concept would validate. Type conversion like this should be explicit, IMO.

Now, perhaps this scenario is less important where we have a strongly typed language with class definitions that correspond to the Concept declarations, but I believe that it's important that we're defensive in the design. The small optimisation omitting $class gives us doesn't justify the loss of precision that we'd get in some scenarios.

[DS-AdamMilazzo]

Without a discriminator in the instance, the client application could assign a Phonebook to a Dictionary (and visa-versa) and the parent concept would validate. Type conversion like this should be explicit, IMO.

I think the same is true for all scalar types, both implicit and explicit. For example, you can assign any string field value to any other string field regardless of the validators applied to it. I see maps as values, like scalars are. As argued previously, their semantics make the most sense when treated analogously to scalar values. Now perhaps you'd say that's a problem with scalars that we shouldn't propagate to maps, but I'm not sure it's a problem. In both cases it could be handled via stronger typing, as you mention, but that might be a medicine worse than the disease if there were wrappers for every scalar and map type.

The small optimisation omitting $class gives us doesn't justify the loss of precision that we'd get in some scenarios.

I don't see it as an optimization but as a matter of consistency and convenience. "$class" would have to be treated specially in all maps because it's the one key that is metadata rather than data. People reading map values would have to specifically ignore it, and deserializing types like the Appointment map above into Dictionary<DateTime, string> wouldn't work easily because "$class" is not a valid DateTime, so some kind of custom serializer and deserializer would need to be used. I think it makes a bunch of things more onerous than they need to be. There's also the admittedly small matter that people wouldn't be able to use a key called "$class" because it's interpreted differently from the other keys.

[mttrbrts]

I see maps as values, like scalars are.

I see your perspective here. The (perhaps implicit) design here is that Maps are not primitive values, but rather are compound values. The design mirrors what we do for Concepts and other class declarations already.

some kind of custom serializer and deserializer would need to be used

We already implement our own (de)serialization logic for other reasons.

I support merging this PR as-is, but I would welcome a debate about the assignment of scalars, or reserved fields in class declarations. Please open issues as necessary.

[DS-AdamMilazzo]

I see your perspective here. The (perhaps implicit) design here is that Maps are not primitive values, but rather are compound values. The design mirrors what we do for Concepts and other class declarations already.

Yes, they're compound, but so are arrays and I think maps are closer to an array in that sense than a concept. They can both contain concepts, but they don't really support polymorphism in and of themselves. (The concepts inside them support polymorphism as normal, of course.)

We already implement our own (de)serialization logic for other reasons.

Yes, but I don't think you have custom deserializers for more than a tiny subset of the languages that you can generate types for (just Javascript and a prototype for C#?). I think most people at this point will have to (or want to) use the built-in or standard JSON deserializers, which may choke on the $class properties in maps in a way that they won't for concepts. When deserializing classes, unknown properties are just ignored. When deserializing dictionaries, no properties are ignored.