celix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [celix] Oipo commented on a change in pull request #310: Adds initial impl for the C++ headers only wrappers
Date Wed, 17 Feb 2021 19:47:12 GMT

Oipo commented on a change in pull request #310:
URL: https://github.com/apache/celix/pull/310#discussion_r576918222



##########
File path: bundles/shell/shell/src/Shell.cc
##########
@@ -0,0 +1,284 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <memory>
+#include <unordered_map>
+
+#include "celix/BundleActivator.h"
+#include "celix/IShellCommand.h"
+#include "std_commands.h"
+#include "celix_shell_command.h"
+#include "celix_log_service.h"
+#include "celix_shell_command.h"
+#include "celix_shell.h"
+
+namespace celix {
+
+    class Shell {
+    public:
+
+        /**
+         * Try to execute a command using the provided command line.
+         */
+        celix_status_t executeCommand(const char *commandLine, FILE *out, FILE *err) {
+            std::vector<std::string> tokens = tokenize(commandLine);
+            if (tokens.empty()) {
+                fprintf(err, "Invalid commandLine '%s'\n", commandLine);
+                return CELIX_ILLEGAL_ARGUMENT;
+            }
+
+            Entry entry = findCommand(tokens[0], err);
+            if (entry.cCommand) {
+                 bool successful = entry.cCommand->executeCommand(entry.cCommand->handle,
commandLine, out, err);
+                 return successful ? CELIX_SUCCESS : CELIX_SERVICE_EXCEPTION;
+            } else if (entry.cxxCommand) {
+                std::vector<std::string> arguments{};
+                auto start = ++tokens.begin();
+                arguments.insert(arguments.begin(), start, tokens.end());
+                entry.cxxCommand->executeCommand(commandLine, arguments, out, err);

Review comment:
       `arguments` is copied for each invocation. Either use `std::move` or make the second
argument a reference type in `IShellCommand`

##########
File path: examples/celix-examples/services_example_cxx/src/SimpleConsumerBundleActivator.cc
##########
@@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "celix/BundleActivator.h"
+#include "examples/ICalc.h"
+
+class SimpleConsumer {
+public:
+    ~SimpleConsumer() {
+        stop();
+    }
+
+    SimpleConsumer() = default;
+    SimpleConsumer(SimpleConsumer&&) = delete;
+    SimpleConsumer& operator=(SimpleConsumer&&) = delete;
+    SimpleConsumer(const SimpleConsumer&) = delete;
+    SimpleConsumer& operator=(const SimpleConsumer&) = delete;
+
+    void start() {
+        std::lock_guard<std::mutex> lck{mutex};
+        calcThread = std::thread{&SimpleConsumer::run, this};
+    }
+
+    void stop() {
+        active = false;
+        std::lock_guard<std::mutex> lck{mutex};

Review comment:
       Possible deadlock:
   1. calcThread checks active
   2. stop sets active to false
   3. stop locks mutex and waits for calcThread
   4. calcThread tries to lock mutex
   
   Probably bettter to use the atomic exchange thing to determine if calcThread needs to be
joined.

##########
File path: libs/framework/include/celix/Framework.h
##########
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#pragma once
+
+#include <memory>
+
+#include "celix_framework.h"
+
+namespace celix {
+
+    class BundleContext; //forward declaration
+
+    /**
+     * @brief A Celix framework instance. A framework is also known as a system bundle.
+     *
+     * Celix framework instances are created using a FrameworkFactory.
+     *
+     * @note Thread safe.
+     */
+    class Framework {
+    public:
+        Framework(std::shared_ptr<celix::BundleContext> _fwCtx, celix_framework_t*
_cFw) :
+            fwCtx{std::move(_fwCtx)},
+            cFw{std::shared_ptr<celix_framework_t >{_cFw, [](celix_framework_t*){/*nop*/}}}
+            {}
+
+        /**
+         * @brief Get the framework UUID.
+         */
+        std::string getUUID() const {
+            return std::string{celix_framework_getUUID(cFw.get())};
+        }
+
+        /**
+         * @brief Get the bundle context for the framework.
+         */
+        std::shared_ptr<celix::BundleContext> getFrameworkBundleContext() const {
+            return fwCtx;
+        }
+
+        /**
+         * @brief Fire a generic Celix framework event.
+         *
+         * The event will be added to the event loop and handled on the event loop thread.
+         *
+         * if bndId >=0 the bundle usage count will be increased while the event is not
yet processed or finished processing.
+         * The eventName is expected to be const char* valid during til the event is finished
processing.
+         *
+         * if eventId >=0 this will be used, otherwise a new event id will be generated.
+         *
+         * @return the event id (can be used in Framework::waitForEvent).
+         */
+        long fireGenericEvent(long bndId, const char* eventName, std::function<void()>
processEventCallback, long eventId = -1) {
+            auto* callbackOnHeap = new std::function<void()>{};
+            *callbackOnHeap = std::move(processEventCallback);
+            return celix_framework_fireGenericEvent(
+                    cFw.get(),
+                    eventId,
+                    bndId,
+                    eventName,
+                    static_cast<void*>(callbackOnHeap),
+                    [](void *data) {
+                        auto* callback = static_cast<std::function<void()>*>(data);
+                        (*callback)();
+                        delete callback;
+                    },
+                    nullptr,
+                    nullptr);
+        }
+
+        /**
+         * @brief Block until the framework is stopped.
+         */
+         void waitForStop() {
+            celix_framework_waitForStop(cFw.get());
+        }
+
+        /**
+         * @brief Wait until all Celix event for this framework are completed.
+         */
+        void waitForEvent(long eventId) {
+            celix_framework_waitForGenericEvent(cFw.get(), eventId);
+        }
+
+        /**
+         * @brief Get the C framework.
+         *
+         * @warning Try not the depend on the C API from a C++ bundle. If features are missing
these should be added to
+         * the C++ API.

Review comment:
       Plenty of functions available in the C API are missing (such as `celix_framework_useBundle`).
Is it not wiser to already add these to the C++ API?

##########
File path: libs/framework/include/celix/ServiceFactory.h
##########
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#pragma once
+
+#include "celix/Bundle.h"
+#include "celix/Properties.h"
+
+namespace celix {
+
+    template<typename I>
+    class ServiceFactory {
+    public:
+        virtual ~ServiceFactory() = default;
+
+        virtual std::shared_ptr<I> createBundleSpecificService(const celix::Bundle&
requestingBundle, const celix::Properties svcFactoryProperties) = 0;
+        //TODO,TBD need ungetService variant?

Review comment:
       If possible to put this in the deleter of the shared_ptr, I would prefer that.

##########
File path: libs/framework/doxygen.md
##########
@@ -0,0 +1,66 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+   
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+
+# Intro
+
+The main way to use Celix is through the bundle context of a bundle.
+When a bundle is started the bundle context will be injected in the bundle activator.
+
+Apache Celix is a C framework with a C and C++ (C++11) API. 

Review comment:
       Is it intentional to keep this at c++11 when some parts are already C++17?

##########
File path: libs/framework/include/celix/Bundle.h
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#pragma once
+
+#include <memory>
+
+#include "celix_bundle.h"
+
+namespace celix {
+
+    /**
+     * @brief An installed bundle in the Celix framework.
+     *
+     * Each bundle installed in the Celix framework must have an associated Bundle object.
+     * A bundle must have a unique identity, a long, chosen by the Celix framework.
+     *
+     * @note Thread safe.
+     */
+    class Bundle {

Review comment:
       What is the intended way for a user to get:
   * bundle state
   * bundle group
   * list of registered services in bundle
   * list of registered trackers in bundle

##########
File path: examples/celix-examples/services_example_cxx/src/DynamicConsumerBundleActivator.cc
##########
@@ -0,0 +1,159 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <utility>
+
+#include "celix/IShellCommand.h"
+#include "celix/BundleActivator.h"
+#include "examples/ICalc.h"
+
+namespace /*anon*/ {
+
+    class DynamicConsumer {
+    public:
+        explicit DynamicConsumer(int _id) : id{_id} {}
+
+        ~DynamicConsumer() {
+            stop();
+        }
+
+        void start() {
+            std::lock_guard<std::mutex> lck{mutex};
+            calcThread = std::thread{&DynamicConsumer::run, this};

Review comment:
       lock_guard implies that calcThread needs to be thread-safe. But `stop()` isn't using
a mutex. Perhaps use the `exchange` method on the `active` variable to determine what to do?

##########
File path: examples/celix-examples/dm_example_cxx/phase1/src/Phase1Activator.cc
##########
@@ -38,17 +38,22 @@ struct InvalidCServ {
 };
 
 Phase1Activator::Phase1Activator(std::shared_ptr<celix::dm::DependencyManager> mng)
{
-    auto cmp = std::shared_ptr<Phase1Cmp>(new Phase1Cmp());
+    dm = mng;
+    auto& cmp = mng->createComponent<Phase1Cmp>();  //using a pointer a instance.
Also supported is lazy initialization (default constructor needed) or a rvalue reference (move)

Review comment:
       Not only is the first sentence hard to follow, a reference is actually used instead
of a pointer.

##########
File path: libs/framework/include/celix/TrackerBuilders.h
##########
@@ -0,0 +1,358 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#pragma once
+
+#include <string>
+#include <vector>
+#include <memory>
+
+#include "Trackers.h"
+
+namespace celix {
+
+    /**
+     * @brief Fluent builder API to track services.
+     *
+     * @see celix::BundleContext::trackServices for more info.
+     * @tparam I The service type to track.
+     * @note Not thread safe.
+     */
+    template<typename I>
+    class ServiceTrackerBuilder {
+    private:
+        friend class BundleContext;
+
+        //NOTE private to prevent move so that a build() call cannot be forgotten
+        ServiceTrackerBuilder(ServiceTrackerBuilder&&) = default;
+    public:
+        explicit ServiceTrackerBuilder(std::shared_ptr<celix_bundle_context_t> _cCtx,
std::string _name) :
+                cCtx{std::move(_cCtx)},
+                name{std::move(_name)} {}
+
+        ServiceTrackerBuilder& operator=(ServiceTrackerBuilder&&) = delete;
+        ServiceTrackerBuilder(const ServiceTrackerBuilder&) = delete;
+        ServiceTrackerBuilder operator=(const ServiceTrackerBuilder&) = delete;
+
+        /**
+         * @brief Set filter to be used to matching services.
+         *
+         * The filter must be LDAP filter.
+         * Example:
+         *      "(property_key=value)"
+         */
+        ServiceTrackerBuilder& setFilter(std::string f) { filter = celix::Filter{std::move(f)};
return *this; }
+
+        /**
+         * @brief Set filter to be used to matching services.
+         */
+        ServiceTrackerBuilder& setFilter(celix::Filter f) { filter = std::move(f); return
*this; }
+
+        /**
+         * @brief Adds a add callback function, which will be called - on the Celix event
thread -
+         * when a new service match is found.
+         *
+         * The add callback function has 1 argument: A shared ptr to the added service.
+         */
+        ServiceTrackerBuilder& addAddCallback(std::function<void(std::shared_ptr<I>)>
add) {
+            addCallbacks.emplace_back([add](std::shared_ptr<I> svc, std::shared_ptr<const
celix::Properties>, std::shared_ptr<const celix::Bundle>) {
+                add(svc);
+            });
+            return *this;
+        }
+
+        /**
+         * @brief Adds a add callback function, which will be called - on the Celix event
thread -
+         * when a new service match is found.
+         *
+         * The add callback function has 2 arguments:
+         *  - A shared ptr to the added service.
+         *  - A shared ptr to the added service properties.
+         */
+        ServiceTrackerBuilder& addAddWithPropertiesCallback(std::function<void(std::shared_ptr<I>,
std::shared_ptr<const celix::Properties>)> add) {
+            addCallbacks.emplace_back([add](std::shared_ptr<I> svc, std::shared_ptr<const
celix::Properties> props, std::shared_ptr<const celix::Bundle>) {
+                add(svc, props);
+            });
+            return *this;
+        }
+
+        /**
+         * @brief Adds a add callback function, which will be called - on the Celix event
thread -
+         * when a new service match is found.
+         *
+         * The add callback function has 3 arguments:
+         *  - A shared ptr to the added service.
+         *  - A shared ptr to the added service properties.
+         *  - A shared ptr to the bundle owning the added service.
+         */
+        ServiceTrackerBuilder& addAddWithOwnerCallback(std::function<void(std::shared_ptr<I>,
std::shared_ptr<const celix::Properties>, std::shared_ptr<const celix::Bundle>)>
add) {
+            addCallbacks.emplace_back(std::move(add));
+            return *this;
+        }
+
+        /**
+         * @brief Adds a remove callback function, which will be called - on the Celix event
thread -
+         * when a service match is being removed.
+         *
+         * The remove callback function has 1 arguments: A shared ptr to the removing service.
+         */
+        ServiceTrackerBuilder& addRemCallback(std::function<void(std::shared_ptr<I>)>
remove) {
+            remCallbacks.emplace_back([remove](std::shared_ptr<I> svc, std::shared_ptr<const
celix::Properties>, std::shared_ptr<const celix::Bundle>) {
+                remove(svc);
+            });
+            return *this;
+        }
+
+        /**
+         * @brief Adds a remove callback function, which will be called - on the Celix event
thread -
+         * when a service match is being removed.
+         *
+         * The remove callback function has 2 arguments:
+         *  - A shared ptr to the removing service.
+         *  - A shared ptr to the removing service properties.
+         */
+        ServiceTrackerBuilder& addRemWithPropertiesCallback(std::function<void(std::shared_ptr<I>,
std::shared_ptr<const celix::Properties>)> remove) {
+            remCallbacks.emplace_back([remove](std::shared_ptr<I> svc, std::shared_ptr<const
celix::Properties> props, std::shared_ptr<const celix::Bundle>) {
+                remove(svc, props);
+            });
+            return *this;
+        }
+
+        /**
+         * @brief Adds a remove callback function, which will be called - on the Celix event
thread -
+         * when a service match is being removed.
+         *
+         * The remove callback function has 3 arguments:
+         *  - A shared ptr to the removing service.
+         *  - A shared ptr to the removing service properties.
+         *  - A shared ptr to the bundle owning the removing service.
+         */
+        ServiceTrackerBuilder& addRemWithOwnerCallback(std::function<void(std::shared_ptr<I>,
std::shared_ptr<const celix::Properties>, std::shared_ptr<const celix::Bundle>)>
remove) {
+            remCallbacks.emplace_back(std::move(remove));
+            return *this;
+        }
+
+        /**
+         * @brief Adds a set callback function, which will be called - on the Celix event
thread -
+         * when there is a new highest ranking service match.
+         * This can can also be an empty match (nullptr).
+         *
+         * The set callback function has 2 arguments: A shared ptr to the highest

Review comment:
       2 arguments? I only see 1

##########
File path: libs/framework/include/celix/Trackers.h
##########
@@ -0,0 +1,783 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#pragma once
+
+#include <memory>
+#include <mutex>
+#include <atomic>
+#include <cassert>
+#include <set>
+#include <unordered_map>
+
+#include "celix_utils.h"
+#include "celix/Properties.h"
+#include "celix/Utils.h"
+#include "celix/Bundle.h"
+#include "celix/Constants.h"
+#include "celix/Filter.h"
+#include "celix_bundle_context.h"
+
+namespace celix {
+
+
+    /**
+     * @brief The tracker state.
+     */
+    enum class TrackerState {
+        OPENING,
+        OPEN,
+        CLOSING,
+        CLOSED
+    };
+
+    /**
+     * @brief The AbstractTracker class is the base of all C++ Celix trackers.
+     *
+     * It defines how trackers are closed and manages the tracker state.
+     *
+     * This class can be used to create a vector of different (shared ptr)
+     * trackers (i.e. ServiceTracker, BundleTracker, MetaTracker).
+     *
+     * AbstractTracker <-----------------------------------
+     *  ^                               |                |
+     *  |                               |                |
+     *  GenericServiceTracker      BundleTracker    MetaTracker
+     *  ^
+     *  |
+     *  ServiceTracker<I>
+     *
+     * @note Thread safe.
+     */
+    class AbstractTracker {
+    public:
+        explicit AbstractTracker(std::shared_ptr<celix_bundle_context_t> _cCtx) :
+            cCtx{std::move(_cCtx)} {}
+
+        virtual ~AbstractTracker() noexcept = default;
+
+        /**
+         * @brief Check if the tracker is open (state == OPEN)
+         */
+        bool isOpen() const {
+            std::lock_guard<std::mutex> lck{mutex};
+            return state == TrackerState::OPEN;
+        }
+
+        /**
+         * @brief Get the current state of the tracker.
+         */
+        TrackerState getState() const {
+            std::lock_guard<std::mutex> lck{mutex};
+            return state;
+        }
+
+        /**
+         * @brief Close the tracker (of the state is not CLOSED or CLOSING).
+         *
+         * This will be done sync so then the close() method return the
+         * tracker is closed and all the needed callbacks have been called.
+         */
+        void close() {
+            long localTrkId = -1;
+            {
+                std::lock_guard<std::mutex> lck{mutex};
+                if (state == TrackerState::OPEN || state == TrackerState::OPENING) {
+                    //not yet closed
+                    state = TrackerState::CLOSING;
+                    localTrkId = trkId;
+                    trkId = -1;
+                }
+            }
+            if (localTrkId >= 0) {
+                celix_bundleContext_stopTracker(cCtx.get(), localTrkId);
+                {
+                    std::lock_guard<std::mutex> lck{mutex};
+                    state = TrackerState::CLOSED;
+                }
+            }
+        }
+
+        /**
+         * @brief Open the tracker (if the state is not OPEN or OPENING).
+         *
+         * This is done async, meaning that the actual opening of the tracker will be
+         * done a a Celix event processed on the Celix event thread.
+         *
+         * @throws celix::Exception
+         */
+        virtual void open() {}

Review comment:
       Any particular reason this is not a pure virtual function?

##########
File path: libs/framework/include/celix/Utils.h
##########
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#pragma once
+
+
+#include <string>
+#include <string.h>
+#include <iostream>
+
+namespace celix {
+    /**
+     * @brief Returns the deferred type name for the template I
+     */
+    template<typename INTERFACE_TYPENAME>
+    std::string typeName() {
+        std::string result;
+
+#ifdef __GXX_RTTI

Review comment:
       I think this would work for clang as well, if one were to use something like https://gist.github.com/brimston3/2be168bb423c83b0f469c0be56e66d31

##########
File path: examples/celix-examples/services_example_cxx/src/DynamicConsumerBundleActivator.cc
##########
@@ -0,0 +1,159 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <utility>
+
+#include "celix/IShellCommand.h"
+#include "celix/BundleActivator.h"
+#include "examples/ICalc.h"
+
+namespace /*anon*/ {
+
+    class DynamicConsumer {
+    public:
+        explicit DynamicConsumer(int _id) : id{_id} {}
+
+        ~DynamicConsumer() {
+            stop();
+        }
+
+        void start() {
+            std::lock_guard<std::mutex> lck{mutex};
+            calcThread = std::thread{&DynamicConsumer::run, this};
+        }
+
+        void stop() {
+            active = false;
+            if (calcThread.joinable()) {
+                calcThread.join();
+                auto msg = std::string{"Destroying consumer nr "} + std::to_string(id) +
"\n";
+                std::cout << msg;
+            }
+        }
+
+        void setCalc(std::shared_ptr<examples::ICalc> _calc, const std::shared_ptr<const
celix::Properties>& props) {
+            std::lock_guard<std::mutex> lck{mutex};
+            if (_calc) {
+                calc = std::move(_calc);
+                svcId = props->getAsLong(celix::SERVICE_ID, -1);
+                ranking = props->getAsLong(celix::SERVICE_RANKING, 0);
+
+                std::string msg = "Setted calc svc for consumer nr " +  std::to_string(id);
+                msg += ". svc id is  "  + std::to_string(svcId);
+                msg += ", and ranking is " + std::to_string(ranking);
+                msg += "\n";
+                std::cout << msg;
+            } else {
+                std::string msg = "Resetting calc svc for consumer " + std::to_string(id)
 + " to nullptr\n";
+                std::cout << msg;
+
+                calc = nullptr;
+                svcId = -1;
+                ranking = 0;
+            }
+        }
+    private:
+        void run() {
+            std::unique_lock<std::mutex> lck{mutex,  std::defer_lock};
+            int count = 1;
+            while (active) {
+                lck.lock();
+                auto localCalc = calc;
+                lck.unlock();
+
+                /*
+                 * note it is safe to use the localCalc outside a mutex,
+                 * because the shared_prt count will ensure the service cannot be unregistered
while in use.
+                 */
+                if (localCalc) {
+                    auto msg = std::string{"Calc result is "} + std::to_string(localCalc->calc(count))
+ "\n";

Review comment:
       Probably nicer to merge this with the cout, that way a std::string{} does not need
to be constructed.

##########
File path: libs/framework/include/celix/BundleContext.h
##########
@@ -0,0 +1,580 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#pragma once
+
+#include <memory>
+#include <mutex>
+#include <thread>
+#include <cstdarg>
+
+#include "celix_bundle_context.h"
+
+#include "celix/ServiceRegistrationBuilder.h"
+#include "celix/UseServiceBuilder.h"
+#include "celix/TrackerBuilders.h"
+#include "celix/Bundle.h"
+#include "celix/Framework.h"
+
+#include "celix/dm/DependencyManager.h" //TODO, TBD include or forward declaration?
+
+namespace celix {
+
+    /**
+     * @brief The bundle context is used to interact with the Celix framework.
+     *
+     * The bundle context represent a bundle and can be used to:
+     *  - Register, use and track services
+     *  - Install, start, stop and uninstall bundles (runtime)
+     *  - log on framework level
+     *  - Access bundles
+     *  - Get config properties
+     *
+     * @note Thread safe.
+     */
+    class BundleContext {

Review comment:
       Are the async methods planned for the future?

##########
File path: libs/framework/include/celix_bundle_activator.h
##########
@@ -139,75 +139,6 @@ celix_status_t celix_bundleActivator_destroy(void *userData, celix_bundle_contex
 
 #ifdef __cplusplus
 }
-
-
-/**
- * This macro generates the required bundle activator functions for C++.
- * This can be used to more type safe bundle activator entries.
- *
- * The macro will create the following bundle activator functions:
- * - bundleActivator_create which allocates a pointer to the provided type.
- * - bundleActivator_start/stop which will call the respectively provided typed start/stop
functions.
- * - bundleActivator_destroy will free the allocated for the provided type.
- *
- * @param type The activator type (e.g. 'ShellActivator'). A type which should have a constructor
with a single arugment of std::shared_ptr<DependencyManager>.
- */
-#define CELIX_GEN_CXX_BUNDLE_ACTIVATOR(actType)                                         
                              \

Review comment:
       Is there any transition macro possible to define to ease refactoring cost for users
that have been using this?

##########
File path: libs/utils/src/filter.c
##########
@@ -58,11 +58,15 @@ void filter_destroy(celix_filter_t * filter) {
 static celix_filter_t * filter_parseFilter(char * filterString, int * pos) {
     celix_filter_t * filter;
     filter_skipWhiteSpace(filterString, pos);
-    if (filterString[*pos] != '(') {
+    if (filterString[*pos] == '\0') {
+        //empty filter
+        fprintf(stderr, "Filter Error: Cannot create LDAP filter from an empty string.\n");
+        return NULL;
+    } else if (filterString[*pos] != '(') {
         fprintf(stderr, "Filter Error: Missing '(' in filter string '%s'.\n", filterString);
         return NULL;
     }
-    (*pos)++;
+    (*pos)++; //eat '('

Review comment:
       :+1: 

##########
File path: libs/utils/src/properties.c
##########
@@ -346,7 +346,7 @@ void celix_properties_store(celix_properties_t *properties, const char
*filename
 
 celix_properties_t* celix_properties_copy(const celix_properties_t *properties) {
     celix_properties_t *copy = celix_properties_create();
-    if (copy != NULL) {
+    if (properties != NULL) {

Review comment:
       And we've not had bugs pop up? Shocking.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



Mime
View raw message