Rework NativePropertiesAdmin to use only new API for callbacks (#1672)

cpp/include/Ice/MetricsAdminI.h

@@ -583,9 +583,6 @@ class MetricsViewI : public IceUtil::Shared
 ICE_DEFINE_PTR(MetricsViewIPtr, MetricsViewI);
 
 class ICE_API MetricsAdminI : public IceMX::MetricsAdmin,
-#ifndef ICE_CPP11_MAPPING
-                              public Ice::PropertiesAdminUpdateCallback,
-#endif
                               private IceUtil::Mutex
 {
 public:

cpp/include/Ice/NativePropertiesAdmin.h

@@ -10,43 +10,6 @@
 namespace Ice
 {
 
-#ifndef ICE_CPP11_MAPPING
-/**
- * An application can be notified when its configuration properties are modified
- * via the Properties admin facet. The application must define a subclass of
- * PropertiesAdminUpdateCallback and register it with the facet. The facet
- * implements the class NativePropertiesAdmin, so the application needs to
- * downcast the facet to this type in order to register the callback.
- *
- * For example:
- *
- * \code
- * Ice::ObjectPtr obj = communicator->findAdminFacet("Properties");
- * assert(obj); // May be null if the facet is not enabled
- * NativePropertiesAdminPtr facet = NativePropertiesAdminPtr::dynamicCast(obj);
- * PropertiesAdminUpdateCallbackPtr myCallback = ...;
- * facet->addUpdateCallback(myCallback);
- * \endcode
- *
- * Ice ignores any exceptions raised by the callback.
- * \headerfile Ice/Ice.h
- */
-class ICE_API PropertiesAdminUpdateCallback
-{
-public:
-
-    virtual ~PropertiesAdminUpdateCallback();
-
-    /**
-     * Called when the communicator's properties have been updated.
-     * @param d A dictionary containing the properties that were added, changed or removed,
-     * with a removed property denoted by an entry whose value is an empty string.
-     */
-    virtual void updated(const PropertyDict& d) = 0;
-};
-using PropertiesAdminUpdateCallbackPtr = SharedPtr<PropertiesAdminUpdateCallback>;
-#endif
-
 /**
  * Base class for the Properties admin facet.
  * \headerfile Ice/Ice.h
@@ -57,24 +20,11 @@ class ICE_API NativePropertiesAdmin
 
     virtual ~NativePropertiesAdmin();
 
-#ifdef ICE_CPP11_MAPPING
     /**
      * Register an update callback that will be invoked when property updates occur.
      * @param cb The callback.
      */
     virtual std::function<void()> addUpdateCallback(std::function<void(const PropertyDict&)> cb) = 0;
-#else
-    /**
-     * Register an update callback that will be invoked when property updates occur.
-     * @param cb The callback.
-     */
-    virtual void addUpdateCallback(const PropertiesAdminUpdateCallbackPtr& cb) = 0;
-    /**
-     * Remove an update callback.
-     * @param cb The callback to be removed.
-     */
-    virtual void removeUpdateCallback(const PropertiesAdminUpdateCallbackPtr& cb) = 0;
-#endif
 };
 ICE_DEFINE_SHARED_PTR(NativePropertiesAdminPtr, NativePropertiesAdmin);
 

cpp/src/Ice/Instance.cpp

@@ -1425,13 +1425,9 @@ IceInternal::Instance::finishSetup(int& argc, const char* argv[], const Ice::Com
             //
             if(propsAdmin)
             {
-#ifdef ICE_CPP11_MAPPING
                 auto metricsAdmin = observer->getFacet();
                 propsAdmin->addUpdateCallback(
                     [metricsAdmin](const PropertyDict& changes) { metricsAdmin->updated(changes); });
-#else
-                propsAdmin->addUpdateCallback(observer->getFacet());
-#endif
             }
         }
     }

cpp/src/Ice/PropertiesAdminI.cpp

@@ -18,13 +18,6 @@ const char* traceCategory = "Admin.Properties";
 
 }
 
-#ifndef ICE_CPP11_MAPPING
-PropertiesAdminUpdateCallback::~PropertiesAdminUpdateCallback()
-{
-    // Out of line to avoid weak vtable
-}
-#endif
-
 NativePropertiesAdmin::~NativePropertiesAdmin()
 {
     // Out of line to avoid weak vtable
@@ -188,21 +181,12 @@ PropertiesAdminI::setProperties(const PropertyDict& props, const Current&)
         changes.insert(removed.begin(), removed.end());
 
         // Copy callbacks to allow callbacks to update callbacks
-#ifdef ICE_CPP11_MAPPING
         auto callbacks = _updateCallbacks;
         for(const auto& cb : callbacks)
-#else
-        vector<PropertiesAdminUpdateCallbackPtr> callbacks = _updateCallbacks;
-        for(vector<PropertiesAdminUpdateCallbackPtr>::const_iterator q = callbacks.begin(); q != callbacks.end(); ++q)
-#endif
         {
             try
             {
-#ifdef ICE_CPP11_MAPPING
                 cb(changes);
-#else
-                (*q)->updated(changes);
-#endif
             }
             catch(const std::exception& ex)
             {
@@ -224,8 +208,6 @@ PropertiesAdminI::setProperties(const PropertyDict& props, const Current&)
     }
 }
 
-#ifdef ICE_CPP11_MAPPING
-
 std::function<void()>
 PropertiesAdminI::addUpdateCallback(std::function<void(const Ice::PropertyDict&)> cb)
 {
@@ -244,22 +226,4 @@ PropertiesAdminI::removeUpdateCallback(std::list<std::function<void(const Ice::P
     _updateCallbacks.erase(p);
 }
 
-#else
-
-void
-PropertiesAdminI::addUpdateCallback(const PropertiesAdminUpdateCallbackPtr& cb)
-{
-    Lock sync(*this);
-    _updateCallbacks.push_back(cb);
-}
-
-void
-PropertiesAdminI::removeUpdateCallback(const PropertiesAdminUpdateCallbackPtr& cb)
-{
-    Lock sync(*this);
-    _updateCallbacks.erase(remove(_updateCallbacks.begin(), _updateCallbacks.end(), cb), _updateCallbacks.end());
-}
-
-#endif
-
 }

cpp/src/Ice/PropertiesAdminI.h

@@ -18,44 +18,35 @@
 namespace IceInternal
 {
 
-class PropertiesAdminI : public Ice::PropertiesAdmin, public Ice::NativePropertiesAdmin,
-#ifdef ICE_CPP11_MAPPING
-                         public std::enable_shared_from_this<PropertiesAdminI>,
-#endif
-                         private IceUtil::RecMutex
+class PropertiesAdminI final : public Ice::PropertiesAdmin,
+                               public Ice::NativePropertiesAdmin,
+                               public std::enable_shared_from_this<PropertiesAdminI>,
+                               private IceUtil::RecMutex
 {
 public:
 
     PropertiesAdminI(const InstancePtr&);
 
 #ifdef ICE_CPP11_MAPPING
-    virtual std::string getProperty(std::string, const Ice::Current&) override;
-    virtual Ice::PropertyDict getPropertiesForPrefix(std::string, const Ice::Current&) override;
-    virtual void setProperties(::Ice::PropertyDict, const Ice::Current&) override;
-
-    virtual std::function<void()> addUpdateCallback(std::function<void(const Ice::PropertyDict&)>) override;
-    void removeUpdateCallback(std::list<std::function<void(const Ice::PropertyDict&)>>::iterator);
-
+    std::string getProperty(std::string, const Ice::Current&) final;
+    Ice::PropertyDict getPropertiesForPrefix(std::string, const Ice::Current&) final;
+    void setProperties(::Ice::PropertyDict, const Ice::Current&) final;
 #else
-    virtual std::string getProperty(const std::string&, const Ice::Current&);
-    virtual Ice::PropertyDict getPropertiesForPrefix(const std::string&, const Ice::Current&);
-    virtual void setProperties(const Ice::PropertyDict&, const Ice::Current&);
-
-    virtual void addUpdateCallback(const Ice::PropertiesAdminUpdateCallbackPtr&);
-    virtual void removeUpdateCallback(const Ice::PropertiesAdminUpdateCallbackPtr&);
+    std::string getProperty(const std::string&, const Ice::Current&) final;
+    Ice::PropertyDict getPropertiesForPrefix(const std::string&, const Ice::Current&) final;
+    void setProperties(const Ice::PropertyDict&, const Ice::Current&) final;
 #endif
 
+    std::function<void()> addUpdateCallback(std::function<void(const Ice::PropertyDict&)>) final;
+
 private:
 
+    void removeUpdateCallback(std::list<std::function<void(const Ice::PropertyDict&)>>::iterator);
+
     const Ice::PropertiesPtr _properties;
     const Ice::LoggerPtr _logger;
 
-#ifdef ICE_CPP11_MAPPING
     std::list<std::function<void(const Ice::PropertyDict&)>> _updateCallbacks;
-#else
-    std::vector<Ice::PropertiesAdminUpdateCallbackPtr> _updateCallbacks;
-#endif
-
 };
 ICE_DEFINE_SHARED_PTR(PropertiesAdminIPtr, PropertiesAdminI);
 

cpp/test/Ice/admin/TestI.cpp

@@ -52,11 +52,7 @@ class NullLogger : public Ice::Logger
 
 RemoteCommunicatorI::RemoteCommunicatorI(const Ice::CommunicatorPtr& communicator) :
     _communicator(communicator),
-#ifdef ICE_CPP11_MAPPING
     _removeCallback(nullptr)
-#else
-    _hasCallback(false)
-#endif
 {
 }
 
@@ -71,11 +67,7 @@ RemoteCommunicatorI::getChanges(const Ice::Current&)
 {
     IceUtil::Monitor<IceUtil::Mutex>::Lock sync(*this);
 
-#ifdef ICE_CPP11_MAPPING
     if(_removeCallback)
-#else
-    if(_hasCallback)
-#endif
     {
        return _changes;
     }
@@ -95,13 +87,8 @@ RemoteCommunicatorI::addUpdateCallback(const Ice::Current&)
     {
         Ice::NativePropertiesAdminPtr admin = ICE_DYNAMIC_CAST(Ice::NativePropertiesAdmin, propFacet);
         assert(admin);
-#ifdef ICE_CPP11_MAPPING
         _removeCallback =
             admin->addUpdateCallback([this](const Ice::PropertyDict& changes) { updated(changes); });
-#else
-        admin->addUpdateCallback(Ice::PropertiesAdminUpdateCallbackPtr(shared_from_this()));
-        _hasCallback = true;
-#endif
     }
 }
 
@@ -115,16 +102,11 @@ RemoteCommunicatorI::removeUpdateCallback(const Ice::Current&)
     {
         Ice::NativePropertiesAdminPtr admin = ICE_DYNAMIC_CAST(Ice::NativePropertiesAdmin, propFacet);
         assert(admin);
-#ifdef ICE_CPP11_MAPPING
         if(_removeCallback)
         {
             _removeCallback();
             _removeCallback = nullptr;
         }
-#else
-        admin->removeUpdateCallback(Ice::PropertiesAdminUpdateCallbackPtr(shared_from_this()));
-        _hasCallback = false;
-#endif
     }
 
 }
@@ -209,7 +191,6 @@ RemoteCommunicatorFactoryI::createCommunicator(ICE_IN(Ice::PropertyDict) props,
     communicator->addAdminFacet(ICE_MAKE_SHARED(TestFacetI), "TestFacet");
 
     //
-    // The RemoteCommunicator servant also implements PropertiesAdminUpdateCallback.
     // Set the callback on the admin facet.
     //
     RemoteCommunicatorIPtr servant = ICE_MAKE_SHARED(RemoteCommunicatorI, communicator);

cpp/test/Ice/admin/TestI.h

@@ -10,10 +10,6 @@
 #include <Ice/NativePropertiesAdmin.h>
 
 class RemoteCommunicatorI : public virtual Test::RemoteCommunicator,
-#ifndef ICE_CPP11_MAPPING
-                            public virtual Ice::PropertiesAdminUpdateCallback,
-                            public std::enable_shared_from_this<RemoteCommunicatorI>,
-#endif
                             public IceUtil::Monitor<IceUtil::Mutex>
 {
 public:
@@ -42,11 +38,7 @@ class RemoteCommunicatorI : public virtual Test::RemoteCommunicator,
     Ice::CommunicatorPtr _communicator;
     Ice::PropertyDict _changes;
 
-#ifdef ICE_CPP11_MAPPING
     std::function<void()> _removeCallback;
-#else
-    bool _hasCallback;
-#endif
 };
 ICE_DEFINE_SHARED_PTR(RemoteCommunicatorIPtr, RemoteCommunicatorI);
 

cpp/test/Ice/metrics/AllTests.cpp

@@ -272,11 +272,7 @@ getServerConnectionMetrics(const IceMX::MetricsAdminPrxPtr& metrics, Ice::Long e
     return s;
 }
 
-class UpdateCallbackI :
-#ifndef ICE_CPP11_MAPPING
-        public Ice::PropertiesAdminUpdateCallback,
-#endif
-private IceUtil::Monitor<IceUtil::Mutex>
+class UpdateCallbackI : private IceUtil::Monitor<IceUtil::Mutex>
 {
 public:
 
@@ -597,12 +593,7 @@ allTests(Test::TestHelper* helper, const CommunicatorObserverIPtr& obsv)
     UpdateCallbackIPtr update = ICE_MAKE_SHARED(UpdateCallbackI, serverProps);
 
     ICE_DYNAMIC_CAST(Ice::NativePropertiesAdmin, communicator->findAdminFacet("Properties"))->addUpdateCallback(
-#ifdef ICE_CPP11_MAPPING
-        [update](const Ice::PropertyDict& changes) { update->updated(changes); }
-#else
-        update
-#endif
-        );
+        [update](const Ice::PropertyDict& changes) { update->updated(changes); });
 
     cout << "ok" << endl;
 

cpp/test/IceBox/admin/Service.cpp

@@ -51,18 +51,13 @@ ServiceI::ServiceI(const CommunicatorPtr& serviceManagerCommunicator)
     serviceManagerCommunicator->addAdminFacet(facet, "TestFacet");
 
     //
-    // The TestFacetI servant also implements PropertiesAdminUpdateCallback.
     // Set the callback on the admin facet.
     //
     ObjectPtr propFacet = serviceManagerCommunicator->findAdminFacet("IceBox.Service.TestService.Properties");
     NativePropertiesAdminPtr admin = ICE_DYNAMIC_CAST(NativePropertiesAdmin, propFacet);
     assert(admin);
 
-#ifdef ICE_CPP11_MAPPING
     admin->addUpdateCallback([facet](const Ice::PropertyDict& changes) { facet->updated(changes); });
-#else
-    admin->addUpdateCallback(facet);
-#endif
 }
 
 ServiceI::~ServiceI()

cpp/test/IceBox/admin/TestI.h

@@ -8,10 +8,7 @@
 #include <Test.h>
 
 class TestFacetI : public virtual ::Test::TestFacet,
-#ifndef ICE_CPP11_MAPPING
-                   public virtual Ice::PropertiesAdminUpdateCallback,
-#endif
-                   IceUtil::Monitor<IceUtil::Mutex>
+                   private IceUtil::Monitor<IceUtil::Mutex>
 {
 public:
 

objective-c/include/objc/Ice/NativePropertiesAdmin.h

@@ -9,10 +9,6 @@ ICE_API @protocol ICEPropertiesAdminUpdateCallback <NSObject>
 -(void) updated:(ICEMutablePropertyDict*)properties;
 @end
 
-ICE_DEPRECATED_API("Use NSObject instead")
-ICE_API @interface ICEPropertiesAdminUpdateCallback : NSObject
-@end
-
 ICE_API @protocol ICENativePropertiesAdmin <NSObject>
 -(void) addUpdateCallback:(id<ICEPropertiesAdminUpdateCallback>)callback;
 -(void) removeUpdateCallback:(id<ICEPropertiesAdminUpdateCallback>)callback;

objective-c/src/Ice/PropertiesI.h

@@ -22,6 +22,6 @@
 @interface ICENativePropertiesAdmin : ICEServantWrapper<ICENativePropertiesAdmin>
 {
     IceUtil::Mutex mutex_;
-    std::vector<Ice::PropertiesAdminUpdateCallbackPtr> callbacks_;
+    std::vector<std::pair<id, std::function<void()>>> callbacks_;
 }
 @end