karlseguin / http.zig

An HTTP/1.1 server for zig
MIT License
454 stars 31 forks source link

Suggestion - move context to the 1st param instead of the last #4

Closed zigster64 closed 1 year ago

zigster64 commented 1 year ago

When using ServerCtx() - the context passed is appended to the end of the handler parameters.

If you move the context to being the 1st parameter instead, then you get a nice side effect of being able to easily connect struct member functions directly as http handlers. (since they expect the 1st param == *Self)

Not saying this is a good vs bad practice (many arguments for and against), but having that option there does make some tasks a bit easier to write.

Another side effect is that it is idiomatic-Go to pass context first, so ... that would also make http.zig a bit easier to swallow for porting existing Go code ?

const World = struct {
   ... lots of state

  pub fn getTwibbles(self, res, req) {
   ... return all my twibbles
  }

  pub fn getFribbles(self, res, req) {
  .. return all my fribbles
 }
};

pub fn main () void {
  my_world = load_world();
  server = httpz.ServerCtx(*World, *RequestCtx).init(...,  &my_world);
  router = server.router();
  // apis that use the World context
  router.get("/api/v1/twibbles", World.getTwibbles);   // same as calling -  my_world.getTwibbles(res, req)
  routre.get("/api/v1/fribbles", World.getFribbles);
}

If interested, the code to do this is more than a few lines ie https://github.com/zigster64/http.zig/pull/1

karlseguin commented 1 year ago

Thanks for the suggestion. I merged it from your branch and made a few cosmetic changes (updated the readme, reverted to tab spacing, moved the index.html to the example folder, ...)

I think it's a good idea with very few drawbacks since you still don't have to use the structure method approach if you don't want to, but now you can if you want.

zigster64 commented 1 year ago

Sweet, thank you.

yeah, seems logical. Feels good to contribute, thx

karlseguin commented 1 year ago

Thanks again for this, it's proven useful to me.

In fact, it allowed me to add more flexibility with respect to global data. Global data can now be set on a route-basis, like a custom dispatcher:

const special_world = load_special_world();
router.getC("/api/v1/twibbles", World.getTwibbles, .{.ctx =  &special_world});

Or, for all subsequent routes (which isn't my favorite API because of how easy it is to mess up, but does save a lot of repetitive configuration):

const special_world = load_special_world();
router.ctx(&special_world);
router.get("/api/v1/twibbles", World.getTwibbles); 
routre.get("/api/v1/fribbles", World.getFribbles);

If the route has a ctx (global data), it uses that and fallsback to using the global ctx set on server init.

zigster64 commented 1 year ago

maybe ... (and I havent really thought this through, just off the top of my head)

Thinking of like router.Group() with a context at that level passed down to descendants

// A horribly contrived example of what Im thinking
const Animal = struct { ...};

apis = router.Group("/api");

const Tigers = some collection of Animal from somewhere;
const Sloths = some collection of Animal from somewhere;

const tiger_api = apis.Group("/tigers", .{.ctx  = &Tigers});
tiger_api.get("/show/:id", Animal.show);
tiger_api.post("/unleash/:id", Animal.attack);

const sloth_api = apis.Group("/sloths", .{.ctx = &Sloths});
sloth_api.get("/show/:id", Animal.show);
sloth_api.post("/unleash/:id", Animal.sleep);

So endpoint /api/sloths/show/1 calls Sloths.show(req, res) and /api/tigers/show/2 calls Tigers.show(req, res) or something.

I like the way this is panning out anyway, its very nice already

karlseguin commented 1 year ago

Yes, I think something like this is the right way forward.

karlseguin commented 1 year ago

Router groups are now available.