[we4sz]

Slack adds fields to its objects faster than SlackNet can add properties for them, and today only events (and message search results) have a way to get at the ones SlackNet doesn't know about yet: Event.ExtraProperties, which I used in #264 before it got real properties. Every other model silently drops them. I ran into it building an app on SlackNet that needs is_email_confirmed from users.info: User has no property for it, so I read the raw response next to the typed one, and the same response also had who_can_share_contact_card, display_name_normalized, image_1024 and is_custom_image. A property for each is its own change, and the next missing field will be on another type, so this gives every model what events have, once. It is eight commits, each one builds and passes the tests.

1. SlackObject, an abstract base class with ExtraProperties: a [JsonExtensionData] dictionary like Event has, declared once.
2. to 5. The models derive from it, one area per commit and one line per class: the objects (User, UserProfile, Conversation, File, Team, BotInfo, UserGroup, ...), the Web API responses and what they contain, blocks and their elements, and the interaction, event, RTM and Socket Mode models. That is 252 classes, which makes 416 concrete types; the 129 events and four other types already had a catch-all of their own.
3. ModelLintTests, in the style of ApiLintTests and BlockLintTests. It fails for a public model that has no way to keep unknown properties, so the next model added has to derive from SlackObject, and it checks that every SlackObject keeps an unknown property.
4. Tests that read a User with its profile and enterprise user, a Conversation with its topic, and a section block with its text, through the API client and the event deserializer the way an app would, and a readme section.
5. An ADR in the style of 0001, with the options I considered. It's the last commit, so it's easy to drop.

Compatibility

Additive: nothing that is sent to Slack changes. The dictionary is read-only for Json.NET (WriteData = false): it is filled when Slack's JSON is read and never written, so an app that sends back an object it received, the blocks of a message for example, sends what SlackNet knows about and nothing else. I did that on purpose, because Slack may not accept on input everything it sends on output, and a catch-all for reading shouldn't change what is sent. Event.ExtraProperties is still written back, as before, so Event keeps its own instead of deriving from SlackObject.

Which types don't derive from it:

• Event, MessageSearchResult, Icons, ErrorResponse and WebApiResponse have extension data of their own (Icons.Images, ErrorResponse.Info and WebApiResponse.Data are made from it). Json.NET fills one extension data member per type, the most derived one, so deriving as well would leave ExtraProperties empty. The lint names them.
• SlackRequestContext and SocketModeConnectionOptions look like models to the lint but are local state and configuration, so it names them too.
• There are no structs and no sealed external base classes among the models. Types with no parameterless constructor (Link, FileUpload, the MessageResponse wrappers) are only built in code and sent, so Json.NET never fills them, and the internal response wrappers (UserResponse and so on) only carry the object an API method returns, which callers never see.

Models that are only ever sent (Message, Dialog, the view definitions, ...) have it too, and it is never filled. The rule is every model Json.NET can fill, which a lint can check, and I didn't want to guess which way each one goes.

A class that derives from a SlackNet model and declares its own [JsonExtensionData] keeps working: Json.NET fills that member and the inherited ExtraProperties stays empty (I checked with a subclass of User).

Testing

ModelLintTests covers all 549 concrete models SlackNet has: 416 derive from SlackObject, 129 are events and 4 have their own. A new model that forgets the base fails AllModelsHaveACatchAllForUnknownProperties (I tried it), and UnknownPropertiesAreReadButNotWritten fails for all 416 if the dictionary is made to write back (I tried that too). ExtraPropertiesTests follows the API client tests: unknown properties on a User, its profile and its enterprise user, on a Conversation and its topic, and on a section block and its text; a user with only known properties having nothing in ExtraProperties; and a block read with an unknown property not sending it back. ./do test passes on every commit (783 tests, then 785 and 790 once the lint and the other tests are in). It also builds in Release without new warnings.

Not verified:

• The cost. Every model now holds an empty dictionary, and properties that used to be skipped when reading are parsed into it. I haven't measured either, for example on a large users.list.
• Whether Slack accepts on input what it sends on output. That is why the dictionary isn't written back, so nothing here depends on it, but it also means this can't be used to send a field SlackNet doesn't have yet. That would be a separate change, and ChatApi builds its arguments from Message property by property, so it wouldn't just fall out of this one.
• The values are what Json.NET reads into object (string, bool, long, double, JObject, JArray), the same as on events. I haven't wrapped them in anything.
• I haven't run my app against a local pack of this branch.

[soxtoby]

Thanks for all of these, @we4sz. I've merged your other PRs and they'll go out in the next release.

I'm going to close this one, though. Missing properties don't cause problems often enough to justify a change as sweeping as adding ExtraProperties to every model. I'll try to provide a narrower escape hatch for handling them. Adding the missing properties to the library is still the ideal, though, so I'll also work on streamlining the release process to make small PRs like your others here easier to get released.