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.

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

New feature: functional callbacks #91

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fhessel
Copy link
Owner

@fhessel fhessel commented Jun 6, 2020

Allow to use std::functional or std::bind to be used with ResourceNodes.

Needs a bit of testing before merging.

References

@fhessel fhessel added the CI: Build Examples When set, the examples will be built for this PR label Jun 6, 2020
@github-actions github-actions bot removed the CI: Build Examples When set, the examples will be built for this PR label Jun 6, 2020
@wubbl0rz
Copy link

tested this PR today on an esp32 ttgo t-display. works greats 😄
used your example code from: issue

i tried wrapping my OTA code inside a class and couldn't get it to work without static methods or functions outside of the class. but with this PR everything just works.

secureServer = new HTTPSServer(cert, port, 5);

HTTPSCallbackFunction updateCallback =
    std::bind(&UpdateServer::handleUpdate, this, std::placeholders::_1, std::placeholders::_2);

ResourceNode *updateNode = new ResourceNode("/update", "POST", updateCallback);
secureServer->registerNode(updateNode);
secureServer->start();

@dethegeek
Copy link

Hi

I also tried to use your PR with success, but it would be also useful to implement middlewares as methods of a class.

I tried to do this myself, but I needed to implement methods callbacks also for middlewares. As i used this PR as base branch, I think it is better to share a diff here for review. Please note that some changes are untested (the operator==, removal of middlewares and old middlewares implementation.

diff --git a/src/HTTPMiddlewareFunction.hpp b/src/HTTPMiddlewareFunction.hpp
index e8b7cec..d0384b5 100644
--- a/src/HTTPMiddlewareFunction.hpp
+++ b/src/HTTPMiddlewareFunction.hpp
@@ -21,6 +21,8 @@ namespace httpsserver {
    * handling in case of missing authentication. Don't forget to call next in case you want to access your
    * resources, though.
    */
-  typedef void (HTTPSMiddlewareFunction)(HTTPRequest * req, HTTPResponse * res, std::function<void()> next);
+  typedef std::function<void(HTTPRequest * req, HTTPResponse * res, std::function<void()> next)> HTTPSMiddlewareFunction;
+
+  bool operator==(const HTTPSMiddlewareFunction& lhs, const HTTPSMiddlewareFunction& rhs);
 }
  #endif /* SRC_HTTPMIDDLEWAREFUNCTION_HPP_ */
diff --git a/src/ResourceResolver.cpp b/src/ResourceResolver.cpp
index 6eab51d..932eb94 100644
--- a/src/ResourceResolver.cpp
+++ b/src/ResourceResolver.cpp
@@ -160,15 +160,23 @@ void ResourceResolver::resolveNode(const std::string &method, const std::string
   }
 }
 
-void ResourceResolver::addMiddleware(const HTTPSMiddlewareFunction * mwFunction) {
+void ResourceResolver::addMiddleware(const HTTPSMiddlewareFunction mwFunction) {
   _middleware.push_back(mwFunction);
 }
 
-void ResourceResolver::removeMiddleware(const HTTPSMiddlewareFunction * mwFunction) {
+void ResourceResolver::addMiddleware(void (*mwFunction)(HTTPRequest * req, HTTPResponse * res, std::function<void()> next)) {
+  _middleware.push_back(HTTPSMiddlewareFunction(mwFunction));
+}
+
+void ResourceResolver::removeMiddleware(const HTTPSMiddlewareFunction mwFunction) {
+  _middleware.erase(std::remove(_middleware.begin(), _middleware.end(), mwFunction), _middleware.end());
+}
+
+void ResourceResolver::removeMiddleware(void (*mwFunction)(HTTPRequest * req, HTTPResponse * res, std::function<void()> next)) {
   _middleware.erase(std::remove(_middleware.begin(), _middleware.end(), mwFunction), _middleware.end());
 }
 
-const std::vector<HTTPSMiddlewareFunction*> ResourceResolver::getMiddleware() {
+const std::vector<HTTPSMiddlewareFunction> ResourceResolver::getMiddleware() {
   return _middleware;
 }
 
diff --git a/src/ResourceResolver.hpp b/src/ResourceResolver.hpp
index cb67e7f..212457e 100644
--- a/src/ResourceResolver.hpp
+++ b/src/ResourceResolver.hpp
@@ -30,11 +30,13 @@ public:
   void resolveNode(const std::string &method, const std::string &url, ResolvedResource &resolvedResource, HTTPNodeType nodeType);
 
   /** Add a middleware function to the end of the middleware function chain. See HTTPSMiddlewareFunction.hpp for details. */
-  void addMiddleware(const HTTPSMiddlewareFunction * mwFunction);
+  void addMiddleware(const HTTPSMiddlewareFunction mwFunction);
+  void addMiddleware(void (*mwFunction)(HTTPRequest *req, HTTPResponse *res, std::function<void()> next));
   /** Remove a specific function from the middleware function chain. */
-  void removeMiddleware(const HTTPSMiddlewareFunction * mwFunction);
+  void removeMiddleware(const HTTPSMiddlewareFunction mwFunction);
+  void removeMiddleware(void (*mwFunction)(HTTPRequest * req, HTTPResponse * res, std::function<void()> next));
   /** Get the current middleware chain with a resource function at the end */
-  const std::vector<HTTPSMiddlewareFunction*> getMiddleware();
+  const std::vector<HTTPSMiddlewareFunction> getMiddleware();
 
 private:
 
@@ -43,7 +45,7 @@ private:
   HTTPNode * _defaultNode;
 
   // Middleware functions, if any are registered. Will be called in order of the vector.
-  std::vector<const HTTPSMiddlewareFunction*> _middleware;
+  std::vector<HTTPSMiddlewareFunction> _middleware;
 };
 
 } /* namespace httpsserver */

Copy link

@Regela Regela left a comment

Choose a reason for hiding this comment

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

It looks good

@Regela
Copy link

Regela commented May 7, 2021

That about other callbacks? For example,

typedef WebsocketHandler* (WebsocketHandlerCreator)();

@flohoss
Copy link

flohoss commented Apr 18, 2022

Hi, need functional callbacks for a project, PR is fixing it. Is this going to be merged any time soon? Awesome Work! Got the server up in no time.

@trullock
Copy link

Whats the chances of getting this moved along? Really need this

Happy to help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a ResourceNode for a method within a class
6 participants