Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use std.http.Method for Request.method #72

Merged
merged 2 commits into from
Jan 31, 2024
Merged

Use std.http.Method for Request.method #72

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
92fdb66
Use std.http.Method for Request.method
Froxcey Jan 27, 2024
37f7b8b
Use custom method enum
Froxcey Jan 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/hello2/hello2.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const std = @import("std");
const zap = @import("zap");

fn on_request(r: zap.Request) void {
const m = r.method orelse "";
const m = r.method_str orelse "";
const p = r.path orelse "/";
const qm = if (r.query) |_| "?" else "";
const qq = r.query orelse "";
Expand Down
3 changes: 1 addition & 2 deletions examples/hello_json/hello_json.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ const User = struct {
};

fn on_request(r: zap.Request) void {
if (!std.mem.eql(u8, r.method.?, "GET"))
return;
if (r.method == null or r.method.? == .GET) return;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be != .GET

I am not sure if I like having to check 2 things instead of one

Copy link
Contributor Author

@Froxcey Froxcey Jan 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be != .GET

yes, that is a bug

I am not sure if I like having to check 2 things instead of one

r.method.? should be checked anyways, I think


// /user/n
if (r.path) |the_path| {
Expand Down
26 changes: 12 additions & 14 deletions src/endpoint.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,20 +60,18 @@ fn nop(self: *Endpoint, r: Request) void {

/// The global request handler for this Endpoint, called by the listener.
pub fn onRequest(self: *Endpoint, r: zap.Request) void {
if (r.method) |m| {
if (std.mem.eql(u8, m, "GET"))
return self.settings.get.?(self, r);
if (std.mem.eql(u8, m, "POST"))
return self.settings.post.?(self, r);
if (std.mem.eql(u8, m, "PUT"))
return self.settings.put.?(self, r);
if (std.mem.eql(u8, m, "DELETE"))
return self.settings.delete.?(self, r);
if (std.mem.eql(u8, m, "PATCH"))
return self.settings.patch.?(self, r);
if (std.mem.eql(u8, m, "OPTIONS"))
return self.settings.options.?(self, r);
}
const Method = std.http.Method;
return if (r.method) |m| {
switch (m) {
Method.GET => self.settings.get.?(self, r),
Method.POST => self.settings.post.?(self, r),
Method.PUT => self.settings.put.?(self, r),
Method.DELETE => self.settings.delete.?(self, r),
Method.PATCH => self.settings.patch.?(self, r),
Method.OPTIONS => self.settings.options.?(self, r),
else => return,
}
};
}

/// Wrap an endpoint with an Authenticator -> new Endpoint of type Endpoint
Expand Down
3 changes: 2 additions & 1 deletion src/request.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,8 @@ pub const CookieArgs = struct {
path: ?[]const u8,
query: ?[]const u8,
body: ?[]const u8,
method: ?[]const u8,
method: ?std.http.Method,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose that the optional makes no sense here. I think an enum (default) value of "invalid" or sth similar makes more sense.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fck. does std.http.Method support indicating it wasn't possible to parse the method?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm. What about defining our own enum? WDYT? It could even be based on std.http.Method, but add a value for "invalid". std.http returns error.HttpHeadersInvalid which we can't do so we might as well put an invalid field into the enum.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that would make switching more convenient, no need for checking != null

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about a custom enum, but I went with std.http.Method simply because it's easier. I agree that .INVALID would be nice, and I will add that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or should it be called .Custom or .Other, cuz it's not entirely impossible that someone needs to define a custom HTTP method for whatever they are working on.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe even Unexpected? Because we expect a fixed set and did not detect any of those. So it comes unexpected to zap. Or just NonStandard?

method_str: ?[]const u8,
h: [*c]fio.http_s,

/// NEVER touch this field!!!!
Expand Down
30 changes: 27 additions & 3 deletions src/zap.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,27 @@ pub const HttpListenerSettings = struct {
tls: ?Tls = null,
};

fn methodToEnum(method: ?[]const u8) ?std.http.Method {
const Method = std.http.Method;
{
if (method) |m| {
if (std.mem.eql(u8, m, "GET"))
return Method.GET;
if (std.mem.eql(u8, m, "POST"))
return Method.POST;
if (std.mem.eql(u8, m, "PUT"))
return Method.PUT;
if (std.mem.eql(u8, m, "DELETE"))
return Method.DELETE;
if (std.mem.eql(u8, m, "PATCH"))
return Method.PATCH;
if (std.mem.eql(u8, m, "OPTIONS"))
return Method.OPTIONS;
}
return null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here is where the invalid would go

}
}

/// Http listener
pub const HttpListener = struct {
settings: HttpListenerSettings,
Expand All @@ -207,7 +228,8 @@ pub const HttpListener = struct {
.path = util.fio2str(r.*.path),
.query = util.fio2str(r.*.query),
.body = util.fio2str(r.*.body),
.method = util.fio2str(r.*.method),
.method = methodToEnum(util.fio2str(r.*.method)),
.method_str = util.fio2str(r.*.method),
.h = r,
._is_finished_request_global = false,
._user_context = undefined,
Expand All @@ -233,7 +255,8 @@ pub const HttpListener = struct {
.path = util.fio2str(r.*.path),
.query = util.fio2str(r.*.query),
.body = util.fio2str(r.*.body),
.method = util.fio2str(r.*.method),
.method = methodToEnum(util.fio2str(r.*.method)),
.method_str = util.fio2str(r.*.method),
.h = r,
._is_finished_request_global = false,
._user_context = undefined,
Expand All @@ -254,7 +277,8 @@ pub const HttpListener = struct {
.path = util.fio2str(r.*.path),
.query = util.fio2str(r.*.query),
.body = util.fio2str(r.*.body),
.method = util.fio2str(r.*.method),
.method = methodToEnum(util.fio2str(r.*.method)),
.method_str = util.fio2str(r.*.method),
.h = r,
._is_finished_request_global = false,
._user_context = undefined,
Expand Down