Propose new MVC module layout

[jtgeibel]

This pull request proposes a new module layout following a model-view-controller style. For now, the functionality is re-exported from the new locations and the use statements have been updated to use the new locations. If we define where things should be moved to now, it should be easier to split future changes into small, reviewable PRs.

Controllers - src/api/*

For now, this is an empty placeholder, but the api route declarations and endpoints would be moved here. (In a separate branch I have some examples where the existing logic is moved.)

Views - src/views/*

Currently this is JSON only and most existing types are prefixed with Encodable. The major exception to this is the types from the cargo publish endpoint, which has been moved to views::krate_publish.

Models - src/models/*

All of the Diesel types would move to here. Many of these were previously reexported from the crate root.

Going forward, I think many of the New* structs can go away using the new syntax as shown in:

crates.io/src/boot/categories.rs

Lines 105 to 109 in e0e57b9

	(
	slug.eq(c.slug.to_lowercase()),
	category.eq(c.name),
	description.eq(c.description),
	)

Future work

We can consider additional top level modules. For instance, the outgoing email and github_auth functionality could be moved to a module for external services. Additionally, src/pagination.rs could be moved into src/api/mod.rs and a few more things could probably move to util. I'm not sure yet where something like the README rendering would fit in.

[kureuil]

Thanks for your work on this PR @jtgeibel!

Even though the crates.io backend could use some cleaning and refactoring, I'm not sure that grouping things as you're proposing is ideal. IMO, I think it is better to group elements by functionnlity rather than usage. Which means that instead of having top-level api, api_types & models modules, I'd have krate, user, version, category, etc. modules and each one of these modules would have its own api, api_types & models modules.

.
├── category
│   ├── controllers.rs
│   ├── mod.rs
│   ├── models.rs
│   └── views.rs
.

Grouping things like this brings up a big pro: it is far easier to encapsulate internal behavior, and it will even get more powerful with the incoming pub(...) changes. I find this extremely important: the crates.io codebase is bound to grow with time and as such we should set-up the architecture that will limit errors as much as possible. For example, instead of exposing all of its endpoints, a module could instead expose a single function named router that would return a conduit_router::RouteBuilder that could then be mounted in src/lib.rs: making such a change would mean that the endpoints wouldn't be accessible from the outside of the module. Another advantage of organizing things this way is that you don't have to wonder about how to handle modules such as render: they are just regular modules that happen to not expose any Conduit related stuff.

Controllers

Should we additionally namespace things under a v1 module?

I don't think that this is necessary right now, there probably is more urgent matters than a new API version. It would complicate things for no obvious benefits at the moment.

Should this be called controllers instead?

👍 on this one, I think controllers better reflects the contents of the module than api which seems too generic IMO. Other possible names: endpoints, routes.

Views

I think that views is a better module name than api_types, and much more explicit about its intent/contents.

Should we spin most of the JSON types off to a separate crate that could be used by clients?

Collaboration probably should happen with the Cargo team, which is currently maintaining the crates-io crate.

Would something like src/views/json/ be better? For instance, we may want to add an RSS endpoint for followed crates.

If I'm not mistaken, all of our current endpoints currently return JSON, it doesn't seem to be necessary to introduce such specific nesting right now.

Models

👍 Nothing to add.

Future work

Building on what said earlier, I think that grouîng things by functionality/service/domain would help a lot in growing the crates.io codebase, and would help avoid putting stuff where it doesn't belong.

[jtgeibel]

Controllers

I'll rename to controllers and agree that v1 is not needed at this time.

For example, instead of exposing all of its endpoints, a module could instead expose a single function named router that would return a conduit_router::RouteBuilder that could then be mounted in src/lib.rs: making such a change would mean that the endpoints wouldn't be accessible from the outside of the module.

Under my scheme, src/controllers/mod.rs would define a private module for each group of endpoint functionality, but only the top level build function would be exported and called from src/lib.rs. The individual endpoints would not be accessible from the rest of the crate.

Views and models

I don't understand why we wouldn't want to have views and models as top-level modules. In your scheme, we would be defining these structs and impls under a large (and growing) number of top-level modules. Things that are view-only or model-only would be polluting the top-level namespace. Something like schema could probably be private to the models code. All of the git operations against the crates.io-index would be contained in the model logic as well.

Multiple endpoints may access the same view, and multiple views may access a model, so I don't think restricting visibility within say, the categories module, would be very common.

[carols10cents]

A downside to a layout like this:

.
├── category
│   ├── controllers.rs
│   ├── mod.rs
│   ├── models.rs
│   └── views.rs
.

is that while working on, say, something that involves the krate model and the version model, i'll have 2 tabs in my editor open with the title "models.rs" and that would make me sad :( I already have enough of a problem with multiple open mod.rs files, and I really really like that the path/modules RFC contains a way of not having lots of mod.rs files anymore. I don't know if I'd like imposing that on ourselves again :(

[carols10cents]

I was going to figure out what the new file+module layout would be once the TODOs in this PR were done, and I started doing it and I think I like it. I agree that controller seems better than api and views seems better than api_types, so with those changes I would r+ this :)

[jtgeibel]

Sounds good to me @carols10cents. I've had this on the back burner for a while now, but I have off next week for the holidays and definitely intend to get this cleaned up soon.

[jtgeibel]

@carols10cents I've finally rebased this PR and renamed the modules to the more standard models, views, and controllers. I've also opened #1247 which is built on top of these commits and implements the split in functionality for keywords and categories.

[carols10cents]

The use statements are looking a lot more organized 👍 👍 👍

bors: r+

[bors-voyager]

Build succeeded

• continuous-integration/travis-ci/push