Controller Refactor Proposal

[vapor]

Controller Refactor Proposal

[tanner0101]

I think this could be solved by creating a function on the Controller class called before(request: Request) that would get called before the desired routing method is called.

//how the application registers the index route for a given controller
self.get("users") { request in
  controller.before(request)
  return controller.index(request)
}

But I think most of the before/after request functionality is already fulfilled by Middleware. Could you provide an example of something that wouldn't work with Middleware?

Additionally, currying is being removed in Swift 3.0

[ketzusaka]

Going from the bottom up:

Currying isn't being removed in Swift 3, just the odd syntax where you can do stuff like this:

func foo()() -> Void

That'll just need to be written as:

func foo() -> () -> Void

Putting something as a Middleware would work if we need it to apply to all controllers, but not if you want to define before/after functionality on a per-controller basis.

Adding a function that is called would also work. The only limitation I still see by doing that is stored properties on the controller that depend on the Request object wouldn't be able to be non-optional.

The particular use-case I have is that I made a protocol called PollRetrievable. This protocol has a requirement for a Channel property, and defines a method to read a poll: readPoll() -> Poll?. This method depends on data contained from the Request class (the channel), which is provided by a path parameter. By passing the data to the initializer, Channel can be a let and non-optional, but if this was setup in another method (such as before(request: Request)), the protocol would have to expect an optional and handle unwrapping inside of the readPoll method, and my controllers would need to define Channel as optional even though every operation within them require it to be defined.

The readPoll() method is used by my ChannelController (which mentioned above is a ChannelDashboard) and my PollController. The PollController has mutation operations on a Poll, while the ChannelController just wants to display information about the Poll, but both need information about the currently open poll (for creating a new poll we verify that a poll isn't already open, so we expect the read method to return nil). In the initialization method defined by Controller, readPoll() is called in both ChannelController and PollController's initializers, making a Poll? object available to all methods of the class.

Lastly, I have an AssetsController, which doesn't care about the channel, has completely different routes, and doesn't depend on a Poll, so it doesn't conform to PollRetrievable and doesn't need a channel to operate on. This is why a Middleware for this wouldn't fit great.

Hopefully that all made sense. Thanks :)

[tanner0101]

That makes sense.

Another issue I see with the current implementation of Controllers is that they might be the same between requests. I haven't tested this, but I think they are only initialized once.

So if you stored any sensitive data for Person A's request on the controller, Person B might be able to see that.

[ketzusaka]

it looks like they are unique instances!

Is it worth me putting together a PR based on my proposals?

[tanner0101]

Yeah for sure.