changeset 978:ce3106e5843f

refactoring
author Sebastien Jodogne <s.jodogne@gmail.com>
date Mon, 30 Jun 2014 16:04:58 +0200
parents c550e99c452b
children 624f44047238 ef02bd1c2f0e
files Core/RestApi/RestApiCall.h Core/RestApi/RestApiHierarchy.cpp Core/RestApi/RestApiHierarchy.h NEWS UnitTestsSources/RestApiTests.cpp
diffstat 5 files changed, 208 insertions(+), 196 deletions(-) [+]
line wrap: on
line diff
--- a/Core/RestApi/RestApiCall.h	Mon Jun 30 14:53:18 2014 +0200
+++ b/Core/RestApi/RestApiCall.h	Mon Jun 30 16:04:58 2014 +0200
@@ -36,11 +36,13 @@
 #include "RestApiPath.h"
 #include "RestApiOutput.h"
 
+#include <boost/noncopyable.hpp>
+
 namespace Orthanc
 {
   class RestApi;
 
-  class RestApiCall
+  class RestApiCall : public boost::noncopyable
   {
   private:
     RestApiOutput& output_;
--- a/Core/RestApi/RestApiHierarchy.cpp	Mon Jun 30 14:53:18 2014 +0200
+++ b/Core/RestApi/RestApiHierarchy.cpp	Mon Jun 30 16:04:58 2014 +0200
@@ -38,7 +38,7 @@
 
 namespace Orthanc
 {
-  RestApiHierarchy::Handlers::Handlers() : 
+  RestApiHierarchy::Resource::Resource() : 
     getHandler_(NULL), 
     postHandler_(NULL),
     putHandler_(NULL), 
@@ -47,7 +47,7 @@
   }
 
 
-  bool RestApiHierarchy::Handlers::HasHandler(HttpMethod method) const
+  bool RestApiHierarchy::Resource::HasHandler(HttpMethod method) const
   {
     switch (method)
     {
@@ -69,7 +69,7 @@
   }
 
 
-  bool RestApiHierarchy::Handlers::IsEmpty() const
+  bool RestApiHierarchy::Resource::IsEmpty() const
   {
     return (getHandler_ == NULL &&
             postHandler_ == NULL &&
@@ -78,31 +78,6 @@
   }
 
 
-  RestApiGetCall::Handler RestApiHierarchy::Handlers::GetGetHandler() const
-  {
-    assert(getHandler_ != NULL);
-    return getHandler_;
-  }
-
-  RestApiPutCall::Handler RestApiHierarchy::Handlers::GetPutHandler() const
-  {
-    assert(putHandler_ != NULL);
-    return putHandler_;
-  }
-
-  RestApiPostCall::Handler RestApiHierarchy::Handlers::GetPostHandler() const
-  {
-    assert(postHandler_ != NULL);
-    return postHandler_;
-  }
-
-  RestApiDeleteCall::Handler RestApiHierarchy::Handlers::GetDeleteHandler() const
-  {
-    assert(deleteHandler_ != NULL);
-    return deleteHandler_;
-  }
-
-
   RestApiHierarchy& RestApiHierarchy::AddChild(Children& children,
                                                const std::string& name)
   {
@@ -122,6 +97,64 @@
   }
 
 
+
+  bool RestApiHierarchy::Resource::Handle(RestApiGetCall& call) const
+  {
+    if (getHandler_ != NULL)
+    {
+      getHandler_(call);
+      return true;
+    }
+    else
+    {
+      return false;
+    }
+  }
+
+
+  bool RestApiHierarchy::Resource::Handle(RestApiPutCall& call) const
+  {
+    if (putHandler_ != NULL)
+    {
+      putHandler_(call);
+      return true;
+    }
+    else
+    {
+      return false;
+    }
+  }
+
+
+  bool RestApiHierarchy::Resource::Handle(RestApiPostCall& call) const
+  {
+    if (postHandler_ != NULL)
+    {
+      postHandler_(call);
+      return true;
+    }
+    else
+    {
+      return false;
+    }
+  }
+
+
+  bool RestApiHierarchy::Resource::Handle(RestApiDeleteCall& call) const
+  {
+    if (deleteHandler_ != NULL)
+    {
+      deleteHandler_(call);
+      return true;
+    }
+    else
+    {
+      return false;
+    }
+  }
+
+
+
   void RestApiHierarchy::DeleteChildren(Children& children)
   {
     for (Children::iterator it = children.begin();
@@ -165,11 +198,10 @@
   }
 
 
-  bool RestApiHierarchy::LookupHandler(HttpHandler::Arguments& components,
+  bool RestApiHierarchy::LookupResource(HttpHandler::Arguments& components,
                                        const UriComponents& uri,
-                                       ResourceCallback callback,
-                                       size_t level,
-                                       void* call)
+                                       IVisitor& visitor,
+                                       size_t level)
   {
     assert(uri.size() >= level);
     UriComponents trailing;
@@ -178,7 +210,7 @@
     if (uri.size() == level)
     {
       if (!handlers_.IsEmpty() &&
-          callback(handlers_, uri, components, trailing, call))
+          visitor.Visit(handlers_, uri, components, trailing))
       {
         return true;
       }
@@ -189,7 +221,7 @@
     Children::const_iterator child = children_.find(uri[level]);
     if (child != children_.end())
     {
-      if (child->second->LookupHandler(components, uri, callback, level + 1, call))
+      if (child->second->LookupResource(components, uri, visitor, level + 1))
       {
         return true;
       }
@@ -203,7 +235,7 @@
       HttpHandler::Arguments subComponents = components;
       subComponents[child->first] = uri[level];
 
-      if (child->second->LookupHandler(components, uri, callback, level + 1, call))
+      if (child->second->LookupResource(components, uri, visitor, level + 1))
       {
         return true;
       }        
@@ -222,7 +254,7 @@
 
       assert(pos == trailing.size());
 
-      if (callback(universalHandlers_, uri, components, trailing, call))
+      if (visitor.Visit(universalHandlers_, uri, components, trailing))
       {
         return true;
       }
@@ -232,15 +264,21 @@
   }
 
 
+  bool RestApiHierarchy::CanGenerateDirectory() const
+  {
+    return (!handlers_.HasHandler(HttpMethod_Get) && 
+            universalHandlers_.IsEmpty() &&
+            wildcardChildren_.size() == 0);
+  }
+
+
   bool RestApiHierarchy::GetDirectory(Json::Value& result,
                                       const UriComponents& uri,
                                       size_t level)
   {
     if (uri.size() == level)
     {
-      if (!handlers_.HasHandler(HttpMethod_Get) && 
-          universalHandlers_.IsEmpty() &&
-          wildcardChildren_.size() == 0)
+      if (CanGenerateDirectory())
       {
         result = Json::arrayValue;
 
@@ -280,78 +318,6 @@
   }
                        
 
-  bool RestApiHierarchy::GetCallback(Handlers& handlers,
-                                     const UriComponents& uri,
-                                     const HttpHandler::Arguments& components,
-                                     const UriComponents& trailing,
-                                     void* call)
-  {
-    if (handlers.HasHandler(HttpMethod_Get))
-    {
-      handlers.GetGetHandler() (*reinterpret_cast<RestApiGetCall*>(call));
-      return true;
-    }
-    else
-    {
-      return false;
-    }
-  }
-
-
-  bool RestApiHierarchy::PostCallback(Handlers& handlers,
-                                      const UriComponents& uri,
-                                      const HttpHandler::Arguments& components,
-                                      const UriComponents& trailing,
-                                      void* call)
-  {
-    if (handlers.HasHandler(HttpMethod_Post))
-    {
-      handlers.GetPostHandler() (*reinterpret_cast<RestApiPostCall*>(call));
-      return true;
-    }
-    else
-    {
-      return false;
-    }
-  }
-
-
-  bool RestApiHierarchy::PutCallback(Handlers& handlers,
-                                     const UriComponents& uri,
-                                     const HttpHandler::Arguments& components,
-                                     const UriComponents& trailing,
-                                     void* call)
-  {
-    if (handlers.HasHandler(HttpMethod_Put))
-    {
-      handlers.GetPutHandler() (*reinterpret_cast<RestApiPutCall*>(call));
-      return true;
-    }
-    else
-    {
-      return false;
-    }
-  }
-
-
-  bool RestApiHierarchy::DeleteCallback(Handlers& handlers,
-                                        const UriComponents& uri,
-                                        const HttpHandler::Arguments& components,
-                                        const UriComponents& trailing,
-                                        void* call)
-  {
-    if (handlers.HasHandler(HttpMethod_Delete))
-    {
-      handlers.GetDeleteHandler() (*reinterpret_cast<RestApiDeleteCall*>(call));
-      return true;
-    }
-    else
-    {
-      return false;
-    }
-  }
-
-
   RestApiHierarchy::~RestApiHierarchy()
   {
     DeleteChildren(children_);
@@ -390,7 +356,7 @@
   {
     if (children_.size() == 0)
     {
-      std::string s = " ";
+      /*std::string s = " ";
       if (handlers_.HasHandler(HttpMethod_Get))
       {
         s += "GET ";
@@ -411,7 +377,9 @@
         s += "DELETE ";
       }
 
-      target = s;
+      target = s;*/
+
+      target = Json::objectValue;
     }
     else
     {
@@ -431,32 +399,74 @@
       }*/
   }
 
-  bool RestApiHierarchy::Handle(RestApiGetCall& call,
-                                const UriComponents& uri)
+
+  bool RestApiHierarchy::LookupResource(const UriComponents& uri,
+                                        IVisitor& visitor)
   {
     HttpHandler::Arguments components;
-    return LookupHandler(components, uri, GetCallback, 0, &call);
-  }    
-
-  bool RestApiHierarchy::Handle(RestApiPutCall& call,
-                                const UriComponents& uri)
-  {
-    HttpHandler::Arguments components;
-    return LookupHandler(components, uri, PutCallback, 0, &call);
+    return LookupResource(components, uri, visitor, 0);
   }    
 
-  bool RestApiHierarchy::Handle(RestApiPostCall& call,
-                                const UriComponents& uri)
+
+
+  namespace
+  {
+    // Anonymous namespace to avoid clashes between compilation modules
+
+    class AcceptedMethodsVisitor : public RestApiHierarchy::IVisitor
+    {
+    private:
+      std::set<HttpMethod>& methods_;
+
+    public:
+      AcceptedMethodsVisitor(std::set<HttpMethod>& methods) : methods_(methods)
+      {
+      }
+
+      virtual bool Visit(const RestApiHierarchy::Resource& resource,
+                         const UriComponents& uri,
+                         const HttpHandler::Arguments& components,
+                         const UriComponents& trailing)
+      {
+        if (trailing.size() == 0)  // Ignore universal handlers
+        {
+          if (resource.HasHandler(HttpMethod_Get))
+          {
+            methods_.insert(HttpMethod_Get);
+          }
+
+          if (resource.HasHandler(HttpMethod_Post))
+          {
+            methods_.insert(HttpMethod_Post);
+          }
+
+          if (resource.HasHandler(HttpMethod_Put))
+          {
+            methods_.insert(HttpMethod_Put);
+          }
+
+          if (resource.HasHandler(HttpMethod_Delete))
+          {
+            methods_.insert(HttpMethod_Delete);
+          }
+        }
+
+        return false;  // Continue to check all the possible ways to access this URI
+      }
+    };
+  }
+
+  void RestApiHierarchy::GetAcceptedMethods(std::set<HttpMethod>& methods,
+                                            const UriComponents& uri)
   {
     HttpHandler::Arguments components;
-    return LookupHandler(components, uri, PostCallback, 0, &call);
-  }    
+    AcceptedMethodsVisitor visitor(methods);
+    LookupResource(components, uri, visitor, 0);
 
-  bool RestApiHierarchy::Handle(RestApiDeleteCall& call,
-                                const UriComponents& uri)
-  {
-    HttpHandler::Arguments components;
-    return LookupHandler(components, uri, DeleteCallback, 0, &call);
-  }    
-
+    Json::Value d;
+    if (GetDirectory(d, uri))
+    {
+      methods.insert(HttpMethod_Get);
+    }
+  }
 }
--- a/Core/RestApi/RestApiHierarchy.h	Mon Jun 30 14:53:18 2014 +0200
+++ b/Core/RestApi/RestApiHierarchy.h	Mon Jun 30 16:04:58 2014 +0200
@@ -37,12 +37,14 @@
 #include "RestApiPutCall.h"
 #include "RestApiDeleteCall.h"
 
+#include <set>
+
 namespace Orthanc
 {
-  class RestApiHierarchy
+  class RestApiHierarchy : public boost::noncopyable
   {
-  private:
-    class Handlers
+  public:
+    class Resource : public boost::noncopyable
     {
     private:
       RestApiGetCall::Handler     getHandler_;
@@ -51,18 +53,10 @@
       RestApiDeleteCall::Handler  deleteHandler_;
 
     public:
-      Handlers();
+      Resource();
 
       bool HasHandler(HttpMethod method) const;
 
-      RestApiGetCall::Handler GetGetHandler() const;
-
-      RestApiPutCall::Handler GetPutHandler() const;
-
-      RestApiPostCall::Handler GetPostHandler() const;
-
-      RestApiDeleteCall::Handler GetDeleteHandler() const;
-
       void Register(RestApiGetCall::Handler handler)
       {
         getHandler_ = handler;
@@ -84,21 +78,38 @@
       }
 
       bool IsEmpty() const;
+
+      bool Handle(RestApiGetCall& call) const;
+
+      bool Handle(RestApiPutCall& call) const;
+
+      bool Handle(RestApiPostCall& call) const;
+
+      bool Handle(RestApiDeleteCall& call) const;
     };
 
 
+    class IVisitor : public boost::noncopyable
+    {
+    public:
+      virtual ~IVisitor()
+      {
+      }
+
+      virtual bool Visit(const Resource& resource,
+                         const UriComponents& uri,
+                         const HttpHandler::Arguments& components,
+                         const UriComponents& trailing) = 0;
+    };
+
+
+  private:
     typedef std::map<std::string, RestApiHierarchy*>  Children;
-    typedef bool (*ResourceCallback) (Handlers& handlers,
-                                      const UriComponents& uri,
-                                      const HttpHandler::Arguments& components,
-                                      const UriComponents& trailing,
-                                      void* call);
 
-    Handlers  handlers_;
+    Resource  handlers_;
     Children  children_;
     Children  wildcardChildren_;
-    Handlers  universalHandlers_;
-
+    Resource  universalHandlers_;
 
     static RestApiHierarchy& AddChild(Children& children,
                                       const std::string& name);
@@ -110,40 +121,16 @@
                           Handler handler,
                           size_t level);
 
-    bool LookupHandler(HttpHandler::Arguments& components,
-                       const UriComponents& uri,
-                       ResourceCallback callback,
-                       size_t level,
-                       void* call);
+    bool CanGenerateDirectory() const;
+
+    bool LookupResource(HttpHandler::Arguments& components,
+                        const UriComponents& uri,
+                        IVisitor& visitor,
+                        size_t level);
 
     bool GetDirectory(Json::Value& result,
                       const UriComponents& uri,
                       size_t level);
-                     
-    static bool GetCallback(Handlers& handlers,
-                            const UriComponents& uri,
-                            const HttpHandler::Arguments& components,
-                            const UriComponents& trailing,
-                            void* call);
-
-    static bool PostCallback(Handlers& handlers,
-                             const UriComponents& uri,
-                             const HttpHandler::Arguments& components,
-                             const UriComponents& trailing,
-                             void* call);
-
-    static bool PutCallback(Handlers& handlers,
-                            const UriComponents& uri,
-                            const HttpHandler::Arguments& components,
-                            const UriComponents& trailing,
-                            void* call);
-
-    static bool DeleteCallback(Handlers& handlers,
-                               const UriComponents& uri,
-                               const HttpHandler::Arguments& components,
-                               const UriComponents& trailing,
-                               void* call);
-                       
 
   public:
     ~RestApiHierarchy();
@@ -168,16 +155,10 @@
       return GetDirectory(result, uri, 0);
     }
 
-    bool Handle(RestApiGetCall& call,
-                const UriComponents& uri);
-
-    bool Handle(RestApiPutCall& call,
-                const UriComponents& uri);
+    bool LookupResource(const UriComponents& uri,
+                        IVisitor& visitor);
 
-    bool Handle(RestApiPostCall& call,
-                const UriComponents& uri);
-
-    bool Handle(RestApiDeleteCall& call,
-                const UriComponents& uri);
+    void GetAcceptedMethods(std::set<HttpMethod>& methods,
+                            const UriComponents& uri);
   };
 }
--- a/NEWS	Mon Jun 30 14:53:18 2014 +0200
+++ b/NEWS	Mon Jun 30 16:04:58 2014 +0200
@@ -6,6 +6,7 @@
 * Extraction of the tags shared by all the instances of a patient/study/series
 * Options to limit the number of results for an incoming C-FIND query
 * Support of kFreeBSD
+* Several code refactorings
 
 
 Version 0.7.6 (2014/06/11)
--- a/UnitTestsSources/RestApiTests.cpp	Mon Jun 30 14:53:18 2014 +0200
+++ b/UnitTestsSources/RestApiTests.cpp	Mon Jun 30 16:04:58 2014 +0200
@@ -212,12 +212,30 @@
 }
 
 
+
+namespace
+{
+  class MyVisitor : public RestApiHierarchy::IVisitor
+  {
+  public:
+    virtual bool Visit(const RestApiHierarchy::Resource& resource,
+                       const UriComponents& uri,
+                       const HttpHandler::Arguments& components,
+                       const UriComponents& trailing)
+    {
+      return resource.Handle(*reinterpret_cast<RestApiGetCall*>(NULL));
+    }
+  };
+}
+
+
 static bool HandleGet(RestApiHierarchy& hierarchy, 
                       const std::string& uri)
 {
   UriComponents p;
   Toolbox::SplitUriComponents(p, uri);
-  return hierarchy.Handle(*reinterpret_cast<RestApiGetCall*>(NULL), p);
+  MyVisitor visitor;
+  return hierarchy.LookupResource(p, visitor);
 }