unity-scopes-api

Merge lp:unity-scopes-api/staging into lp:unity-scopes-api

  1. staging
  2. Merge into trunk
Proposed by Paweł Stołowski
Status: Merged
Approved by: Marcus Tomlinson
Approved revision: 297
Merged at revision: 336
Proposed branch: lp:unity-scopes-api/staging
Merge into: lp:unity-scopes-api
Diff against target: 367 lines (+55/-65)
8 files modified
include/unity/scopes/Result.h (+5/-0)
include/unity/scopes/internal/SettingsDB.h (+2/-1)
src/scopes/internal/JsonCppNode.cpp (+10/-3)
src/scopes/internal/SettingsDB.cpp (+6/-5)
test/gtest/scopes/ResultCache/CacheScope.cpp (+5/-0)
test/gtest/scopes/ResultCache/ResultCache_test.cpp (+6/-0)
test/gtest/scopes/internal/CMakeLists.txt (+1/-1)
test/gtest/scopes/internal/SettingsDB/SettingsDB_test.cpp (+20/-55)
To merge this branch: bzr merge lp:unity-scopes-api/staging
Reviewer Review Type Date Requested Status
Marcus Tomlinson (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+264287@code.launchpad.net

Commit message

Merged devel.

Description of the change

Merged devel.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:296
http://jenkins.qa.ubuntu.com/job/unity-scopes-api-ci/626/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-wily-amd64-ci/14
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-wily-armhf-ci/14
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-wily-armhf-ci/14/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-wily-i386-ci/14

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-api-ci/626/rebuild

review: Approve (continuous-integration)
lp:unity-scopes-api/staging updated
297. By Paweł Stołowski

Merged devel

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:297
http://jenkins.qa.ubuntu.com/job/unity-scopes-api-ci/627/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-wily-amd64-ci/15
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-wily-armhf-ci/15
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-wily-armhf-ci/15/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-wily-i386-ci/15

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-api-ci/627/rebuild

review: Approve (continuous-integration)
Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

ACK

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/unity/scopes/Result.h'
2--- include/unity/scopes/Result.h 2015-04-15 10:05:42 +0000
3+++ include/unity/scopes/Result.h 2015-07-14 09:21:51 +0000
4@@ -217,6 +217,11 @@
5 /**
6 \brief Get the value of an attribute.
7
8+ Note: if int64_t value has been stored in the Result but it doesn't exceed maxium range of 32 bit integer and if results come
9+ from the cache (see SearchReply::push_surfacing_results_from_cache()), then the value may be made available as 32 bit int;
10+ therefore the code should always check the type of returned Variant and depending on that use Variant::get_int() or
11+ Variant::get_int_64_t() when dealing with 64-bit integers. This is not needed when using 32 bit integers only.
12+
13 \param key The attribute name.
14 \return The attribute value.
15 \throws unity::InvalidArgumentException if given attribute hasn't been set.
16
17=== modified file 'include/unity/scopes/internal/SettingsDB.h'
18--- include/unity/scopes/internal/SettingsDB.h 2015-06-15 11:19:26 +0000
19+++ include/unity/scopes/internal/SettingsDB.h 2015-07-14 09:21:51 +0000
20@@ -74,7 +74,8 @@
21 void set_defaults();
22
23 std::string db_path_;
24- decltype(::timespec::tv_nsec) last_write_time_;
25+ decltype(::timespec::tv_nsec) last_write_time_nsec_;
26+ decltype(::timespec::tv_sec) last_write_time_sec_;
27 ::ino_t last_write_inode_;
28 VariantArray definitions_; // Returned by SettingsSchema
29 std::map<std::string, Variant> def_map_; // Allows fast access to the Variants in definitions_
30
31=== modified file 'src/scopes/internal/JsonCppNode.cpp'
32--- src/scopes/internal/JsonCppNode.cpp 2015-02-25 05:20:57 +0000
33+++ src/scopes/internal/JsonCppNode.cpp 2015-07-14 09:21:51 +0000
34@@ -53,6 +53,8 @@
35 {
36 case Variant::Type::Int:
37 return Json::Value(var.get_int());
38+ case Variant::Type::Int64:
39+ return Json::Value(static_cast<Json::Int64>(var.get_int64_t()));
40 case Variant::Type::Bool:
41 return Json::Value(var.get_bool());
42 case Variant::Type::String:
43@@ -82,7 +84,7 @@
44 default:
45 {
46 std::ostringstream s;
47- s << "json_to_variant(): unsupported json type ";
48+ s << "JsonCppNode::from_variant(): unsupported variant type ";
49 s << static_cast<int>(var.which());
50 throw unity::LogicException(s.str());
51 }
52@@ -117,7 +119,12 @@
53 return Variant(value.asString());
54 case Json::ValueType::intValue:
55 case Json::ValueType::uintValue:
56- return Variant(value.asInt()); // this can throw std::runtime_error from jsoncpp if unsigned int to int conversion is not possible
57+ // this can throw std::runtime_error from jsoncpp if unsigned int to int conversion is not possible
58+ if (value.isInt())
59+ {
60+ return Variant(value.asInt());
61+ }
62+ return Variant(static_cast<int64_t>(value.asInt64()));
63 case Json::ValueType::realValue:
64 return Variant(value.asDouble());
65 case Json::ValueType::booleanValue:
66@@ -125,7 +132,7 @@
67 default:
68 {
69 std::ostringstream s;
70- s << "json_to_variant(): unsupported json type ";
71+ s << "JsonCppNode::to_variant(): unsupported json type ";
72 s << static_cast<int>(value.type());
73 throw unity::LogicException(s.str());
74 }
75
76=== modified file 'src/scopes/internal/SettingsDB.cpp'
77--- src/scopes/internal/SettingsDB.cpp 2015-06-18 10:26:20 +0000
78+++ src/scopes/internal/SettingsDB.cpp 2015-07-14 09:21:51 +0000
79@@ -125,7 +125,8 @@
80 SettingsSchema const& schema,
81 boost::log::sources::severity_channel_logger_mt<>& logger)
82 : db_path_(db_path)
83- , last_write_time_(-1)
84+ , last_write_time_nsec_(-1)
85+ , last_write_time_sec_(-1)
86 , last_write_inode_(0)
87 , logger_(logger)
88 {
89@@ -198,9 +199,8 @@
90 FileLock lock = unix_lock(db_path_);
91 if (::fstat(lock.get(), &st) == 0) // re-stat the file
92 {
93- auto const wt = st.st_mtim.tv_nsec;
94-
95- if (wt != last_write_time_ || st.st_ino != last_write_inode_)
96+ // it's neccessary to use both sec and nsec, cause it seems that on low-res kernels tv_nsec can duplicate from second to second.
97+ if (st.st_mtim.tv_nsec != last_write_time_nsec_ || st.st_mtim.tv_sec != last_write_time_sec_ || st.st_ino != last_write_inode_)
98 {
99 // We re-establish the defaults and re-read everything. We need to put the defaults back because
100 // settings may have been deleted from the database.
101@@ -227,7 +227,8 @@
102 throw ResourceException(e.what());
103 }
104
105- last_write_time_ = wt;
106+ last_write_time_nsec_ = st.st_mtim.tv_nsec;
107+ last_write_time_sec_ = st.st_mtim.tv_sec;
108 last_write_inode_ = st.st_ino;
109
110 return;
111
112=== modified file 'test/gtest/scopes/ResultCache/CacheScope.cpp'
113--- test/gtest/scopes/ResultCache/CacheScope.cpp 2015-02-05 09:35:42 +0000
114+++ test/gtest/scopes/ResultCache/CacheScope.cpp 2015-07-14 09:21:51 +0000
115@@ -30,6 +30,7 @@
116
117 #include <atomic>
118 #include <mutex>
119+#include <cstdint>
120
121 using namespace std;
122 using namespace unity::scopes;
123@@ -103,6 +104,10 @@
124 CategorisedResult res(cat);
125 res.set_uri("uri");
126 res.set_title(query().query_string());
127+ int64_t v = 1;
128+ res["int64value"] = Variant(v);
129+ int64_t v2 = INT64_MAX;
130+ res["int64value2"] = Variant(v2);
131 if (valid())
132 {
133 reply->push(res);
134
135=== modified file 'test/gtest/scopes/ResultCache/ResultCache_test.cpp'
136--- test/gtest/scopes/ResultCache/ResultCache_test.cpp 2015-02-17 08:31:43 +0000
137+++ test/gtest/scopes/ResultCache/ResultCache_test.cpp 2015-07-14 09:21:51 +0000
138@@ -24,6 +24,7 @@
139
140 #include <boost/filesystem.hpp>
141 #include <gtest/gtest.h>
142+#include <cstdint>
143
144 #include "CacheScope.h"
145
146@@ -181,6 +182,8 @@
147
148 auto r = receiver->result();
149 EXPECT_EQ(r->title(), "");
150+ EXPECT_EQ(r->value("int64value").get_int64_t(), 1);
151+ EXPECT_EQ(r->value("int64value2").get_int64_t(), INT64_MAX);
152 auto d = receiver->dept();
153 EXPECT_EQ(d->id(), "");
154 auto sd = *d->subdepartments().begin();
155@@ -223,6 +226,8 @@
156
157 auto r = receiver->result();
158 EXPECT_EQ(r->title(), "");
159+ EXPECT_EQ(r->value("int64value").get_int(), 1);
160+ EXPECT_EQ(r->value("int64value2").get_int64_t(), INT64_MAX);
161 auto d = receiver->dept();
162 EXPECT_EQ(d->id(), "");
163 auto sd = *d->subdepartments().begin();
164@@ -276,6 +281,7 @@
165
166 auto r = receiver->result();
167 EXPECT_EQ(r->title(), "");
168+ EXPECT_EQ(r->value("int64value").get_int64_t(), 1);
169 auto d = receiver->dept();
170 EXPECT_EQ(d->id(), "");
171 auto sd = *d->subdepartments().begin();
172
173=== modified file 'test/gtest/scopes/internal/CMakeLists.txt'
174--- test/gtest/scopes/internal/CMakeLists.txt 2015-07-08 14:11:14 +0000
175+++ test/gtest/scopes/internal/CMakeLists.txt 2015-07-14 09:21:51 +0000
176@@ -16,7 +16,7 @@
177 add_subdirectory(ScopeConfig)
178 add_subdirectory(ScopeLoader)
179 add_subdirectory(ScopeMetadataImpl)
180-#add_subdirectory(SettingsDB)
181+add_subdirectory(SettingsDB)
182 add_subdirectory(smartscopes)
183 add_subdirectory(ThreadPool)
184 add_subdirectory(ThreadSafeQueue)
185
186=== modified file 'test/gtest/scopes/internal/SettingsDB/SettingsDB_test.cpp'
187--- test/gtest/scopes/internal/SettingsDB/SettingsDB_test.cpp 2015-06-30 09:24:10 +0000
188+++ test/gtest/scopes/internal/SettingsDB/SettingsDB_test.cpp 2015-07-14 09:21:51 +0000
189@@ -36,6 +36,11 @@
190
191 void write_db(const string& src)
192 {
193+ // make sure the next write doesn't happen too fast or otherwise modification time of settings db
194+ // will be the same and change will not be detected by SettingsDB. Note, this is not an issue with
195+ // real use cases when settings are modified by the UI (and SettingsDB uses nanosecond-based mtime).
196+ std::this_thread::sleep_for(std::chrono::milliseconds(1100));
197+
198 int fd = ::open(db_name.c_str(), O_WRONLY|O_CREAT, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP);
199 if (fd == -1)
200 {
201@@ -70,48 +75,6 @@
202 }
203 ::close(fd2);
204 ::close(fd);
205-
206- // make sure the next write doesn't happen too fast or otherwise modification time of settings db
207- // will be the same and change will not be detected by SettingsDB. Note, this is not an issue with
208- // real use cases when settings are modified by the UI (and SettingsDB uses nanosecond-based mtime).
209- std::this_thread::sleep_for(std::chrono::seconds(1));
210-}
211-
212-#define TRY_EXPECT_EQ(expected, actual) \
213-{ \
214- auto now = chrono::system_clock::now(); \
215- auto later = now + chrono::seconds(10); \
216- while (now < later) \
217- { \
218- if ((expected) == (actual)) \
219- { \
220- break; \
221- } \
222- this_thread::sleep_for(chrono::milliseconds(10)); \
223- now = chrono::system_clock::now(); \
224- } \
225- EXPECT_EQ((expected), (actual)); \
226-}
227-
228-#define TRY_EXPECT_TRUE(expr) \
229-{ \
230- auto now = chrono::system_clock::now(); \
231- auto later = now + chrono::seconds(10); \
232- while (now < later) \
233- { \
234- if (expr) \
235- { \
236- break; \
237- } \
238- this_thread::sleep_for(chrono::milliseconds(10)); \
239- now = chrono::system_clock::now(); \
240- } \
241- EXPECT_TRUE(expr); \
242-}
243-
244-#define TRY_EXPECT_FALSE(expr) \
245-{ \
246- TRY_EXPECT_TRUE(!(expr)); \
247 }
248
249 BOOST_LOG_INLINE_GLOBAL_LOGGER_DEFAULT(test_logger, boost::log::sources::severity_channel_logger_mt<>)
250@@ -154,7 +117,7 @@
251 // Change the location.
252 write_db("db_location.ini");
253 EXPECT_EQ(4, db->settings().size());
254- TRY_EXPECT_EQ("New York", db->settings()["locationSetting"].get_string());
255+ EXPECT_EQ("New York", db->settings()["locationSetting"].get_string());
256 EXPECT_EQ(1, db->settings()["unitTempSetting"].get_int());
257 EXPECT_EQ(23, db->settings()["ageSetting"].get_double());
258 EXPECT_TRUE(db->settings()["enabledSetting"].get_bool());
259@@ -163,7 +126,7 @@
260 write_db("db_loctemp.ini");
261 EXPECT_EQ(4, db->settings().size());
262 EXPECT_EQ("New York", db->settings()["locationSetting"].get_string());
263- TRY_EXPECT_EQ(0, db->settings()["unitTempSetting"].get_int());
264+ EXPECT_EQ(0, db->settings()["unitTempSetting"].get_int());
265 EXPECT_EQ(23, db->settings()["ageSetting"].get_double());
266 EXPECT_TRUE(db->settings()["enabledSetting"].get_bool());
267
268@@ -172,7 +135,7 @@
269 EXPECT_EQ(4, db->settings().size());
270 EXPECT_EQ("New York", db->settings()["locationSetting"].get_string());
271 EXPECT_EQ(0, db->settings()["unitTempSetting"].get_int());
272- TRY_EXPECT_EQ(42.0, db->settings()["ageSetting"].get_double());
273+ EXPECT_EQ(42.0, db->settings()["ageSetting"].get_double());
274 EXPECT_TRUE(db->settings()["enabledSetting"].get_bool());
275
276 // Call settings again. This causes state_changed_ in the implementation
277@@ -190,7 +153,7 @@
278 EXPECT_EQ("New York", db->settings()["locationSetting"].get_string());
279 EXPECT_EQ(0, db->settings()["unitTempSetting"].get_int());
280 EXPECT_EQ(42, db->settings()["ageSetting"].get_double());
281- TRY_EXPECT_FALSE(db->settings()["enabledSetting"].get_bool());
282+ EXPECT_FALSE(db->settings()["enabledSetting"].get_bool());
283 }
284
285 {
286@@ -210,7 +173,7 @@
287
288 // Check that they are correct
289 EXPECT_EQ(4, db->settings().size());
290- TRY_EXPECT_EQ("New York", db->settings()["locationSetting"].get_string());
291+ EXPECT_EQ("New York", db->settings()["locationSetting"].get_string());
292 EXPECT_EQ(0, db->settings()["unitTempSetting"].get_int());
293 EXPECT_EQ(23, db->settings()["ageSetting"].get_double());
294 EXPECT_TRUE(db->settings()["enabledSetting"].get_bool());
295@@ -228,7 +191,7 @@
296
297 // Check that they are correct.
298 EXPECT_EQ(4, db->settings().size());
299- TRY_EXPECT_EQ("New York", db->settings()["locationSetting"].get_string());
300+ EXPECT_EQ("New York", db->settings()["locationSetting"].get_string());
301 EXPECT_EQ(0, db->settings()["unitTempSetting"].get_int());
302 EXPECT_EQ(23, db->settings()["ageSetting"].get_double());
303 EXPECT_TRUE(db->settings()["enabledSetting"].get_bool());
304@@ -238,7 +201,7 @@
305
306 // Check that nothing has changed.
307 EXPECT_EQ(4, db->settings().size());
308- TRY_EXPECT_EQ("Paris", db->settings()["locationSetting"].get_string());
309+ EXPECT_EQ("Paris", db->settings()["locationSetting"].get_string());
310 EXPECT_EQ(0, db->settings()["unitTempSetting"].get_int());
311 EXPECT_EQ(23, db->settings()["ageSetting"].get_double());
312 EXPECT_TRUE(db->settings()["enabledSetting"].get_bool());
313@@ -260,7 +223,7 @@
314 // Change the location.
315 write_db("db_chinese_location.ini");
316 EXPECT_EQ(1, db->settings().size());
317- TRY_EXPECT_EQ("丈", db->settings()["locationSetting"].get_string());
318+ EXPECT_EQ("丈", db->settings()["locationSetting"].get_string());
319 }
320 }
321
322@@ -284,7 +247,7 @@
323
324 // Settings should be updated.
325 EXPECT_EQ(4, db->settings().size());
326- TRY_EXPECT_EQ("New York", db->settings()["locationSetting"].get_string());
327+ EXPECT_EQ("New York", db->settings()["locationSetting"].get_string());
328 EXPECT_EQ(0, db->settings()["unitTempSetting"].get_int());
329 EXPECT_EQ(42, db->settings()["ageSetting"].get_double());
330 EXPECT_FALSE(db->settings()["enabledSetting"].get_bool());
331@@ -294,7 +257,7 @@
332
333 // Default values should come back.
334 EXPECT_EQ(4, db->settings().size());
335- TRY_EXPECT_EQ("London", db->settings()["locationSetting"].get_string());
336+ EXPECT_EQ("London", db->settings()["locationSetting"].get_string());
337 EXPECT_EQ(1, db->settings()["unitTempSetting"].get_int());
338 EXPECT_EQ(23, db->settings()["ageSetting"].get_double());
339 EXPECT_TRUE(db->settings()["enabledSetting"].get_bool());
340@@ -330,7 +293,7 @@
341 // Change the location.
342 write_db("db_location_json.ini");
343 EXPECT_EQ(1, db->settings().size());
344- TRY_EXPECT_EQ("New York", db->settings()["location"].get_string());
345+ EXPECT_EQ("New York", db->settings()["location"].get_string());
346 }
347 }
348
349@@ -399,14 +362,16 @@
350 auto db = SettingsDB::create_from_ini_file(db_name, schema, test_logger::get());
351
352 EXPECT_EQ(4, db->settings().size());
353- TRY_EXPECT_EQ("Munich", db->settings()["locationSetting"].get_string());
354+ EXPECT_EQ("Munich", db->settings()["locationSetting"].get_string());
355+
356+
357+ std::this_thread::sleep_for(std::chrono::milliseconds(1100));
358
359 // Clobber the DB.
360 if (system((string("cat >") + db_name + " <<EOF\nx\nEOF" + db_name).c_str()) < 0)
361 {
362 FAIL();
363 }
364- usleep(100000);
365
366 // Call settings(), which will fail.
367 db->settings();

Subscribers

People subscribed via source and target branches

to all changes: