[nexus] Break up db model into multiple files

[smklein]

• moved nexus/src/db/model.rs into nexus/src/db/model/mod.rs
• migrated all DB structures into separate files

[smklein]

On the bright side, this is done now

[smklein]

By line count, the other heavy offenders are:

   3038 ./external_api/http_entrypoints.rs
   3685 ./db/datastore.rs
   4028 ./nexus.rs

If we're happy with the direction of this PR, I think we'd benefit from breaking these up into smaller files too.

This is subjective, but IMO, having smaller files improves discoverability while also making decoupling a bit more natural.

[david-crespo]

long overdue, big improvement. let's do the other ones

[davepacheco]

Thanks for looking at this. Yeah, some of the files have definitely gotten very large. At the same time, I find it a lot harder to discover and grok a large number of tiny files than a smaller number of larger files, especially if the latter are well organized. For example, this PR has almost four dozen files now in nexus/src/db/model, and the relationships between related models are gone. "region.rs" is next to "role_assignment.rs" but they have nothing to do with each other, while "user_builtin.rs" (which is only 23 lines) is not near other IAM-related things. What do you think of grouping these a little more thematically, like maybe "iam.rs", "networking.rs", etc?

[smklein]

Sure, that would be okay with me. I think there are a number of ways to resolve the "grouping":

• Use a common prefix. instance.rs is right next to instance_state.rs. vpc.rs is right next to vpc_route.rs, vpc_firewall_rule.rs, etc.
• Have related models in the same file, as you suggested. I actually think something as broad as networking.rs will run into the same problem as model.rs did - it's still going to grow to be thousands of lines long. But, as an example, I did leave structures like VpcSubnet and VpcSubnetUpdate in the same file, because they're very tightly coupled.

[david-crespo]

I think I like the prefix idea better than the bigger files.

[davepacheco]

I prefer bigger files because they can be structured with a logical narrative instead of strictly alphabetical. When I'm coming up to speed on something I like to pull up a big file, read it top to bottom (ideally starting with a big subsystem block comment), and feel like I understand the structure of the thing.

[bnaecker]

Another option is additional layers of hierarchy in the module structure. For example, db::model::networking::{vpc,vpc_subnet,firewall_rules,...}.rs and similar. That might provide some level of discoverability, while still having relatively small files and related types grouped closely together.

[smklein]

@bnaecker I like that idea - it's a nice compromise, because it lets those of us who like small files keep them, while it also provides a natural grouping (by directory) in which we could include, for example, area-specific documentation

[davepacheco]

I don't know where to discuss the other ones in flight before we go ahead and do them...

Regarding nexus.rs: is it worth rethinking this one? The original idea was to have a sort of business logic layer between the datastore and the HTTP endpoints that could be shared with interfaces besides than the HTTP endpoints. The best case for something like this is the logic around instance_set_runtime(), which is non-trivial, unrelated to the HTTP API itself, and might be used elsewhere like sagas or Nexus background tasks.

At the same time it feels like there's a ton of boilerplate here where this layer isn't adding anything. For the various CRUD functions that are all just (1) use LookupPath to look up a resource and then (2) call a datastore function to do something with that resource, I wonder if we're better off doing that stuff directly from the HTTP entrypoints.

The datastore layer exposes a pretty low-level interface atop the database that attempts to ensure database-level integrity, but doesn't know about higher-level application-level concerns. Examples: the idea of an Actor or ConsoleSessionWithSiloId is specific to the authn subsystem. Maybe the way to break up Nexus is to have more of these subsystems, maybe for different groups of resources, like an "instance" submodule that would contain the equivalent of instance_set_runtime(), etc. This might be clearer than a monolithic layer like nexus.rs currently provides. This is also different from just splitting the existing Nexus functions into different files. There's still a question of what to do for things that really have no specialized logic around them that are just basic CRUD.

[david-crespo]

I'm ambivalent about the nexus.rs CRUD-only functions. When I see a function that does one thing and is called once, I usually would like to see it inlined at the call site (the endpoint handler). On the other hand, I can see a maintainability argument for keeping the one-liner functions in the service layer (what I call nexus.rs in my head), because it means when you want to expand that logic or share it between endpoints, you don't have to complicate the diff by also doing the extraction to a function in nexus.rs. That's definitely the Railsy approach, where one-liner functions are the norm, in the name of having things in the "right" place. Like I said, I'm ambivalent — sometimes I feel the Rails style is overly defensive, putting too much junk in my way to guard against a hypothetical future "bad" developer that doesn't exist.

[smklein]

FWIW, I've filed #1016 so we can discuss further, if folks want to redirect conversation there. Apologies for not making an issue earlier!