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.

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

HTTPServerRequest & HTTPServerResponse as task-global objects #432

Closed
etcimon opened this Issue Dec 9, 2013 · 5 comments

Comments

Projects
None yet
3 participants
@etcimon
Contributor

etcimon commented Dec 9, 2013

I've found this to somewhat of a shortcut in developing a clean server application architecture. I'd like to have those available at all times through the program. My main concern is that it would let me use native D objects and register through REST without losing the session, but it would also help avoid any further issues with data access.

I've tried adding this to server.d

private TaskLocal!(FreeListRef!HTTPServerRequest) req;
private TaskLocal!(FreeListRef!HTTPServerResponse) res;

and then
req = FreeListRef!HTTPServerRequest(reqtime, listen_info.bindPort);

but it fails with
Error: function vibe.core.core.TaskLocal!(FreeListRef!(HTTPServerRequest)).TaskLocal.opAssign (TaskLocal!(FreeListRef!(HTTPServerRequest)) p) is not callable using argument types (FreeListRef!(HTTPServerRequest))

I think the issue is in TaskLocal, it needs to implement a type-safe storage with it's own allocator in m_fls. Any ideas?

@mihails-strasuns

This comment has been minimized.

Show comment
Hide comment
@mihails-strasuns

mihails-strasuns Dec 9, 2013

Contributor

I am against making any sort of global objects that are not semantically server-global. Again, you are trying to hack REST module to do what it shouldn't do. I am somewhat satisfied with fact that current design makes it hard.

Contributor

mihails-strasuns commented Dec 9, 2013

I am against making any sort of global objects that are not semantically server-global. Again, you are trying to hack REST module to do what it shouldn't do. I am somewhat satisfied with fact that current design makes it hard.

@mihails-strasuns

This comment has been minimized.

Show comment
Hide comment
@mihails-strasuns

mihails-strasuns Dec 9, 2013

Contributor

Better way would have been to write own module similar to rest that provides req and res as an arguments for class methods in HTTP handler (similar to how those are currently provided by @before / @after.

Contributor

mihails-strasuns commented Dec 9, 2013

Better way would have been to write own module similar to rest that provides req and res as an arguments for class methods in HTTP handler (similar to how those are currently provided by @before / @after.

@s-ludwig s-ludwig closed this in 5fd03bb Dec 9, 2013

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Dec 9, 2013

Member

The signature of TaskLocal.opAssign was wrong, so this should work now. However, this has several drawbacks for performance and safety and is deliberately not part of the core system. In a more specialized setting, this may be a valid approach for a more convenient interface, though. You can of course also achieve it without modifying the library by storing the req/res parameters of the request callback and then calling your request handlers without the parameters.

Member

s-ludwig commented Dec 9, 2013

The signature of TaskLocal.opAssign was wrong, so this should work now. However, this has several drawbacks for performance and safety and is deliberately not part of the core system. In a more specialized setting, this may be a valid approach for a more convenient interface, though. You can of course also achieve it without modifying the library by storing the req/res parameters of the request callback and then calling your request handlers without the parameters.

@etcimon

This comment has been minimized.

Show comment
Hide comment
@etcimon

etcimon Dec 9, 2013

Contributor

I'm mostly trying to avoid the garbage associated with wrapping each handler. I'm sure I can come up with something convenient. From the general reaction, what I was suggesting pushes the limits a bit too far but it's a fair try on my end.

Contributor

etcimon commented Dec 9, 2013

I'm mostly trying to avoid the garbage associated with wrapping each handler. I'm sure I can come up with something convenient. From the general reaction, what I was suggesting pushes the limits a bit too far but it's a fair try on my end.

@etcimon

This comment has been minimized.

Show comment
Hide comment
@etcimon

etcimon Dec 9, 2013

Contributor

I came up with a good name for compiling with this.

    version(With_REST_hack){
         private TaskLocal!(FreeListRef!HTTPServerRequest) req;
         private TaskLocal!(FreeListRef!HTTPServerResponse) res;
         @property Session _session(){ return req.session; }
    }

...

    void index()
    {
        auto session = _session;

...
Compiles 100% and very convenient

Contributor

etcimon commented Dec 9, 2013

I came up with a good name for compiling with this.

    version(With_REST_hack){
         private TaskLocal!(FreeListRef!HTTPServerRequest) req;
         private TaskLocal!(FreeListRef!HTTPServerResponse) res;
         @property Session _session(){ return req.session; }
    }

...

    void index()
    {
        auto session = _session;

...
Compiles 100% and very convenient

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment