[api][nexus] Migrate DB-specific types into Nexus, away from API

[smklein]

I have found myself repeatedly bumping into some related issues:

• I want to use a type, but it's unclear if it's used in the DB, part of an API, or both.
• If I want to change the representation of a type in the DB, it's difficult to do so without simultaneously modifying the type exposed in the API, even though that may be unintentional (the reverse situation of modifying something in the API, but not the DB, would also apply).
• For types which are stored in the DB, it's kinda a pain in the butt to modify them, since their serialization/deserialization functions are not co-located with their types.

This PR is an attempt at addressing this imbalance, starting with the Instance types. If we like this direction, I could do the same for all other DB-stored types:

• Add types to nexus/db/model.rs which are stored directly in the DB.
• Define the serialization/deserialization functions alongside these declarations.
• Define conversion functions to/from API-based types. Conversion to "in-memory" types would also be viable here, but that has't been necessary yet.

The net result of this is "if you wanna store something in the database, you should update the init.sql file and db/model.rs, and you'll need to poke at little else".

[smklein]

TODO: Rename from "types.rs" to "model.rs" to make @david-crespo happy 😁

(also consider splitting "model.rs" into a bunch of smaller files)

[david-crespo]

I kinda think this should just implement the same struct independently rather than wrapping internal::nexus::Project. Renaming internal::nexus::Project to internal::nexus::ProjectView (which I think is more accurate) shows why this is a little weird.

This would let us take all references to internal::nexus::Project out of this file by moving the from/to implementations elsewhere, which fits better with my mental model — this file is about the models and their relation to the database, not their relation to the view structs.

[david-crespo]

Same for the other uses of internal::nexus in this file. I think it would really simplify reasoning to know that the models in this file don't know anything about the views in omicron-common, i.e., you don't need to know anything about the latter to understand what's in here. If they happen to be the same, that's a contingent fact, not one we need to hard-code.

[smklein]

I kinda think this should just implement the same struct independently rather than wrapping internal::nexus::Project. Renaming internal::nexus::Project to internal::nexus::ProjectView (which I think is more accurate) shows why this is a little weird.

I agree with you here - separating the DB type from the API type is definitely the plan. I'll go ahead and update this type.

This would let us take all references to internal::nexus::Project out of this file by moving the from/to implementations elsewhere, which fits better with my mental model — this file is about the models and their relation to the database, not their relation to the view structs.

So I'm happy for all structs in this file to be more decoupled from the API-based types, but those conversion functions should exist somewhere, and they'll need to act on both types. Do we really want them to be separate from the struct definition if they're functions acting on that struct?

[smklein]

Same for the other uses of internal::nexus in this file. I think it would really simplify reasoning to know that the models in this file don't know anything about the views in omicron-common, i.e., you don't need to know anything about the latter to understand what's in here. If they happen to be the same, that's a contingent fact, not one we need to hard-code.

I agree with this in principle, and I support doing it for most structures, but as an FYI, this will also require re-definitions of the all primitive types that aren't in std. external::Name, external::ByteCount, external::Generation, etc.

This is possible, but it'll expand the size of this PR a bit. Do you mind if I hold off on the full extent of this until we get feedback from @davepacheco ?

[david-crespo]

Do we really want them to be separate from the struct definition if they're functions acting on that struct?

Yeah, I think of them as the view layer. I think it makes sense to separate them. It might feel better if you think of it as From on ProjectView rather than as Into on Project, if that makes any sense. Even though I just learned From gets you Into for free anyway, so it doesn't actually matter, conceptually I like to think of the view layer as knowing about the model layer (like you said, it has to take the model as an argument) but not the other way around. And that would be mirrored in keeping the Model -> View conversions elsewhere. Here is an example of them doing it this way in the crates.io source.

By the way, why would we need to convert from the view to the model? (Though again it doesn't matter implementation-wise because From gets you Into.)

[david-crespo]

And yeah, this PR is plenty big. I just thought it was a good spot to make the note.

[david-crespo]

FYI, this will also require re-definitions of the all primitive types that aren't in std. external::Name, external::ByteCount, external::Generation, etc.

Maybe for the primitives this makes less sense 😁 From a reasoning-about-code point of view I think importing those from common is not as big a burden as pulling in whole View structs. There are a limited number of primitives, hopefully, whereas the number of views to import is large and growing.

[david-crespo]

yes