location-service

Merge lp:~mardy/location-service/providers-dir into lp:location-service/trunk

  1. providers-dir
  2. Merge into trunk
Proposed by Alberto Mardegan on 2015-12-16
Status: Work in progress
Proposed branch: lp:~mardy/location-service/providers-dir
Merge into: lp:location-service/trunk
Diff against target: 890 lines (+420/-141)
20 files modified
include/location_service/com/ubuntu/location/default_provider_selection_policy.h (+4/-4)
include/location_service/com/ubuntu/location/provider_collection.h (+34/-12)
include/location_service/com/ubuntu/location/provider_selection_policy.h (+2/-3)
src/location_service/com/ubuntu/location/CMakeLists.txt (+2/-0)
src/location_service/com/ubuntu/location/default_provider_selection_policy.cpp (+10/-10)
src/location_service/com/ubuntu/location/engine.h (+6/-25)
src/location_service/com/ubuntu/location/filesystem_provider_loader.cpp (+97/-0)
src/location_service/com/ubuntu/location/filesystem_provider_loader.h (+69/-0)
src/location_service/com/ubuntu/location/non_selecting_provider_selection_policy.cpp (+2/-2)
src/location_service/com/ubuntu/location/non_selecting_provider_selection_policy.h (+1/-1)
src/location_service/com/ubuntu/location/provider_loader.h (+54/-0)
src/location_service/com/ubuntu/location/provider_manifest.cpp (+45/-0)
src/location_service/com/ubuntu/location/provider_manifest.h (+76/-0)
src/location_service/com/ubuntu/location/service/daemon.cpp (+14/-70)
src/location_service/com/ubuntu/location/service/daemon.h (+0/-4)
src/location_service/com/ubuntu/location/service/default_configuration.cpp (+1/-6)
src/location_service/com/ubuntu/location/service/default_configuration.h (+0/-1)
tests/engine_test.cpp (+1/-1)
tests/null_provider_selection_policy.h (+1/-1)
tests/provider_selection_policy_test.cpp (+1/-1)
To merge this branch: bzr merge lp:~mardy/location-service/providers-dir
Reviewer Review Type Date Requested Status
Thomas Voß (community) 2015-12-16 Needs Fixing on 2015-12-18
Review via email: mp+280724@code.launchpad.net

Description of the Change

WIP

Manifest-based provider loading

Don't hardcode the list of available providers. Instead, scan some directories from the filesystem ($XDG_DATA_DIRS/location/providers) for <provider>.json files, which tell which providers are available.

The goal is to allow a simple way of configuring providers; for instance, we might have a file

  /usr/share/location/providers/google-location-service.json

with these contents:

  {
    "plugin": "gls",
    "configuration": {
      "endpoint": "https://www.googleapis.com/geolocation/v1/geolocate",
      "api-key": "xyz"
    }
  }

which will cause the location service to load a "gls" plugin and initialize it with the given configuration, suitable for the Google service. Also, a user might create a file

  ~/.local/share/location/providers/mozilla.json

with these contents:

  {
    "plugin": "gls",
    "configuration": {
      "endpoint": "https://location.services.mozilla.com/v1/geolocate",
      "api-key": "abc"
    }
  }

By doing this, the user is asking ULS to create a second instance of the "gls" plugin, this time setup to use the similar service provided by Mozilla.

The ULS remembers which manifest files have been loaded (using the basename of the manifest file as a key), and will refuse to load the same provider twice. So, continuing our example above, the user could create a file

  ~/.local/share/location/providers/google-location-service.json

which would override the system one. This can be useful when a different configuration is desired (for example, we the plugin might accept a configuration option to decide which of WIFI, CellId, or GeoIP positioning methods are allowed, and the user might want to force a specific one).

Some providers are built into the location service; in order to enable them, their plugin name must be prefixed with a "@". So, to enable the GPS provider, we should create a file

  /usr/share/location/providers/gps.json

with contents:

  {
    "plugin": "@gps"
  }

(note that the filename is irrelevant in this case: what matters is the value of the "plugin" key)
The current implementation assumes that if the plugin name does not start with a "@", the plugin must be loaded using the "remote::Provider"; so, in the "configuration" key one should store the options needed by the remote provider. So, for example:

  {
    "plugin": "espoo",
    "configuration": {
      "name": "com.ubuntu.espoo.Service.Provider",
      "path": "/com/ubuntu/espoo/Service/Provider"
    }
  }

========== OPEN QUESTIONS =========

The current WIP implementation still supports passing provider options from the command line (and actually, requires that at least one provider is specified). Command line options do override those from the manifest file. However, despite having implemented this, I think that there is no point in having this code, and I'd propose removing it and letting all the provider configuration happen in the manifest. Do you agree?

Then, about remote providers, to make landing this branch as easy as possible, I'd propose (at least for the time being) to keep the existing mechanism of loading remove providers via the remote::Provider interface; we can actually get rid of the D-Bus name and path from the "configuration" key and always assume that they are in the form:
    "com.ubuntu.<plugin>.Service.Provider"
    "/com/ubuntu/<plugin>/Service/Provider"
where "<plugin>" is the value of the "plugin" key in the manifest files.

In the long term, I'd like to get rid of D-Bus and just use a socket (maybe with a p2p D-Bus server on top of it, to minimize changes -- but we should add this to dbuscpp first), because there's no need to route all location updates through the server when all we want is actually a p2p communication. But that's for the far future.

To post a comment you must log in.
Thomas Voß (thomas-voss) wrote :

Thanks for the proposal, the general approach looks fine, I left a couple of comments inline.

For your questions:

  (1.) Let's not keep redundant code paths around and go all in.
  (2.) Please keep the keys, and let's avoid implicit conventions only visible in code.
       In addition, let's load providers with { plugin: remote; configuration { plugin ... }}.
       While being a little more verbose, it is absolutely clear what is happening just from
       reading the configuration file.
  (3.) We can think about spawning a private bus instance, but I disagree that the service should
       become a communication hub and take over responsibilities that are in the area of the bus
       daemon or the respective kernel facilities once kdbus is widely available.

With (2.), I would also propose to get rid of the "@" prefix convention as indicated in the corresponding inline comment.

Looking forward to see this landing in the not-too-distant future :)

review: Needs Fixing
Reply
Alberto Mardegan (mardy) wrote :

Thanks Thomas for the review.

I just want to clarify a couple of things about the FilesystemProviderLoader: why I have the base_dir parameter on the load_providers() method and why it's stateful (the instantiated providers set).

The point is that it needs to load provider manifests from several directories (/usr/share/location/providers and ~/.local/share/location/providers, at least) and at the same time there's an override mechanism so that the manifests defined by the user can override the system ones (as per the description).
So, I think we have three possibilities:
1) keep it as is
2) do as you suggested, but then the constructor needs to take a std::vector<fs::path>, not just a single path
3) no paths visible in the FilesystemProviderLoader API, handle everything internally

Reply
Thomas Voß (thomas-voss) wrote :

> Thanks Thomas for the review.
>
> I just want to clarify a couple of things about the FilesystemProviderLoader:
> why I have the base_dir parameter on the load_providers() method and why it's
> stateful (the instantiated providers set).
>
> The point is that it needs to load provider manifests from several directories
> (/usr/share/location/providers and ~/.local/share/location/providers, at
> least) and at the same time there's an override mechanism so that the
> manifests defined by the user can override the system ones (as per the
> description).

@stateful: I get why you maintain the set, but that could just be scoped to the function itself iiuc.

> So, I think we have three possibilities:
> 1) keep it as is
> 2) do as you suggested, but then the constructor needs to take a
> std::vector<fs::path>, not just a single path

I think this one is the best way forward, I originally made that proposal in an inline comment:

  "This should take const std::set<boost::filesystem::path>& base_dir at construction time."

The joys of the laucnhpad review UI then :)
> 3) no paths visible in the FilesystemProviderLoader API, handle everything
> internally

Reply
lp:~mardy/location-service/providers-dir updated on 2015-12-23
229. By Alberto Mardegan on 2015-12-21

Move provider_loader to src/

230. By Alberto Mardegan on 2015-12-21

Remove provider_options from load_providers

231. By Alberto Mardegan on 2015-12-21

Simplify consdruction of path list

232. By Alberto Mardegan on 2015-12-21

Pass dir list in constructor

233. By Alberto Mardegan on 2015-12-21

Make load_providers() stateless

234. By Alberto Mardegan on 2015-12-21

Use boost::fs:path in public methods

235. By Alberto Mardegan on 2015-12-21

Remove is_internal method

236. By Alberto Mardegan on 2015-12-23

Add override clause

237. By Alberto Mardegan on 2015-12-23

no providers from command line

238. By Alberto Mardegan on 2015-12-23

Don't return reference to temporary

Alberto Mardegan (mardy) wrote :

I've updated the MP according to your advice; just a couple of notes:

- I didn't introduce the Receiver class and use it instead of ProviderCollection: I'll do that when the async proivider loading MP has landed.

- regarding ProviderManifest, I cannot add a constructor which takes a stream, because I need the file name; this also answer your question about that name_ variable: it's not a leftover, it's initialized in the constructor's body.

- on a side note, what's the naming convention for member variables? I see both "variable" and "variable_", are you fine with both?

- This is still not building: the src/ directory builds fine, but examples and tests still don't; I'm leaving that for after the holidays.

Reply

Unmerged revisions

238. By Alberto Mardegan on 2015-12-23

Don't return reference to temporary

237. By Alberto Mardegan on 2015-12-23

no providers from command line

236. By Alberto Mardegan on 2015-12-23

Add override clause

235. By Alberto Mardegan on 2015-12-21

Remove is_internal method

234. By Alberto Mardegan on 2015-12-21

Use boost::fs:path in public methods

233. By Alberto Mardegan on 2015-12-21

Make load_providers() stateless

232. By Alberto Mardegan on 2015-12-21

Pass dir list in constructor

231. By Alberto Mardegan on 2015-12-21

Simplify consdruction of path list

230. By Alberto Mardegan on 2015-12-21

Remove provider_options from load_providers

229. By Alberto Mardegan on 2015-12-21

Move provider_loader to src/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/location_service/com/ubuntu/location/default_provider_selection_policy.h'
2--- include/location_service/com/ubuntu/location/default_provider_selection_policy.h 2013-12-14 09:10:29 +0000
3+++ include/location_service/com/ubuntu/location/default_provider_selection_policy.h 2015-12-23 14:51:28 +0000
4@@ -34,19 +34,19 @@
5
6 ProviderSelection determine_provider_selection_for_criteria(
7 const Criteria& criteria,
8- const ProviderEnumerator& enumerator);
9+ const ProviderCollection& collection);
10
11 Provider::Ptr determine_position_updates_provider(
12 const Criteria& criteria,
13- const ProviderEnumerator& enumerator);
14+ const ProviderCollection& collection);
15
16 Provider::Ptr determine_heading_updates_provider(
17 const Criteria& criteria,
18- const ProviderEnumerator& enumerator);
19+ const ProviderCollection& collection);
20
21 Provider::Ptr determine_velocity_updates_provider(
22 const Criteria& criteria,
23- const ProviderEnumerator& enumerator);
24+ const ProviderCollection& collection);
25 };
26 }
27 }
28
29=== renamed file 'include/location_service/com/ubuntu/location/provider_enumerator.h' => 'include/location_service/com/ubuntu/location/provider_collection.h'
30--- include/location_service/com/ubuntu/location/provider_enumerator.h 2013-12-14 09:10:29 +0000
31+++ include/location_service/com/ubuntu/location/provider_collection.h 2015-12-23 14:51:28 +0000
32@@ -15,8 +15,8 @@
33 *
34 * Authored by: Thomas Voß <thomas.voss@canonical.com>
35 */
36-#ifndef LOCATION_SERVICE_COM_UBUNTU_PROVIDER_ENUMERATOR_H_
37-#define LOCATION_SERVICE_COM_UBUNTU_PROVIDER_ENUMERATOR_H_
38+#ifndef LOCATION_SERVICE_COM_UBUNTU_PROVIDER_COLLECTION_H_
39+#define LOCATION_SERVICE_COM_UBUNTU_PROVIDER_COLLECTION_H_
40
41 #include <functional>
42 #include <memory>
43@@ -28,22 +28,44 @@
44 namespace location
45 {
46 class Provider;
47-class ProviderEnumerator
48+class ProviderCollection
49 {
50 public:
51- ProviderEnumerator(const ProviderEnumerator&) = delete;
52- virtual ~ProviderEnumerator() = default;
53-
54- ProviderEnumerator& operator=(const ProviderEnumerator&) = delete;
55- bool operator==(const ProviderEnumerator&) const = delete;
56-
57- virtual void for_each_provider(const std::function<void(const std::shared_ptr<Provider>&)>&) const = 0;
58+ ProviderCollection(const ProviderCollection&) = delete;
59+ virtual ~ProviderCollection() = default;
60+
61+ ProviderCollection& operator=(const ProviderCollection&) = delete;
62+ bool operator==(const ProviderCollection&) const = delete;
63+
64+ /**
65+ * @brief Checks if the engine knows about a specific provider.
66+ * @return True iff the engine knows about the provider.
67+ */
68+ virtual bool has_provider(const std::shared_ptr<Provider>& provider) noexcept = 0;
69+
70+ /**
71+ * @brief Makes a provider known to the engine.
72+ * @param provider The new provider.
73+ */
74+ virtual void add_provider(const std::shared_ptr<Provider>& provider) = 0;
75+
76+ /**
77+ * @brief Removes a provider from the engine.
78+ * @param provider The provider to be removed.
79+ */
80+ virtual void remove_provider(const std::shared_ptr<Provider>& provider) noexcept = 0;
81+
82+ /**
83+ * @brief Iterates all known providers and invokes the provided enumerator for each of them.
84+ * @param enumerator The functor to be invoked for each provider.
85+ */
86+ virtual void for_each_provider(const std::function<void(const std::shared_ptr<Provider>&)>&) const noexcept = 0;
87
88 protected:
89- ProviderEnumerator() = default;
90+ ProviderCollection() = default;
91 };
92 }
93 }
94 }
95
96-#endif // LOCATION_SERVICE_COM_UBUNTU_PROVIDER_ENUMERATOR_H_
97+#endif // LOCATION_SERVICE_COM_UBUNTU_PROVIDER_COLLECTION_H_
98
99=== modified file 'include/location_service/com/ubuntu/location/provider_selection_policy.h'
100--- include/location_service/com/ubuntu/location/provider_selection_policy.h 2014-06-20 07:40:34 +0000
101+++ include/location_service/com/ubuntu/location/provider_selection_policy.h 2015-12-23 14:51:28 +0000
102@@ -18,7 +18,7 @@
103 #ifndef LOCATION_SERVICE_COM_UBUNTU_PROVIDER_SELECTION_POLICY_H_
104 #define LOCATION_SERVICE_COM_UBUNTU_PROVIDER_SELECTION_POLICY_H_
105
106-#include <com/ubuntu/location/provider_enumerator.h>
107+#include <com/ubuntu/location/provider_collection.h>
108 #include <com/ubuntu/location/provider_selection.h>
109
110 #include <memory>
111@@ -30,7 +30,6 @@
112 namespace location
113 {
114 struct Criteria;
115-class Engine;
116
117 class ProviderSelectionPolicy
118 {
119@@ -45,7 +44,7 @@
120
121 virtual ProviderSelection determine_provider_selection_for_criteria(
122 const Criteria& criteria,
123- const ProviderEnumerator& enumerator) = 0;
124+ const ProviderCollection& enumerator) = 0;
125
126 protected:
127 ProviderSelectionPolicy() = default;
128
129=== modified file 'src/location_service/com/ubuntu/location/CMakeLists.txt'
130--- src/location_service/com/ubuntu/location/CMakeLists.txt 2015-04-23 14:48:44 +0000
131+++ src/location_service/com/ubuntu/location/CMakeLists.txt 2015-12-23 14:51:28 +0000
132@@ -20,10 +20,12 @@
133
134 criteria.cpp
135 engine.cpp
136+ filesystem_provider_loader.cpp
137 init_and_shutdown.cpp
138 position.cpp
139 provider.cpp
140 provider_factory.cpp
141+ provider_manifest.cpp
142 proxy_provider.cpp
143 satellite_based_positioning_state.cpp
144 settings.cpp
145
146=== modified file 'src/location_service/com/ubuntu/location/default_provider_selection_policy.cpp'
147--- src/location_service/com/ubuntu/location/default_provider_selection_policy.cpp 2014-06-20 07:40:34 +0000
148+++ src/location_service/com/ubuntu/location/default_provider_selection_policy.cpp 2015-12-23 14:51:28 +0000
149@@ -50,13 +50,13 @@
150 cul::ProviderSelection
151 cul::DefaultProviderSelectionPolicy::determine_provider_selection_for_criteria(
152 const cul::Criteria& criteria,
153- const cul::ProviderEnumerator& enumerator)
154+ const cul::ProviderCollection& collection)
155 {
156 ProviderSelection selection
157 {
158- determine_position_updates_provider(criteria, enumerator),
159- determine_heading_updates_provider(criteria, enumerator),
160- determine_velocity_updates_provider(criteria, enumerator)
161+ determine_position_updates_provider(criteria, collection),
162+ determine_heading_updates_provider(criteria, collection),
163+ determine_velocity_updates_provider(criteria, collection)
164 };
165
166 return selection;
167@@ -65,7 +65,7 @@
168 cul::Provider::Ptr
169 cul::DefaultProviderSelectionPolicy::determine_position_updates_provider(
170 const cul::Criteria& criteria,
171- const cul::ProviderEnumerator& enumerator)
172+ const cul::ProviderCollection& collection)
173 {
174 auto less =
175 [](const Provider::Ptr& lhs, const Provider::Ptr& rhs)
176@@ -78,7 +78,7 @@
177 Provider::Ptr,
178 std::function<bool(const Provider::Ptr&, const Provider::Ptr&)>> matching_providers(less);
179
180- enumerator.for_each_provider(
181+ collection.for_each_provider(
182 [&](const Provider::Ptr& provider)
183 {
184 if (provider->matches_criteria(criteria))
185@@ -90,7 +90,7 @@
186
187 cul::Provider::Ptr cul::DefaultProviderSelectionPolicy::determine_heading_updates_provider(
188 const cul::Criteria& criteria,
189- const cul::ProviderEnumerator& enumerator)
190+ const cul::ProviderCollection& collection)
191 {
192 auto less =
193 [](const Provider::Ptr& lhs, const Provider::Ptr& rhs)
194@@ -102,7 +102,7 @@
195 Provider::Ptr,
196 std::function<bool(const Provider::Ptr&, const Provider::Ptr&)>> matching_providers(less);
197
198- enumerator.for_each_provider(
199+ collection.for_each_provider(
200 [&](const Provider::Ptr& provider)
201 {
202 if (provider->matches_criteria(criteria))
203@@ -114,7 +114,7 @@
204
205 cul::Provider::Ptr cul::DefaultProviderSelectionPolicy::determine_velocity_updates_provider(
206 const cul::Criteria& criteria,
207- const cul::ProviderEnumerator& enumerator)
208+ const cul::ProviderCollection& collection)
209 {
210 auto less =
211 [](const Provider::Ptr& lhs, const Provider::Ptr& rhs)
212@@ -126,7 +126,7 @@
213 Provider::Ptr,
214 std::function<bool(const Provider::Ptr&, const Provider::Ptr&)>> matching_providers(less);
215
216- enumerator.for_each_provider(
217+ collection.for_each_provider(
218 [&](const Provider::Ptr& provider)
219 {
220 if (provider->matches_criteria(criteria))
221
222=== modified file 'src/location_service/com/ubuntu/location/engine.h'
223--- src/location_service/com/ubuntu/location/engine.h 2015-04-23 14:48:44 +0000
224+++ src/location_service/com/ubuntu/location/engine.h 2015-12-23 14:51:28 +0000
225@@ -19,7 +19,7 @@
226 #define LOCATION_SERVICE_COM_UBUNTU_LOCATION_ENGINE_H_
227
228 #include <com/ubuntu/location/provider.h>
229-#include <com/ubuntu/location/provider_enumerator.h>
230+#include <com/ubuntu/location/provider_collection.h>
231 #include <com/ubuntu/location/provider_selection.h>
232 #include <com/ubuntu/location/provider_selection_policy.h>
233 #include <com/ubuntu/location/satellite_based_positioning_state.h>
234@@ -47,7 +47,7 @@
235 * @brief The Engine class encapsulates a positioning engine, relying on a set
236 * of providers and reporters to acquire and publish location information.
237 */
238-class Engine : public ProviderEnumerator
239+class Engine : public ProviderCollection
240 {
241 public:
242 typedef std::shared_ptr<Engine> Ptr;
243@@ -150,29 +150,10 @@
244 */
245 virtual ProviderSelection determine_provider_selection_for_criteria(const Criteria& criteria);
246
247- /**
248- * @brief Checks if the engine knows about a specific provider.
249- * @return True iff the engine knows about the provider.
250- */
251- virtual bool has_provider(const Provider::Ptr& provider) noexcept;
252-
253- /**
254- * @brief Makes a provider known to the engine.
255- * @param provider The new provider.
256- */
257- virtual void add_provider(const Provider::Ptr& provider);
258-
259- /**
260- * @brief Removes a provider from the engine.
261- * @param provider The provider to be removed.
262- */
263- virtual void remove_provider(const Provider::Ptr& provider) noexcept;
264-
265- /**
266- * @brief Iterates all known providers and invokes the provided enumerator for each of them.
267- * @param enumerator The functor to be invoked for each provider.
268- */
269- virtual void for_each_provider(const std::function<void(const Provider::Ptr&)>& enumerator) const noexcept;
270+ virtual bool has_provider(const Provider::Ptr& provider) noexcept override;
271+ virtual void add_provider(const Provider::Ptr& provider) override;
272+ virtual void remove_provider(const Provider::Ptr& provider) noexcept override;
273+ virtual void for_each_provider(const std::function<void(const Provider::Ptr&)>& enumerator) const noexcept override;
274
275 /** @brief The engine's configuration. */
276 Configuration configuration;
277
278=== added file 'src/location_service/com/ubuntu/location/filesystem_provider_loader.cpp'
279--- src/location_service/com/ubuntu/location/filesystem_provider_loader.cpp 1970-01-01 00:00:00 +0000
280+++ src/location_service/com/ubuntu/location/filesystem_provider_loader.cpp 2015-12-23 14:51:28 +0000
281@@ -0,0 +1,97 @@
282+/*
283+ * Copyright © 2015 Canonical Ltd.
284+ *
285+ * This program is free software: you can redistribute it and/or modify it
286+ * under the terms of the GNU Lesser General Public License version 3,
287+ * as published by the Free Software Foundation.
288+ *
289+ * This program is distributed in the hope that it will be useful,
290+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
291+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
292+ * GNU Lesser General Public License for more details.
293+ *
294+ * You should have received a copy of the GNU Lesser General Public License
295+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
296+ *
297+ * Authored by: Alberto Mardegan <alberto.mardegan@canonical.com>
298+ */
299+#include "filesystem_provider_loader.h"
300+
301+#include <com/ubuntu/location/logging.h>
302+#include <com/ubuntu/location/provider_collection.h>
303+#include <com/ubuntu/location/provider_factory.h>
304+#include <com/ubuntu/location/provider_manifest.h>
305+
306+namespace cul = com::ubuntu::location;
307+
308+cul::FilesystemProviderLoader::FilesystemProviderLoader(const std::set<boost::filesystem::path>& data_dirs)
309+ : ProviderLoader()
310+ , data_dirs(data_dirs)
311+{
312+}
313+
314+cul::FilesystemProviderLoader::~FilesystemProviderLoader()
315+{
316+}
317+
318+void cul::FilesystemProviderLoader::load_providers(const std::shared_ptr<ProviderCollection>& collection)
319+{
320+ std::set<std::string> instantiated_providers;
321+ for (const auto& dir: data_dirs)
322+ {
323+ load_providers_from_dir(dir, collection, instantiated_providers);
324+ }
325+}
326+
327+void cul::FilesystemProviderLoader::load_providers_from_dir(const boost::filesystem::path& dir,
328+ const std::shared_ptr<ProviderCollection>& collection,
329+ std::set<std::string>& instantiated_providers)
330+{
331+ VLOG(2) << "Looking for providers in " << dir;
332+ try
333+ {
334+ if (!boost::filesystem::is_directory(dir))
335+ return;
336+ }
337+ catch (const boost::filesystem::filesystem_error& e)
338+ {
339+ VLOG(1) << e.what();
340+ return;
341+ }
342+
343+ for (auto entry: boost::filesystem::directory_iterator(dir))
344+ {
345+ if (entry.path().extension() == ".json")
346+ {
347+ load_provider(entry.path(), collection, instantiated_providers);
348+ }
349+ }
350+}
351+
352+void cul::FilesystemProviderLoader::load_provider(const boost::filesystem::path& p,
353+ const std::shared_ptr<ProviderCollection>& collection,
354+ std::set<std::string>& instantiated_providers)
355+{
356+ VLOG(1) << "Loading provider manifest: " << p;
357+ ProviderManifest manifest(p);
358+ std::string name = manifest.name();
359+ std::string plugin_name = manifest.plugin_name();
360+ if (name.empty() || plugin_name.empty()) return;
361+
362+ if (instantiated_providers.find(name) != instantiated_providers.end())
363+ {
364+ VLOG(0) << "Provider already loaded: " << name;
365+ return;
366+ }
367+
368+ ProviderFactory& factory = ProviderFactory::instance();
369+
370+ std::string provider_name = plugin_name + "::Provider";
371+
372+ VLOG(0) << "Instantiating and configuring: " << provider_name;
373+ auto provider =
374+ factory.create_provider_for_name_with_config(provider_name,
375+ manifest.configuration());
376+ collection->add_provider(provider);
377+ instantiated_providers.insert(name);
378+}
379
380=== added file 'src/location_service/com/ubuntu/location/filesystem_provider_loader.h'
381--- src/location_service/com/ubuntu/location/filesystem_provider_loader.h 1970-01-01 00:00:00 +0000
382+++ src/location_service/com/ubuntu/location/filesystem_provider_loader.h 2015-12-23 14:51:28 +0000
383@@ -0,0 +1,69 @@
384+/*
385+ * Copyright © 2015 Canonical Ltd.
386+ *
387+ * This program is free software: you can redistribute it and/or modify it
388+ * under the terms of the GNU Lesser General Public License version 3,
389+ * as published by the Free Software Foundation.
390+ *
391+ * This program is distributed in the hope that it will be useful,
392+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
393+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
394+ * GNU Lesser General Public License for more details.
395+ *
396+ * You should have received a copy of the GNU Lesser General Public License
397+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
398+ *
399+ * Authored by: Alberto Mardegan <alberto.mardegan@canonical.com>
400+ */
401+#ifndef LOCATION_SERVICE_COM_UBUNTU_LOCATION_FILESYSTEM_PROVIDER_LOADER_H_
402+#define LOCATION_SERVICE_COM_UBUNTU_LOCATION_FILESYSTEM_PROVIDER_LOADER_H_
403+
404+#include <com/ubuntu/location/provider.h>
405+#include <com/ubuntu/location/provider_collection.h>
406+#include <com/ubuntu/location/provider_loader.h>
407+
408+#include <com/ubuntu/location/settings.h>
409+
410+#include <boost/filesystem.hpp>
411+
412+#include <set>
413+
414+namespace com
415+{
416+namespace ubuntu
417+{
418+namespace location
419+{
420+/**
421+ * @brief The FilesystemProviderLoader class is used to enumerate and load all
422+ * available providers into a ProviderCollection.
423+ */
424+class FilesystemProviderLoader : public ProviderLoader
425+{
426+public:
427+ typedef std::shared_ptr<FilesystemProviderLoader> Ptr;
428+
429+ FilesystemProviderLoader(const std::set<boost::filesystem::path>& data_dirs);
430+
431+ FilesystemProviderLoader(const FilesystemProviderLoader&) = delete;
432+ FilesystemProviderLoader& operator=(const FilesystemProviderLoader&) = delete;
433+ virtual ~FilesystemProviderLoader();
434+
435+ virtual void load_providers(const std::shared_ptr<ProviderCollection>& collection) override;
436+
437+private:
438+ void load_providers_from_dir(const boost::filesystem::path& dir,
439+ const std::shared_ptr<ProviderCollection>& collection,
440+ std::set<std::string>& instantiated_providers);
441+ void load_provider(const boost::filesystem::path& p,
442+ const std::shared_ptr<ProviderCollection>& collection,
443+ std::set<std::string>& instantiated_providers);
444+
445+ std::set<boost::filesystem::path> data_dirs;
446+};
447+
448+}
449+}
450+}
451+
452+#endif // LOCATION_SERVICE_COM_UBUNTU_LOCATION_FILESYSTEM_PROVIDER_LOADER_H_
453
454=== modified file 'src/location_service/com/ubuntu/location/non_selecting_provider_selection_policy.cpp'
455--- src/location_service/com/ubuntu/location/non_selecting_provider_selection_policy.cpp 2015-04-23 14:48:44 +0000
456+++ src/location_service/com/ubuntu/location/non_selecting_provider_selection_policy.cpp 2015-12-23 14:51:28 +0000
457@@ -143,11 +143,11 @@
458
459 location::ProviderSelection location::NonSelectingProviderSelectionPolicy::determine_provider_selection_for_criteria(
460 const location::Criteria&,
461- const location::ProviderEnumerator& enumerator)
462+ const location::ProviderCollection& collection)
463 {
464 // We put all providers in a set.
465 std::set<location::Provider::Ptr> bag;
466- enumerator.for_each_provider([&bag](const location::Provider::Ptr& provider)
467+ collection.for_each_provider([&bag](const location::Provider::Ptr& provider)
468 {
469 bag.insert(provider);
470 });
471
472=== modified file 'src/location_service/com/ubuntu/location/non_selecting_provider_selection_policy.h'
473--- src/location_service/com/ubuntu/location/non_selecting_provider_selection_policy.h 2014-09-18 11:15:35 +0000
474+++ src/location_service/com/ubuntu/location/non_selecting_provider_selection_policy.h 2015-12-23 14:51:28 +0000
475@@ -33,7 +33,7 @@
476 // to actually select providers based on requirements, features and criteria.
477 struct NonSelectingProviderSelectionPolicy : public ProviderSelectionPolicy
478 {
479- ProviderSelection determine_provider_selection_for_criteria(const Criteria&, const ProviderEnumerator&) override;
480+ ProviderSelection determine_provider_selection_for_criteria(const Criteria&, const ProviderCollection&) override;
481 };
482 }
483 }
484
485=== added file 'src/location_service/com/ubuntu/location/provider_loader.h'
486--- src/location_service/com/ubuntu/location/provider_loader.h 1970-01-01 00:00:00 +0000
487+++ src/location_service/com/ubuntu/location/provider_loader.h 2015-12-23 14:51:28 +0000
488@@ -0,0 +1,54 @@
489+/*
490+ * Copyright © 2015 Canonical Ltd.
491+ *
492+ * This program is free software: you can redistribute it and/or modify it
493+ * under the terms of the GNU Lesser General Public License version 3,
494+ * as published by the Free Software Foundation.
495+ *
496+ * This program is distributed in the hope that it will be useful,
497+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
498+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
499+ * GNU Lesser General Public License for more details.
500+ *
501+ * You should have received a copy of the GNU Lesser General Public License
502+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
503+ *
504+ * Authored by: Alberto Mardegan <alberto.mardegan@canonical.com>
505+ */
506+#ifndef LOCATION_SERVICE_COM_UBUNTU_PROVIDER_LOADER_H_
507+#define LOCATION_SERVICE_COM_UBUNTU_PROVIDER_LOADER_H_
508+
509+#include <com/ubuntu/location/configuration.h>
510+
511+#include <memory>
512+
513+namespace com
514+{
515+namespace ubuntu
516+{
517+namespace location
518+{
519+class ProviderCollection;
520+class ProviderLoader
521+{
522+public:
523+ ProviderLoader(const ProviderLoader&) = delete;
524+ virtual ~ProviderLoader() = default;
525+
526+ ProviderLoader& operator=(const ProviderLoader&) = delete;
527+ bool operator==(const ProviderLoader&) const = delete;
528+
529+ /**
530+ * @brief Loads available providers.
531+ * @param collection The provider collection where found providers will be added.
532+ */
533+ virtual void load_providers(const std::shared_ptr<ProviderCollection>& collection) = 0;
534+
535+protected:
536+ ProviderLoader() = default;
537+};
538+}
539+}
540+}
541+
542+#endif // LOCATION_SERVICE_COM_UBUNTU_PROVIDER_LOADER_H_
543
544=== added file 'src/location_service/com/ubuntu/location/provider_manifest.cpp'
545--- src/location_service/com/ubuntu/location/provider_manifest.cpp 1970-01-01 00:00:00 +0000
546+++ src/location_service/com/ubuntu/location/provider_manifest.cpp 2015-12-23 14:51:28 +0000
547@@ -0,0 +1,45 @@
548+/*
549+ * Copyright © 2015 Canonical Ltd.
550+ *
551+ * This program is free software: you can redistribute it and/or modify it
552+ * under the terms of the GNU Lesser General Public License version 3,
553+ * as published by the Free Software Foundation.
554+ *
555+ * This program is distributed in the hope that it will be useful,
556+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
557+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
558+ * GNU Lesser General Public License for more details.
559+ *
560+ * You should have received a copy of the GNU Lesser General Public License
561+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
562+ *
563+ * Authored by: Alberto Mardegan <alberto.mardegan@canonical.com>
564+ */
565+#include "provider_manifest.h"
566+
567+#include <com/ubuntu/location/logging.h>
568+
569+#include <boost/property_tree/json_parser.hpp>
570+
571+namespace cul = com::ubuntu::location;
572+
573+cul::ProviderManifest::ProviderManifest(const boost::filesystem::path& p)
574+{
575+ try
576+ {
577+ boost::property_tree::read_json(p.string(), tree);
578+ }
579+ catch (boost::property_tree::json_parser_error& e)
580+ {
581+ LOG(WARNING) << "Could not parse manifest file " << p << ": " << e.what();
582+ return;
583+ }
584+
585+ name_ = p.stem().string();
586+ plugin_name_ = tree.get(Keys::plugin, std::string());
587+}
588+
589+const cul::Configuration &cul::ProviderManifest::configuration() const
590+{
591+ return tree.get_child(Keys::configuration, empty_configuration);
592+}
593
594=== added file 'src/location_service/com/ubuntu/location/provider_manifest.h'
595--- src/location_service/com/ubuntu/location/provider_manifest.h 1970-01-01 00:00:00 +0000
596+++ src/location_service/com/ubuntu/location/provider_manifest.h 2015-12-23 14:51:28 +0000
597@@ -0,0 +1,76 @@
598+/*
599+ * Copyright © 2015 Canonical Ltd.
600+ *
601+ * This program is free software: you can redistribute it and/or modify it
602+ * under the terms of the GNU Lesser General Public License version 3,
603+ * as published by the Free Software Foundation.
604+ *
605+ * This program is distributed in the hope that it will be useful,
606+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
607+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
608+ * GNU Lesser General Public License for more details.
609+ *
610+ * You should have received a copy of the GNU Lesser General Public License
611+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
612+ *
613+ * Authored by: Alberto Mardegan <alberto.mardegan@canonical.com>
614+ */
615+#ifndef LOCATION_SERVICE_COM_UBUNTU_LOCATION_PROVIDER_MANIFEST_H_
616+#define LOCATION_SERVICE_COM_UBUNTU_LOCATION_PROVIDER_MANIFEST_H_
617+
618+#include <boost/filesystem.hpp>
619+#include <boost/property_tree/ptree.hpp>
620+
621+#include <com/ubuntu/location/configuration.h>
622+
623+namespace com
624+{
625+namespace ubuntu
626+{
627+namespace location
628+{
629+/**
630+ * @brief The ProviderManifest class reads and parses provider manifest files.
631+ */
632+class ProviderManifest
633+{
634+public:
635+ /**
636+ * @brief Loads a provider manifest file.
637+ */
638+ ProviderManifest(const boost::filesystem::path& p);
639+ ProviderManifest(const ProviderManifest&) = delete;
640+ ProviderManifest& operator=(const ProviderManifest&) = delete;
641+ virtual ~ProviderManifest() = default;
642+
643+ const std::string& name() const { return name_; }
644+ const std::string& plugin_name() const { return plugin_name_; }
645+
646+ const Configuration &configuration() const;
647+
648+private:
649+ /** Keys used in provider manifest files. */
650+ struct Keys
651+ {
652+ /** Key for plugin name */
653+ static constexpr const char* plugin
654+ {
655+ "plugin"
656+ };
657+ /** Key for provider configuration */
658+ static constexpr const char* configuration
659+ {
660+ "configuration"
661+ };
662+ };
663+ boost::property_tree::ptree tree;
664+ std::string name_;
665+ std::string plugin_name_;
666+ Configuration empty_configuration;
667+};
668+
669+}
670+}
671+}
672+
673+#endif // LOCATION_SERVICE_COM_UBUNTU_LOCATION_PROVIDER_MANIFEST_H_
674
675=== modified file 'src/location_service/com/ubuntu/location/service/daemon.cpp'
676--- src/location_service/com/ubuntu/location/service/daemon.cpp 2015-04-16 10:03:29 +0000
677+++ src/location_service/com/ubuntu/location/service/daemon.cpp 2015-12-23 14:51:28 +0000
678@@ -18,6 +18,7 @@
679
680 #include <com/ubuntu/location/logging.h>
681 #include <com/ubuntu/location/boost_ptree_settings.h>
682+#include <com/ubuntu/location/filesystem_provider_loader.h>
683 #include <com/ubuntu/location/provider_factory.h>
684
685 #include <com/ubuntu/location/service/default_configuration.h>
686@@ -28,6 +29,8 @@
687
688 #include <com/ubuntu/location/service/runtime_tests.h>
689
690+#include <boost/filesystem.hpp>
691+
692 #include "program_options.h"
693 #include "daemon.h"
694
695@@ -108,49 +111,6 @@
696 result.incoming = factory(mutable_daemon_options().bus());
697 result.outgoing = factory(mutable_daemon_options().bus());
698
699- if (mutable_daemon_options().value_count_for_key("testing") == 0 && mutable_daemon_options().value_count_for_key("provider") == 0)
700- {
701- std::stringstream ss;
702- ss << "A set of providers need to be specified. The following providers are known:" << std::endl;
703- location::ProviderFactory::instance().enumerate(
704- [&ss](const std::string& name, const location::ProviderFactory::Factory&)
705- {
706- ss << "\t" << name << std::endl;
707- });
708- throw std::runtime_error{ss.str()};
709- }
710-
711- if(mutable_daemon_options().value_count_for_key("provider") > 0)
712- {
713- result.providers = mutable_daemon_options().value_for_key<std::vector<std::string>>("provider");
714-
715- for (const std::string& provider : result.providers)
716- {
717- mutable_daemon_options().enumerate_unrecognized_options(
718- [&result, provider](const std::string& s)
719- {
720- std::stringstream in(s);
721- std::string key, value;
722-
723- std::getline(in, key, '=');
724- std::getline(in, value, '=');
725-
726- std::size_t pos = key.find(provider);
727- if (pos == std::string::npos)
728- return;
729- static const std::string option_marker{"--"};
730- static const std::string scope_separator{"::"};
731- key = key.erase(key.find_first_of(option_marker), option_marker.size());
732- key = key.erase(key.find_first_of(provider), provider.size());
733- key = key.erase(key.find_first_of(scope_separator), scope_separator.size());
734-
735- std::cout << "\t" << key << " -> " << value << std::endl;
736-
737- result.provider_options[provider].put(key, value);
738- });
739- }
740- }
741-
742 auto settings = std::make_shared<location::BoostPtreeSettings>(mutable_daemon_options().value_for_key<std::string>("config-file"));
743 result.settings = std::make_shared<location::SyncingSettings>(settings);
744
745@@ -175,32 +135,6 @@
746 trap->stop();
747 });
748
749- const location::Configuration empty_provider_configuration;
750-
751- std::set<location::Provider::Ptr> instantiated_providers;
752-
753- for (const std::string& provider : config.providers)
754- {
755- std::cout << "Instantiating and configuring: " << provider << std::endl;
756-
757- try
758- {
759- auto p = location::ProviderFactory::instance().create_provider_for_name_with_config(
760- provider,
761- config.provider_options.count(provider) > 0 ?
762- config.provider_options.at(provider) : empty_provider_configuration);
763-
764- if (p)
765- instantiated_providers.insert(p);
766- else
767- throw std::runtime_error("Problem instantiating provider");
768-
769- } catch(const std::runtime_error& e)
770- {
771- std::cerr << "Issue instantiating provider: " << e.what() << std::endl;
772- }
773- }
774-
775 config.incoming->install_executor(dbus::asio::make_executor(config.incoming));
776 config.outgoing->install_executor(dbus::asio::make_executor(config.outgoing));
777
778@@ -210,7 +144,7 @@
779 {
780 config.incoming,
781 config.outgoing,
782- dc.the_engine(instantiated_providers, dc.the_provider_selection_policy(), config.settings),
783+ dc.the_engine(dc.the_provider_selection_policy(), config.settings),
784 dc.the_permission_manager(config.outgoing),
785 location::service::Harvester::Configuration
786 {
787@@ -240,6 +174,16 @@
788 }
789 };
790
791+ // TODO: Use xdg::Data once available as a shared library
792+ namespace fs = boost::filesystem;
793+ auto data_dirs = {
794+ fs::path{std::getenv("HOME")} / ".local/share/location/providers",
795+ fs::path("/usr/share/location/providers"),
796+ };
797+
798+ FilesystemProviderLoader loader(data_dirs);
799+ loader.load_providers(configuration.engine);
800+
801 auto location_service = std::make_shared<location::service::Implementation>(configuration);
802 // We need to ensure that any exception raised by the executor does not crash the app
803 // and also gets logged.
804
805=== modified file 'src/location_service/com/ubuntu/location/service/daemon.h'
806--- src/location_service/com/ubuntu/location/service/daemon.h 2015-01-25 12:45:30 +0000
807+++ src/location_service/com/ubuntu/location/service/daemon.h 2015-12-23 14:51:28 +0000
808@@ -152,10 +152,6 @@
809 {
810 false
811 };
812- /** @brief Providers that have been requested on the command line. */
813- std::vector<std::string> providers;
814- /** @brief Provider-specific options keyed on the provider name. */
815- std::map< std::string, location::Configuration > provider_options;
816 /** @brief Settings instance to read values from. */
817 Settings::Ptr settings;
818 };
819
820=== modified file 'src/location_service/com/ubuntu/location/service/default_configuration.cpp'
821--- src/location_service/com/ubuntu/location/service/default_configuration.cpp 2014-11-14 11:33:05 +0000
822+++ src/location_service/com/ubuntu/location/service/default_configuration.cpp 2015-12-23 14:51:28 +0000
823@@ -26,15 +26,10 @@
824 namespace culs = com::ubuntu::location::service;
825
826 cul::Engine::Ptr culs::DefaultConfiguration::the_engine(
827- const std::set<cul::Provider::Ptr>& provider_set,
828 const cul::ProviderSelectionPolicy::Ptr& provider_selection_policy,
829 const cul::Settings::Ptr& settings)
830 {
831- auto engine = std::make_shared<cul::Engine>(provider_selection_policy, settings);
832- for (const auto& provider : provider_set)
833- engine->add_provider(provider);
834-
835- return engine;
836+ return std::make_shared<cul::Engine>(provider_selection_policy, settings);
837 }
838
839 cul::ProviderSelectionPolicy::Ptr culs::DefaultConfiguration::the_provider_selection_policy()
840
841=== modified file 'src/location_service/com/ubuntu/location/service/default_configuration.h'
842--- src/location_service/com/ubuntu/location/service/default_configuration.h 2014-11-14 11:33:05 +0000
843+++ src/location_service/com/ubuntu/location/service/default_configuration.h 2015-12-23 14:51:28 +0000
844@@ -51,7 +51,6 @@
845 // Creates an engine instance given a set of providers, a provider selection policy
846 // and a settings instance.
847 virtual std::shared_ptr<Engine> the_engine(
848- const std::set<Provider::Ptr>& provider_set,
849 const ProviderSelectionPolicy::Ptr& provider_selection_policy,
850 const Settings::Ptr& settings);
851
852
853=== modified file 'tests/engine_test.cpp'
854--- tests/engine_test.cpp 2015-01-21 20:04:56 +0000
855+++ tests/engine_test.cpp 2015-12-23 14:51:28 +0000
856@@ -110,7 +110,7 @@
857 MOCK_METHOD2(determine_provider_selection_for_criteria,
858 location::ProviderSelection(
859 const location::Criteria&,
860- const location::ProviderEnumerator&));
861+ const location::ProviderCollection&));
862 };
863 }
864
865
866=== modified file 'tests/null_provider_selection_policy.h'
867--- tests/null_provider_selection_policy.h 2014-06-01 17:56:58 +0000
868+++ tests/null_provider_selection_policy.h 2015-12-23 14:51:28 +0000
869@@ -26,7 +26,7 @@
870 com::ubuntu::location::ProviderSelection
871 determine_provider_selection_for_criteria(
872 const com::ubuntu::location::Criteria&,
873- const com::ubuntu::location::ProviderEnumerator&)
874+ const com::ubuntu::location::ProviderCollection&)
875 {
876 return com::ubuntu::location::ProviderSelection
877 {
878
879=== modified file 'tests/provider_selection_policy_test.cpp'
880--- tests/provider_selection_policy_test.cpp 2014-09-18 11:15:35 +0000
881+++ tests/provider_selection_policy_test.cpp 2015-12-23 14:51:28 +0000
882@@ -10,7 +10,7 @@
883
884 namespace
885 {
886-struct ProviderSetEnumerator : public cul::ProviderEnumerator
887+struct ProviderSetEnumerator : public cul::ProviderCollection
888 {
889 ProviderSetEnumerator(const std::set<std::shared_ptr<cul::Provider>>& providers)
890 : providers(providers)

Subscribers

People subscribed via source and target branches