Fixed allocateByType bug, fixed TODO XXX

10 files changed, 86 insertions, 105 deletions

diff --git a/cpp/src/IceGrid/Allocatable.cpp b/cpp/src/IceGrid/Allocatable.cpp
index da88a46db4e..4f7dff85b83 100644
--- a/cpp/src/IceGrid/Allocatable.cpp
+++ b/cpp/src/IceGrid/Allocatable.cpp
@@ -152,12 +152,12 @@ AllocationRequest::AllocationRequest(const SessionIPtr& session) :
 }
 
 Allocatable::Allocatable(bool allocatable, const AllocatablePtr& parent) : 
-    _allocatable(allocatable || parent && parent->allocatable()),
-    _parent((parent && parent->allocatable()) ? parent : AllocatablePtr()),
+    _allocatable(allocatable || parent && parent->isAllocatable()),
+    _parent((parent && parent->isAllocatable()) ? parent : AllocatablePtr()),
     _count(0),
     _releasing(false)
 {
-    assert(!_parent || _parent->allocatable()); // Parent is only set if it's allocatable.
+    assert(!_parent || _parent->isAllocatable()); // Parent is only set if it's allocatable.
 }
 
 Allocatable::~Allocatable()
diff --git a/cpp/src/IceGrid/Allocatable.h b/cpp/src/IceGrid/Allocatable.h
index 8b9ad9c0a61..a9ef7f037c1 100644
--- a/cpp/src/IceGrid/Allocatable.h
+++ b/cpp/src/IceGrid/Allocatable.h
@@ -81,7 +81,7 @@ public:
     virtual bool tryAllocate(const AllocationRequestPtr&, bool = false);
     virtual void release(const SessionIPtr&, bool = false);
 
-    bool allocatable() const { return _allocatable; }
+    bool isAllocatable() const { return _allocatable; }
     SessionIPtr getSession() const;
 
     virtual void allocated(const SessionIPtr&) = 0;
diff --git a/cpp/src/IceGrid/Database.cpp b/cpp/src/IceGrid/Database.cpp
index 0740e6a19b6..ccee67a1a24 100644
--- a/cpp/src/IceGrid/Database.cpp
+++ b/cpp/src/IceGrid/Database.cpp
@@ -1014,13 +1014,6 @@ Database::allocateObjectByType(const string& type, const ObjectAllocationRequest
 }
 
 void
-Database::allocateObjectByTypeOnLeastLoadedNode(const string& type, const ObjectAllocationRequestPtr& request, 
-						LoadSample sample)
-{
-    _objectCache.allocateByTypeOnLeastLoadedNode(type, request, sample);
-}
-
-void
 Database::releaseObject(const Ice::Identity& id, const SessionIPtr& session)
 {
     _objectCache.get(id)->release(session);
diff --git a/cpp/src/IceGrid/Database.h b/cpp/src/IceGrid/Database.h
index ec0fccc8837..e0606b27408 100644
--- a/cpp/src/IceGrid/Database.h
+++ b/cpp/src/IceGrid/Database.h
@@ -91,7 +91,6 @@ public:
 
     void allocateObject(const Ice::Identity&, const ObjectAllocationRequestPtr&);
     void allocateObjectByType(const std::string&, const ObjectAllocationRequestPtr&);
-    void allocateObjectByTypeOnLeastLoadedNode(const std::string&, const ObjectAllocationRequestPtr&, LoadSample);
     void releaseObject(const Ice::Identity&, const SessionIPtr&);
 
     Ice::ObjectPrx getObjectProxy(const Ice::Identity&);
diff --git a/cpp/src/IceGrid/ObjectCache.cpp b/cpp/src/IceGrid/ObjectCache.cpp
index 9e28dbd3555..c8694cac6fd 100644
--- a/cpp/src/IceGrid/ObjectCache.cpp
+++ b/cpp/src/IceGrid/ObjectCache.cpp
@@ -23,6 +23,16 @@ pointer_to_unary_function<int, int> ObjectCache::_rand(IceUtil::random);
 namespace IceGrid
 {
 
+struct ObjectEntryCI : binary_function<ObjectEntryPtr&, ObjectEntryPtr&, bool>
+{
+
+    bool
+    operator()(const ObjectEntryPtr& lhs, const ObjectEntryPtr& rhs)
+    {
+	return ::Ice::proxyIdentityLess(lhs->getProxy(), rhs->getProxy());
+    }
+};
+
 struct ObjectLoadCI : binary_function<pair<Ice::ObjectPrx, float>&, pair<Ice::ObjectPrx, float>&, bool>
 {
     bool operator()(const pair<Ice::ObjectPrx, float>& lhs, const pair<Ice::ObjectPrx, float>& rhs)
@@ -33,27 +43,29 @@ struct ObjectLoadCI : binary_function<pair<Ice::ObjectPrx, float>&, pair<Ice::Ob
 
 };
 
-ObjectCache::TypeEntry::TypeEntry(ObjectCache& cache) : _cache(cache)
+ObjectCache::TypeEntry::TypeEntry() : _allocatablesCount(0)
 {
 }
 
 void
-ObjectCache::TypeEntry::add(const Ice::ObjectPrx& obj)
+ObjectCache::TypeEntry::add(const ObjectEntryPtr& obj)
 {
     //
     // No mutex protection here, this is called with the cache locked.
     //
-    _objects.insert(lower_bound(_objects.begin(), _objects.end(), obj, ::Ice::proxyIdentityLess), obj);
+    _objects.insert(lower_bound(_objects.begin(), _objects.end(), obj, ObjectEntryCI()), obj);
+    _allocatablesCount += obj->isAllocatable() ? 1 : 0;
 }
 
 bool
-ObjectCache::TypeEntry::remove(const Ice::ObjectPrx& obj)
+ObjectCache::TypeEntry::remove(const ObjectEntryPtr& obj)
 {
     //
     // No mutex protection here, this is called with the cache locked.
     //
-    Ice::ObjectProxySeq::iterator q = lower_bound(_objects.begin(), _objects.end(), obj, ::Ice::proxyIdentityLess);
-    assert((*q)->ice_getIdentity() == obj->ice_getIdentity());
+    _allocatablesCount -= obj->isAllocatable() ? 1 : 0;
+    vector<ObjectEntryPtr>::iterator q = lower_bound(_objects.begin(), _objects.end(), obj, ObjectEntryCI());
+    assert(q->get() == obj.get());
     _objects.erase(q);
     return _objects.empty();
 }
@@ -108,6 +120,12 @@ ObjectCache::TypeEntry::canTryAllocate(const ObjectEntryPtr& entry)
     return false;
 }
 
+bool
+ObjectCache::TypeEntry::hasAllocatables() const
+{
+    return _allocatablesCount;
+}
+
 ObjectCache::ObjectCache(AdapterCache& adapterCache) : 
     _adapterCache(adapterCache)
 {
@@ -127,9 +145,9 @@ ObjectCache::add(const ObjectInfo& info, const string& application, bool allocat
     map<string, TypeEntry>::iterator p = _types.find(entry->getType());
     if(p == _types.end())
     {
-	p = _types.insert(p, make_pair(entry->getType(), TypeEntry(*this)));
+	p = _types.insert(p, make_pair(entry->getType(), TypeEntry()));
     }
-    p->second.add(info.proxy);
+    p->second.add(entry);
 
     if(_traceLevels && _traceLevels->object > 0)
     {
@@ -159,7 +177,7 @@ ObjectCache::remove(const Ice::Identity& id)
 
     map<string, TypeEntry>::iterator p = _types.find(entry->getType());
     assert(p != _types.end());
-    if(p->second.remove(entry->getObjectInfo().proxy))
+    if(p->second.remove(entry))
     {	
 	_types.erase(p);
     }
@@ -178,20 +196,18 @@ ObjectCache::allocateByType(const string& type, const ObjectAllocationRequestPtr
 {
     Lock sync(*this);
     map<string, TypeEntry>::iterator p = _types.find(type);
-    if(p == _types.end())
+    if(p == _types.end() || !p->second.hasAllocatables())
     {
-	request->response(0);
-	return;
+	throw AllocationException("no allocatable objects with type `" + type + "' registered");
     }
 
-    Ice::ObjectProxySeq objects = p->second.getObjects();
+    vector<ObjectEntryPtr> objects = p->second.getObjects();
     random_shuffle(objects.begin(), objects.end(), _rand); // TODO: OPTIMIZE
     try
     {
-	for(Ice::ObjectProxySeq::const_iterator q = objects.begin(); q != objects.end(); ++q)
+	for(vector<ObjectEntryPtr>::const_iterator q = objects.begin(); q != objects.end(); ++q)
 	{
-	    ObjectEntryPtr entry = getImpl((*q)->ice_getIdentity());
-	    if(entry->tryAllocate(request))
+	    if((*q)->tryAllocate(request))
 	    {
 		return;
 	    }
@@ -202,65 +218,6 @@ ObjectCache::allocateByType(const string& type, const ObjectAllocationRequestPtr
 	return;
     }
 
-    //
-    // TODO: XXX Only add the allocation request if there's allocatables for this type!!!
-    //
-    p->second.addAllocationRequest(request);
-}
-
-void
-ObjectCache::allocateByTypeOnLeastLoadedNode(const string& type, 
-					     const ObjectAllocationRequestPtr& request, 
-					     LoadSample sample)
-{
-    Lock sync(*this);
-    map<string, TypeEntry>::iterator p = _types.find(type);
-    if(p == _types.end())
-    {
-	request->response(0);
-	return;
-    }
-
-    Ice::ObjectProxySeq objects = p->second.getObjects();
-    random_shuffle(objects.begin(), objects.end(), _rand); // TODO: OPTIMIZE
-    vector<pair<Ice::ObjectPrx, float> > objsWLoad;
-    objsWLoad.reserve(objects.size());
-    for(Ice::ObjectProxySeq::const_iterator o = objects.begin(); o != objects.end(); ++o)
-    {
-	float load = 1.0f;
-	if(!(*o)->ice_getAdapterId().empty())
-	{
-	    try
-	    {
-		load = _adapterCache.get((*o)->ice_getAdapterId())->getLeastLoadedNodeLoad(sample);
-	    }
-	    catch(const AdapterNotExistException&)
-	    {
-	    }
-	}
-	objsWLoad.push_back(make_pair(*o, load));
-    }
-    sort(objsWLoad.begin(), objsWLoad.end(), ObjectLoadCI());
-
-    for(vector<pair<Ice::ObjectPrx, float> >::const_iterator q = objsWLoad.begin(); q != objsWLoad.end(); ++q)
-    {
-	ObjectEntryPtr entry = getImpl(q->first->ice_getIdentity());
-	if(entry->allocatable())
-	{
-	    if(entry->tryAllocate(request))
-	    {
-		return;
-	    }
-	    else if(request->isCanceled())
-	    {
-		return;
-	    }
-	}
-    }
-    
-    //
-    // TODO: XXX Only add the allocation request if there's allocatables for this type!!!
-    //
     p->second.addAllocationRequest(request);
 }
 
@@ -289,13 +246,12 @@ ObjectCache::getObjectsByType(const string& type)
     {
 	return proxies;
     }
-    const Ice::ObjectProxySeq& objects = p->second.getObjects();
-    for(Ice::ObjectProxySeq::const_iterator q = objects.begin(); q != objects.end(); ++q)
+    const vector<ObjectEntryPtr>& objects = p->second.getObjects();
+    for(vector<ObjectEntryPtr>::const_iterator q = objects.begin(); q != objects.end(); ++q)
     {
-	ObjectEntryPtr entry = getImpl((*q)->ice_getIdentity());
-	if(!entry->allocatable()) // Only return non-allocatable objects.
+	if((*q)->isAllocatable()) // Only return non-allocatable objects.
 	{
-	    proxies.push_back(*q);
+	    proxies.push_back((*q)->getProxy());
 	}
     }
     return proxies;
@@ -326,10 +282,11 @@ ObjectCache::getAllByType(const string& type)
     {
 	return infos;
     }
-    const Ice::ObjectProxySeq& objects = p->second.getObjects();
-    for(Ice::ObjectProxySeq::const_iterator q = objects.begin(); q != objects.end(); ++q)
+
+    const vector<ObjectEntryPtr>& objects = p->second.getObjects();
+    for(vector<ObjectEntryPtr>::const_iterator q = objects.begin(); q != objects.end(); ++q)
     {
-	infos.push_back(getImpl((*q)->ice_getIdentity())->getObjectInfo());
+	infos.push_back((*q)->getObjectInfo());
     }
     return infos;
 }
diff --git a/cpp/src/IceGrid/ObjectCache.h b/cpp/src/IceGrid/ObjectCache.h
index 6620f040f37..24a1a0f597d 100644
--- a/cpp/src/IceGrid/ObjectCache.h
+++ b/cpp/src/IceGrid/ObjectCache.h
@@ -87,7 +87,6 @@ public:
     ObjectEntryPtr remove(const Ice::Identity&);
 
     void allocateByType(const std::string&, const ObjectAllocationRequestPtr&);
-    void allocateByTypeOnLeastLoadedNode(const std::string&, const ObjectAllocationRequestPtr&, LoadSample);
     bool canTryAllocate(const ObjectEntryPtr&);
 
     Ice::ObjectProxySeq getObjectsByType(const std::string&); 
@@ -100,22 +99,22 @@ private:
     {
     public:
 
-	TypeEntry(ObjectCache&);
+	TypeEntry();
 
-
-	void add(const Ice::ObjectPrx&);
-	bool remove(const Ice::ObjectPrx&);
+	void add(const ObjectEntryPtr&);
+	bool remove(const ObjectEntryPtr&);
 	
 	void addAllocationRequest(const ObjectAllocationRequestPtr&);
 	bool canTryAllocate(const ObjectEntryPtr&);
 
-	const Ice::ObjectProxySeq& getObjects() const { return _objects; }
+	const std::vector<ObjectEntryPtr>& getObjects() const { return _objects; }
+	bool hasAllocatables() const;
 
     private:
 	
-	ObjectCache& _cache;
-	Ice::ObjectProxySeq _objects;
+	std::vector<ObjectEntryPtr> _objects;
 	std::list<ObjectAllocationRequestPtr> _requests;
+	int _allocatablesCount;
     };
 
     AdapterCache& _adapterCache;
diff --git a/cpp/src/IceGrid/ServerCache.cpp b/cpp/src/IceGrid/ServerCache.cpp
index 5f101ea3296..f5a9cb4acc2 100644
--- a/cpp/src/IceGrid/ServerCache.cpp
+++ b/cpp/src/IceGrid/ServerCache.cpp
@@ -248,7 +248,7 @@ ServerEntry::update(const ServerInfo& info)
     _adapters.clear();
     
     //
-    // TODO: XXX REVIEW
+    // Update the allocatable flag.
     //
     const_cast<bool&>(_allocatable) = info.descriptor->allocatable;
 }
diff --git a/cpp/src/IceGrid/SessionI.cpp b/cpp/src/IceGrid/SessionI.cpp
index b6774252021..f16733acaed 100644
--- a/cpp/src/IceGrid/SessionI.cpp
+++ b/cpp/src/IceGrid/SessionI.cpp
@@ -325,7 +325,11 @@ ClientSessionManagerI::create(const string& userId, const Glacier2::SessionContr
     // care of reaping the session.
     //
     SessionIPtr session = new ClientSessionI(userId, _database, current.adapter, _waitQueue, _timeout);
-    return Glacier2::SessionPrx::uncheckedCast(current.adapter->addWithUUID(session)); // TODO: XXX: category = userid?
+
+    //
+    // TODO: XXX: Update the Glacier2 allowable table to allow access to this object!
+    //
+    return Glacier2::SessionPrx::uncheckedCast(current.adapter->addWithUUID(session));
 }
 
 SessionPrx
diff --git a/cpp/test/IceGrid/allocation/AllTests.cpp b/cpp/test/IceGrid/allocation/AllTests.cpp
index abd833962cc..600aa967ec5 100644
--- a/cpp/test/IceGrid/allocation/AllTests.cpp
+++ b/cpp/test/IceGrid/allocation/AllTests.cpp
@@ -494,6 +494,33 @@ allTests(const Ice::CommunicatorPtr& communicator)
 	session1->setAllocationTimeout(0);
 	session2->setAllocationTimeout(0);
 
+
+	try
+	{
+	    obj = session1->allocateObjectByType("::Unknown");
+	    test(false);
+	}
+	catch(const AllocationTimeoutException&)
+	{
+	    test(false);
+	}
+	catch(const AllocationException&)
+	{
+	}
+
+	try
+	{
+	    obj = session1->allocateObjectByType("::NotAllocatable");
+	    test(false);
+	}
+	catch(const AllocationTimeoutException&)
+	{
+	    test(false);
+	}
+	catch(const AllocationException&)
+	{
+	}
+
 	obj = session1->allocateObjectByType("::Test");
 	test(obj && obj->ice_getIdentity().name == "allocatable");
 	try
diff --git a/cpp/test/IceGrid/allocation/application.xml b/cpp/test/IceGrid/allocation/application.xml
index 46b1a045110..0a9b9edd29f 100644
--- a/cpp/test/IceGrid/allocation/application.xml
+++ b/cpp/test/IceGrid/allocation/application.xml
@@ -9,9 +9,11 @@
   	  <object identity="nonallocatable" type="::Test" allocatable="false"/>
   	  <object identity="allocatable" type="::Test" allocatable="true"/>
   	  <object identity="allocatablebis" type="::TestBis" allocatable="true"/>
+  	  <object identity="nonallocatable2" type="::NotAllocatable"/>
         </adapter>
       </server>
 
+
       <server id="AdapterAllocation" exe="${test.dir}/server" activation="on-demand" pwd=".">
         <adapter name="Server" endpoints="default" allocatable="true" id="AdapterAlloc">
   	  <object identity="allocatable1" type="::TestAdapter1"/>