Removed add/prepend. Use append only since we don't have priority imp…

…lemented yet

iocore/net/test_I_UDPNet.cc

@@ -300,7 +300,7 @@ Log::trace_out(sockaddr const *, unsigned short, char const *, ...)
 
 #include "InkAPIInternal.h"
 int
-APIHook::invoke(int, void *)
+APIHook::invoke(int, void *) const
 {
   ink_assert(false);
   return 0;
@@ -314,7 +314,14 @@ APIHook::next() const
 }
 
 APIHook *
-APIHooks::get() const
+APIHook::prev() const
+{
+  ink_assert(false);
+  return nullptr;
+}
+
+APIHook *
+APIHooks::head() const
 {
   ink_assert(false);
   return nullptr;

proxy/InkAPIInternal.h

@@ -31,6 +31,7 @@
 #include "ProxyConfig.h"
 #include "P_Cache.h"
 #include "I_Tasks.h"
+#include "Plugin.h"
 
 #include "ts/InkAPIPrivateIOCore.h"
 #include "ts/experimental.h"
@@ -121,19 +122,22 @@ class APIHook
 {
 public:
   INKContInternal *m_cont;
-  int invoke(int event, void *edata);
+  int invoke(int event, void *edata) const;
   APIHook *next() const;
+  APIHook *prev() const;
   LINK(APIHook, m_link);
 };
 
 /// A collection of API hooks.
 class APIHooks
 {
 public:
-  void prepend(INKContInternal *cont);
   void append(INKContInternal *cont);
-  APIHook *get() const;
+  /// Get the first hook.
+  APIHook *head() const;
+  /// Remove all hooks.
   void clear();
+  /// Check if there are no hooks.
   bool is_empty() const;
 
 private:
@@ -166,8 +170,6 @@ class FeatureAPIHooks
 
   /// Remove all hooks.
   void clear();
-  /// Add the hook @a cont to the front of the hooks for @a id.
-  void prepend(ID id, INKContInternal *cont);
   /// Add the hook @a cont to the end of the hooks for @a id.
   void append(ID id, INKContInternal *cont);
   /// Get the list of hooks for @a id.
@@ -188,13 +190,16 @@ class FeatureAPIHooks
   /// @return @c true if any hooks of type @a id are present.
   bool has_hooks_for(ID id) const;
 
+  /// Get a pointer to the set of hooks for a specific hook @id
+  APIHooks const *operator[](ID id) const;
+
 private:
-  bool hooks_p; ///< Flag for (not) empty container.
+  bool m_hooks_p = false; ///< Flag for (not) empty container.
   /// The array of hooks lists.
   APIHooks m_hooks[N];
 };
 
-template <typename ID, ID N> FeatureAPIHooks<ID, N>::FeatureAPIHooks() : hooks_p(false) {}
+template <typename ID, ID N> FeatureAPIHooks<ID, N>::FeatureAPIHooks() {}
 
 template <typename ID, ID N> FeatureAPIHooks<ID, N>::~FeatureAPIHooks()
 {
@@ -205,28 +210,18 @@ template <typename ID, ID N>
 void
 FeatureAPIHooks<ID, N>::clear()
 {
-  for (int i = 0; i < N; ++i) {
-    m_hooks[i].clear();
-  }
-  hooks_p = false;
-}
-
-template <typename ID, ID N>
-void
-FeatureAPIHooks<ID, N>::prepend(ID id, INKContInternal *cont)
-{
-  if (likely(is_valid(id))) {
-    hooks_p = true;
-    m_hooks[id].prepend(cont);
+  for (auto &h : m_hooks) {
+    h.clear();
   }
+  m_hooks_p = false;
 }
 
 template <typename ID, ID N>
 void
 FeatureAPIHooks<ID, N>::append(ID id, INKContInternal *cont)
 {
-  if (likely(is_valid(id))) {
-    hooks_p = true;
+  if (is_valid(id)) {
+    m_hooks_p = true;
     m_hooks[id].append(cont);
   }
 }
@@ -235,7 +230,12 @@ template <typename ID, ID N>
 APIHook *
 FeatureAPIHooks<ID, N>::get(ID id) const
 {
-  return likely(is_valid(id)) ? m_hooks[id].get() : nullptr;
+  return likely(is_valid(id)) ? m_hooks[id].head() : nullptr;
+}
+
+template <typename ID, ID N> APIHooks const *FeatureAPIHooks<ID, N>::operator[](ID id) const
+{
+  return likely(is_valid(id)) ? &(m_hooks[id]) : nullptr;
 }
 
 template <typename ID, ID N>
@@ -251,7 +251,7 @@ template <typename ID, ID N>
 bool
 FeatureAPIHooks<ID, N>::has_hooks() const
 {
-  return hooks_p;
+  return m_hooks_p;
 }
 
 template <typename ID, ID N>
@@ -333,6 +333,59 @@ class ConfigUpdateCbTable
   std::unordered_map<std::string, INKContInternal *> cb_table;
 };
 
+class HttpHookState
+{
+public:
+  /// Scope tags for interacting with a live instance.
+  enum ScopeTag { GLOBAL, SSN, TXN };
+
+  /// Default Constructor
+  HttpHookState();
+
+  /// Initialize the hook state to track up to 3 sources of hooks.
+  /// The argument order to this method is used to break priority ties (callbacks from earlier args are invoked earlier)
+  /// The order in terms of @a ScopeTag is GLOBAL, SESSION, TRANSACTION.
+  void init(TSHttpHookID id, HttpAPIHooks const *global, HttpAPIHooks const *ssn = nullptr, HttpAPIHooks const *txn = nullptr);
+
+  /// Select a hook for invocation and advance the state to the next valid hook
+  /// @return nullptr if no current hook.
+  APIHook const *getNext();
+
+  /// Get the hook ID
+  TSHttpHookID id() const;
+
+  /// Temporary function to return true. Later will be used to decide if a plugin is enabled for the hooks
+  bool is_enabled();
+
+protected:
+  /// Track the state of one scope of hooks.
+  struct Scope {
+    APIHook const *_c; ///< Current hook (candidate for invocation).
+    APIHook const *_p; ///< Previous hook (already invoked).
+
+    /// Initialize the scope.
+    void init(HttpAPIHooks const *scope, TSHttpHookID id);
+    /// Clear the scope.
+    void clear();
+    /// Return the current candidate.
+    APIHook const *candidate();
+    /// Advance state to the next hook.
+    void operator++();
+  };
+
+private:
+  TSHttpHookID _id;
+  Scope _global; ///< Chain from global hooks.
+  Scope _ssn;    ///< Chain from session hooks.
+  Scope _txn;    ///< Chain from transaction hooks.
+};
+
+inline TSHttpHookID
+HttpHookState::id() const
+{
+  return _id;
+}
+
 void api_init();
 
 extern HttpAPIHooks *http_global_hooks;

proxy/ProxyClientSession.cc

@@ -77,12 +77,6 @@ static const TSEvent eventmap[TS_HTTP_LAST_HOOK + 1] = {
   TS_EVENT_NONE,                       // TS_HTTP_LAST_HOOK
 };
 
-static bool
-is_valid_hook(TSHttpHookID hookid)
-{
-  return (hookid >= 0) && (hookid < TS_HTTP_LAST_HOOK);
-}
-
 void
 ProxyClientSession::free()
 {
@@ -107,34 +101,28 @@ ProxyClientSession::state_api_callout(int event, void *data)
   case EVENT_NONE:
   case EVENT_INTERVAL:
   case TS_EVENT_HTTP_CONTINUE:
-    if (likely(is_valid_hook(this->api_hookid))) {
-      if (this->api_current == nullptr && this->api_scope == API_HOOK_SCOPE_GLOBAL) {
-        this->api_current = http_global_hooks->get(this->api_hookid);
-        this->api_scope   = API_HOOK_SCOPE_LOCAL;
-      }
+    if (nullptr == cur_hook) {
+      /// Get the next hook to invoke from HttpHookState
+      cur_hook = hook_state.getNext();
+    }
+    if (nullptr != cur_hook) {
+      APIHook const *hook = cur_hook;
 
-      if (this->api_current == nullptr && this->api_scope == API_HOOK_SCOPE_LOCAL) {
-        this->api_current = ssn_hook_get(this->api_hookid);
-        this->api_scope   = API_HOOK_SCOPE_NONE;
-      }
+      MUTEX_TRY_LOCK(lock, hook->m_cont->mutex, mutex->thread_holding);
 
-      if (this->api_current) {
-        APIHook *hook = this->api_current;
-
-        MUTEX_TRY_LOCK(lock, hook->m_cont->mutex, mutex->thread_holding);
-        // Have a mutex but did't get the lock, reschedule
-        if (!lock.is_locked()) {
-          SET_HANDLER(&ProxyClientSession::state_api_callout);
-          if (!schedule_event) { // Don't bother to schedule is there is already one out.
-            schedule_event = mutex->thread_holding->schedule_in(this, HRTIME_MSECONDS(10));
-          }
-          return 0;
+      // Have a mutex but didn't get the lock, reschedule
+      if (!lock.is_locked()) {
+        SET_HANDLER(&ProxyClientSession::state_api_callout);
+        if (!schedule_event) { // Don't bother if there is already one
+          schedule_event = mutex->thread_holding->schedule_in(this, HRTIME_MSECONDS(10));
         }
-
-        this->api_current = this->api_current->next();
-        hook->invoke(eventmap[this->api_hookid], this);
         return 0;
       }
+
+      cur_hook = nullptr; // mark current callback at dispatched.
+      hook->invoke(eventmap[hook_state.id()], this);
+
+      return 0;
     }
 
     handle_api_return(event);
@@ -156,12 +144,10 @@ void
 ProxyClientSession::do_api_callout(TSHttpHookID id)
 {
   ink_assert(id == TS_HTTP_SSN_START_HOOK || id == TS_HTTP_SSN_CLOSE_HOOK);
-
-  this->api_hookid  = id;
-  this->api_scope   = API_HOOK_SCOPE_GLOBAL;
-  this->api_current = nullptr;
-
-  if (this->has_hooks()) {
+  hook_state.init(id, http_global_hooks, &api_hooks);
+  /// Verify if there is any hook to invoke
+  cur_hook = hook_state.getNext();
+  if (nullptr != cur_hook) {
     SET_HANDLER(&ProxyClientSession::state_api_callout);
     this->state_api_callout(EVENT_NONE, nullptr);
   } else {
@@ -172,13 +158,11 @@ ProxyClientSession::do_api_callout(TSHttpHookID id)
 void
 ProxyClientSession::handle_api_return(int event)
 {
-  TSHttpHookID hookid = this->api_hookid;
+  TSHttpHookID hookid = hook_state.id();
 
   SET_HANDLER(&ProxyClientSession::state_api_callout);
 
-  this->api_hookid  = TS_HTTP_LAST_HOOK;
-  this->api_scope   = API_HOOK_SCOPE_NONE;
-  this->api_current = nullptr;
+  cur_hook = nullptr;
 
   switch (hookid) {
   case TS_HTTP_SSN_START_HOOK:

proxy/ProxyClientSession.h

@@ -90,19 +90,13 @@ class ProxyClientSession : public VConnection
   virtual const char *get_protocol_string() const = 0;
 
   virtual void
-  ssn_hook_append(TSHttpHookID id, INKContInternal *cont)
+  hook_add(TSHttpHookID id, INKContInternal *cont)
   {
     this->api_hooks.append(id, cont);
   }
 
-  virtual void
-  ssn_hook_prepend(TSHttpHookID id, INKContInternal *cont)
-  {
-    this->api_hooks.prepend(id, cont);
-  }
-
   APIHook *
-  ssn_hook_get(TSHttpHookID id) const
+  hook_get(TSHttpHookID id) const
   {
     return this->api_hooks.get(id);
   }
@@ -152,6 +146,11 @@ class ProxyClientSession : public VConnection
     return ts_is_draining;
   }
 
+  HttpAPIHooks const *
+  feature_hooks() const
+  {
+    return &api_hooks;
+  }
   // Initiate an API hook invocation.
   void do_api_callout(TSHttpHookID id);
 
@@ -221,7 +220,7 @@ class ProxyClientSession : public VConnection
   TSHttpHookID
   get_hookid() const
   {
-    return api_hookid;
+    return hook_state.id();
   }
 
   virtual void
@@ -300,6 +299,9 @@ class ProxyClientSession : public VConnection
   ProxyClientSession &operator=(const ProxyClientSession &) = delete;
 
 protected:
+  // Hook dispatching state
+  HttpHookState hook_state;
+
   // XXX Consider using a bitwise flags variable for the following flags, so
   // that we can make the best use of internal alignment padding.
 
@@ -314,9 +316,7 @@ class ProxyClientSession : public VConnection
   void handle_api_return(int event);
   int state_api_callout(int event, void *edata);
 
-  APIHookScope api_scope  = API_HOOK_SCOPE_NONE;
-  TSHttpHookID api_hookid = TS_HTTP_READ_REQUEST_HDR_HOOK;
-  APIHook *api_current    = nullptr;
+  APIHook const *cur_hook = nullptr;
   HttpAPIHooks api_hooks;
   void *user_args[TS_HTTP_MAX_USER_ARG];
 

proxy/ProxyClientTransaction.h

@@ -100,9 +100,15 @@ class ProxyClientTransaction : public VConnection
   }
 
   APIHook *
-  ssn_hook_get(TSHttpHookID id) const
+  hook_get(TSHttpHookID id) const
   {
-    return parent ? parent->ssn_hook_get(id) : nullptr;
+    return parent ? parent->hook_get(id) : nullptr;
+  }
+
+  HttpAPIHooks const *
+  feature_hooks() const
+  {
+    return parent ? parent->feature_hooks() : nullptr;
   }
 
   bool

proxy/Transform.cc

@@ -546,7 +546,7 @@ TransformControl::handle_event(int event, void * /* edata ATS_UNUSED */)
     if (http_global_hooks && http_global_hooks->get(TS_HTTP_RESPONSE_TRANSFORM_HOOK)) {
       m_tvc = transformProcessor.open(this, http_global_hooks->get(TS_HTTP_RESPONSE_TRANSFORM_HOOK));
     } else {
-      m_tvc = transformProcessor.open(this, m_hooks.get());
+      m_tvc = transformProcessor.open(this, m_hooks.head());
     }
     ink_assert(m_tvc != nullptr);