Merge pull request #464 from leapmotion/fix-autorequired-parentinject

Fix autorequired parentinject

autowiring/CoreContext.h

@@ -161,7 +161,8 @@ class CoreContext:
   /// </summary>
   struct MemoEntry {
     MemoEntry(void) :
-      pFirst(nullptr)
+      pFirst(nullptr),
+      m_local(true)
     {}
 
     // The first deferrable autowiring which requires this type, if one exists:
@@ -174,6 +175,9 @@ class CoreContext:
     // Once this memo entry is satisfied, this will contain the AnySharedPointer instance that performs
     // the satisfaction
     AnySharedPointer m_value;
+
+    // A flag to determine if this memo entry was set from the current context.
+    bool m_local;
   };
 
 protected:
@@ -345,7 +349,7 @@ class CoreContext:
   /// <summary>
   /// Updates all deferred autowiring fields, generally called after a new member has been added
   /// </summary>
-  void UpdateDeferredElements(std::unique_lock<std::mutex>&& lk, const CoreObjectDescriptor& entry);
+  void UpdateDeferredElements(std::unique_lock<std::mutex>&& lk, const CoreObjectDescriptor& entry, bool local);
 
   /// \internal
   /// <summary>
@@ -432,13 +436,13 @@ class CoreContext:
   /// The memo entry where this type was found
   /// </returns>
   /// <param name="reference">An initialized shared pointer slot which may be used in type detection</param>
-  void FindByType(AnySharedPointer& reference) const;
+  void FindByType(AnySharedPointer& reference, bool localOnly = false) const;
 
   /// \internal
   /// <summary>
   /// Unsynchronized version of FindByType
   /// </summary>
-  MemoEntry& FindByTypeUnsafe(AnySharedPointer& reference) const;
+  MemoEntry& FindByTypeUnsafe(AnySharedPointer& reference, bool localOnly = false) const;
 
   /// \internal
   /// <summary>
@@ -695,7 +699,7 @@ class CoreContext:
     // member does not inherit CoreObject and this member is eventually satisfied by one that does.
     {
       std::shared_ptr<T> pure;
-      FindByType(pure);
+      FindByType(pure, true); //skip non-local slots.
       if (pure)
         return pure;
     }
@@ -1047,9 +1051,9 @@ class CoreContext:
   /// Locates an available context member in this context
   /// </summary>
   template<class T>
-  void FindByType(std::shared_ptr<T>& slot) const {
+  void FindByType(std::shared_ptr<T>& slot, bool localOnly = false) const {
     AnySharedPointerT<T> reference;
-    FindByType(reference);
+    FindByType(reference, localOnly);
     slot = reference.slot()->template as<T>();
   }
 

src/autowiring/CoreContext.cpp

@@ -245,14 +245,21 @@ void CoreContext::AddInternal(const CoreObjectDescriptor& traits) {
     // the value, rather than the type passed in via traits.type, because the proper type might be a
     // concrete type defined in another context or potentially a unifier type.  Creating a slot here
     // is also undesirable because the complete type is not available and we can't create a dynaimc
-    // caster to identify when this slot gets satisfied.
+    // caster to identify when this slot gets satisfied. If a slot was non-local, overwrite it.
     auto q = m_typeMemos.find(*traits.actual_type);
     if(q != m_typeMemos.end()) {
       auto& v = q->second;
-      if(traits.pCoreObject && *v.m_value == traits.pCoreObject)
-        throw std::runtime_error("An attempt was made to add the same value to the same context more than once");
-      if(*v.m_value)
-        throw std::runtime_error("An attempt was made to add the same type to the same context more than once");
+
+      if (v.m_local) {
+        if (traits.pCoreObject && *v.m_value == traits.pCoreObject)
+          throw std::runtime_error("An attempt was made to add the same value to the same context more than once");
+        if (*v.m_value)
+          throw std::runtime_error("An attempt was made to add the same type to the same context more than once");
+      }
+      else {
+        v.m_value = traits.value;
+        v.m_local = true;
+      }
     }
 
     // Add the new concrete type:
@@ -270,7 +277,7 @@ void CoreContext::AddInternal(const CoreObjectDescriptor& traits) {
 
     // Notify any autowiring field that is currently waiting that we have a new member
     // to be considered.
-    UpdateDeferredElements(std::move(lk), m_concreteTypes.back());
+    UpdateDeferredElements(std::move(lk), m_concreteTypes.back(), true);
   }
 
   // Moving this outside the lock because AddCoreRunnable will perform the checks inside its function
@@ -320,19 +327,21 @@ void CoreContext::AddInternal(const AnySharedPointer& ptr) {
   UpdateDeferredElement(std::move(lk), entry);
 }
 
-void CoreContext::FindByType(AnySharedPointer& reference) const {
+void CoreContext::FindByType(AnySharedPointer& reference, bool localOnly) const {
   std::lock_guard<std::mutex> lk(m_stateBlock->m_lock);
-  FindByTypeUnsafe(reference);
+  FindByTypeUnsafe(reference, localOnly);
 }
 
-CoreContext::MemoEntry& CoreContext::FindByTypeUnsafe(AnySharedPointer& reference) const {
+CoreContext::MemoEntry& CoreContext::FindByTypeUnsafe(AnySharedPointer& reference, bool localOnly) const {
   const std::type_info& type = reference->type();
 
   // If we've attempted to search for this type before, we will return the value of the memo immediately:
   auto q = m_typeMemos.find(type);
   if(q != m_typeMemos.end()) {
     // We can copy over and return here
-    reference = q->second.m_value;
+    if (!localOnly || q->second.m_local){
+      reference = q->second.m_value;
+    }
     return q->second;
   }
 
@@ -801,7 +810,7 @@ void CoreContext::UpdateDeferredElement(std::unique_lock<std::mutex>&& lk, MemoE
   lk.unlock();
 }
 
-void CoreContext::UpdateDeferredElements(std::unique_lock<std::mutex>&& lk, const CoreObjectDescriptor& entry) {
+void CoreContext::UpdateDeferredElements(std::unique_lock<std::mutex>&& lk, const CoreObjectDescriptor& entry, bool local) {
   {
     // Collection of items needing finalization:
     std::vector<DeferrableUnsynchronizedStrategy*> delayedFinalize;
@@ -823,8 +832,8 @@ void CoreContext::UpdateDeferredElements(std::unique_lock<std::mutex>&& lk, cons
     for (auto& cur : m_typeMemos) {
       MemoEntry& value = cur.second;
 
-      if (value.m_value)
-        // This entry is already satisfied, no need to process it
+      if (value.m_value && value.m_local)
+        // This entry is already satisfied locally, no need to process it
         continue;
 
       // Determine whether the current candidate element satisfies the autowiring we are considering.
@@ -836,6 +845,9 @@ void CoreContext::UpdateDeferredElements(std::unique_lock<std::mutex>&& lk, cons
       // Success, assign the traits
       value.pObjTraits = &entry;
 
+      // Store if it was injected from the local context or not
+      value.m_local = local;
+
       // Now we need to take on the responsibility of satisfying this deferral.  We will do this by
       // nullifying the flink, and by ensuring that the memo is satisfied at the point where we
       // release the lock.
@@ -882,7 +894,8 @@ void CoreContext::UpdateDeferredElements(std::unique_lock<std::mutex>&& lk, cons
     lk.unlock();
     ctxt->UpdateDeferredElements(
       std::unique_lock<std::mutex>(ctxt->m_stateBlock->m_lock),
-      entry
+      entry,
+      false
     );
     lk.lock();
   }

src/autowiring/test/ScopeTest.cpp

@@ -170,3 +170,109 @@ TEST_F(ScopeTest, AutowiringOrdering) {
   }
 }
 
+class InnerContext {};
+class MiddleContext {};
+TEST_F(ScopeTest, RequireVsWire) {
+  AutoCurrentContext ctxt_outer;
+  auto ctxt_middle = ctxt_outer->Create<MiddleContext>();
+  auto ctxt_inner = ctxt_middle->Create<InnerContext>();
+
+  CurrentContextPusher pshr(ctxt_inner);
+
+  Autowired<A> a_wired_inner;
+  ASSERT_FALSE(a_wired_inner.IsAutowired()) << "Autowired member became autowired too quickly";
+  ASSERT_EQ(a_wired_inner.GetContext(), ctxt_inner) << "Autowired member created in the wrong context";
+
+  AutoRequired<A> a_required_outer(ctxt_outer);
+  ASSERT_TRUE(a_required_outer.IsAutowired()) << "AutoRequired member unsatisfied after construction";
+  ASSERT_EQ(a_required_outer->GetContext(), ctxt_outer) << "AutoRequired member satisfied in the wrong context";
+
+  ASSERT_TRUE(a_wired_inner.IsAutowired()) << "AutoRequired member in parent did not satisfy Autowired member in child";
+  ASSERT_EQ(a_wired_inner.get(), a_required_outer.get()) << "Mismatch between Autowired and Autorequired members";
+
+  //this overrides the slot in the middle context;
+  AutoRequired<A> a_required_middle(ctxt_middle);
+  ASSERT_TRUE(a_required_middle.IsAutowired()) << "AutoRequired member not satisfied!";
+  ASSERT_EQ(a_required_middle->GetContext(), ctxt_middle) << "AutoRequired member not created in child context";
+  ASSERT_NE(a_required_middle, a_required_outer) << "AutoRequired member not constructed in child context";
+
+  Autowired<A> a_wired_inner2(ctxt_inner);
+  ASSERT_TRUE(a_wired_inner2.IsAutowired());
+  ASSERT_EQ(a_wired_inner2.get(), a_required_middle.get()) << "Autowired member did not redirect to the closest context";
+  ASSERT_EQ(a_wired_inner.get(), a_required_outer.get()) << "Autowired object changed after satisfaction!";
+
+  AutoRequired<B> b_required_middle(ctxt_middle);
+  AutoRequired<B> b_required_outer(ctxt_outer);
+  Autowired<B> b_wired_inner;
+  ASSERT_TRUE(b_wired_inner.IsAutowired()) << "Autowired failed to find member";
+  ASSERT_EQ(b_wired_inner.get(), b_required_middle.get()) << "Autorequiring overwrote a slot it shouldn't have!";
+}
+
+TEST_F(ScopeTest, RequireVsWireFast) {
+  AutoCurrentContext ctxt_outer;
+  auto ctxt_middle = ctxt_outer->Create<MiddleContext>();
+  auto ctxt_inner = ctxt_middle->Create<InnerContext>();
+
+  CurrentContextPusher pshr(ctxt_inner);
+
+  //This adds a memo for this type to the inner context
+  AutowiredFast<A> a_wired(ctxt_inner);
+  ASSERT_FALSE(a_wired.IsAutowired()) << "Autowired member became autowired too quickly";
+
+  //This satisfies the memo in the inner context
+  AutoRequired<A> a_required_outer(ctxt_outer);
+  ASSERT_TRUE(a_required_outer.IsAutowired()) << "AutoRequired member unsatisfied after construction";
+  ASSERT_EQ(a_required_outer->GetContext(), ctxt_outer) << "AutoRequired member satisfied in the wrong context";
+
+  ASSERT_FALSE(a_wired.IsAutowired()) << "AutowiredFast was satisfied after construction!";
+
+  //This overrides the memo in the middle context.
+  AutoRequired<A> a_required_middle(ctxt_middle);
+  ASSERT_TRUE(a_required_middle.IsAutowired()) << "AutoRequired member not satisfied!";
+  ASSERT_EQ(a_required_middle->GetContext(), ctxt_middle) << "AutoRequired member not created in child context";
+  ASSERT_NE(a_required_middle.get(), a_required_outer.get()) << "AutoRequired member not constructed in child context";
+
+  //This should direct to the middle context
+  AutowiredFast<A> a_wired2(ctxt_inner);
+  ASSERT_TRUE(a_wired2.IsAutowired());
+  ASSERT_EQ(a_wired2, a_required_middle) << "Autowired member did not grab the most derived object";
+}
+
+class ParentInjector : public ContextMember {
+public:
+  ParentInjector(int v, bool inject = false) : ContextMember("Parent Injector"), value(v) {
+    if (!inject) {
+      return;
+    }
+
+    const auto ctxt = m_context.lock();
+    if (!ctxt) {
+      return;
+    }
+
+    auto parent = ctxt->GetParentContext();
+    if (!parent) {
+      return;
+    }
+
+    parent->Inject<ParentInjector>(9000, false);
+  }
+  int value;
+};
+
+TEST_F(ScopeTest, RecursiveInject) {
+  AutoCurrentContext ctxt_outer;
+  auto ctxt_inner = ctxt_outer->Create<InnerContext>();
+
+  CurrentContextPusher pshr(ctxt_inner);
+
+  AutoRequired<ParentInjector> a_inner(ctxt_inner, 1, true);
+
+  ASSERT_EQ(a_inner->GetContext(), ctxt_inner);
+  ASSERT_EQ(a_inner->value, 1);
+
+  AutoRequired<ParentInjector> a_inner_2(ctxt_inner, 2);
+
+  ASSERT_EQ(a_inner, a_inner_2);
+  ASSERT_EQ(a_inner->GetContext(), a_inner_2->GetContext());
+}