Merge lp:~day-scope-team/day-scope/fixes-1500486-cache-sun-info into lp:day-scope
- fixes-1500486-cache-sun-info
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 41 |
Proposed branch: | lp:~day-scope-team/day-scope/fixes-1500486-cache-sun-info |
Merge into: | lp:day-scope |
Diff against target: |
433 lines (+186/-70) 4 files modified
CMakeLists.txt (+2/-2) include/query.h (+10/-4) src/CMakeLists.txt (+6/-6) src/query.cpp (+168/-58) |
To merge this branch: | bzr merge lp:~day-scope-team/day-scope/fixes-1500486-cache-sun-info |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jin (community) | Approve | ||
Review via email: mp+285126@code.launchpad.net |
Commit message
Description of the change
cache sunrise/sunset info and use it:
* if it was retrieved today
* and if we have network and location. If we don't have those, show "No Information" (localized).
This should speed up Day scope and therefore speed up Today scope.
- 42. By Kyle Nitzsche
-
when the lunar phase is not translated, it needs a space inserted so, for
example, "fullmoon" becomse "Full moon". This commit does this for the
eight phases returned by the web api - 43. By Kyle Nitzsche
-
modified fix to change the moon phase names AFTER its translation has been
retrieved, thus it does not break translations.
Jin (jindallo) : | # |
Jin (jindallo) : | # |
Jin (jindallo) : | # |
Jin (jindallo) wrote : | # |
Jin (jindallo) : | # |
- 44. By Kyle Nitzsche
-
combine useNetworkSuns() and useNetworkLunar() into single method:
useNetwork() and pass an enum to control which code runs inside - 45. By Kyle Nitzsche
-
combine getLunarFromNet
work() and getSunsFromNetw ork() into single method:
getFromNetwork and use passed enum to control code path - 46. By Kyle Nitzsche
-
combine getLunarFromLocal() and getSunsFromLocal() into:
getFromLocal() and use passed data_t to specify code path
Kyle Nitzsche (knitzsche) wrote : | # |
Hi Jin,
I've combined three pairs of lunar/sun methods into single methods that now handle both cases controlled by the passed enum arg as you requested:
combine getLunarFromLocal() and getSunsFromLocal() into:
getFromLocal() and use passed data_t to specify code path
combine getLunarFromNet
getFromNetwork and use passed enum to control code path
combine useNetworkSuns() and useNetworkLunar() into single method:
useNetwork() and pass an enum to control which code runs inside
Jin (jindallo) wrote : | # |
Hello Kyle,
Oh super!
The code looks great to me,
I can't wait for this landing!
Many thanks.
Preview Diff
1 | === modified file 'CMakeLists.txt' |
2 | --- CMakeLists.txt 2016-01-26 07:06:29 +0000 |
3 | +++ CMakeLists.txt 2016-02-16 20:55:07 +0000 |
4 | @@ -1,4 +1,4 @@ |
5 | -set(VERSION "1.2.2") |
6 | +set(VERSION "1.3") |
7 | |
8 | # Supress qDebug() output |
9 | ADD_DEFINITIONS( -DQT_NO_DEBUG_OUTPUT ) |
10 | @@ -66,7 +66,7 @@ |
11 | include(GNUInstallDirs) |
12 | |
13 | include(FindPkgConfig) |
14 | -pkg_check_modules(UNITY_SCOPES libunity-scopes>=0.4.0 REQUIRED) |
15 | +pkg_check_modules(UNITY_SCOPES libunity-scopes>=0.6.0 REQUIRED) |
16 | |
17 | include_directories( |
18 | "${CMAKE_SOURCE_DIR}/include" |
19 | |
20 | === modified file 'include/query.h' |
21 | --- include/query.h 2015-08-06 06:01:34 +0000 |
22 | +++ include/query.h 2016-02-16 20:55:07 +0000 |
23 | @@ -49,7 +49,7 @@ |
24 | QCoreApplication *app; |
25 | QNetworkAccessManager manager; |
26 | QEventLoop loop; |
27 | - QMap<QString,QString> getSuns(); |
28 | + QMap<QString,QString> getSuns(QFile &, QFile &); |
29 | QString cacheDir_; |
30 | QString mUserAgent; |
31 | QString getLunarPhase(QFile &, QFile &); |
32 | @@ -72,14 +72,20 @@ |
33 | |
34 | QFile lunarData_f; |
35 | QString lunarData_filename; |
36 | + QFile sunData_f; |
37 | + QString sunData_filename; |
38 | QFile lunarLastRefresh_f; |
39 | QString lunarLastRefresh_filename; |
40 | + QFile sunsLastRefresh_f; |
41 | + QString sunsLastRefresh_filename; |
42 | QString phase; |
43 | - bool useNetwork(QFile &, QFile &); |
44 | + QMap<QString,QString> suns; |
45 | + enum class data_t {suns, lunar}; |
46 | + bool useNetwork(QFile &, QFile &, data_t); |
47 | QString formatToday(); |
48 | QString parseLunarPhase(QJsonDocument &); |
49 | - void getFromNetwork(QFile &, QFile &); |
50 | - void getFromLocal(QFile &, QFile &); |
51 | + void getFromNetwork(QFile &, QFile &, data_t); |
52 | + void getFromLocal(QFile &, QFile &, data_t); |
53 | }; |
54 | |
55 | #endif |
56 | |
57 | === modified file 'src/CMakeLists.txt' |
58 | --- src/CMakeLists.txt 2015-03-30 16:16:43 +0000 |
59 | +++ src/CMakeLists.txt 2016-02-16 20:55:07 +0000 |
60 | @@ -1,3 +1,9 @@ |
61 | +add_library(${SO_NAME} SHARED |
62 | + preview.cpp |
63 | + query.cpp |
64 | + scope.cpp |
65 | +) |
66 | + |
67 | add_definitions( |
68 | -DGETTEXT_PACKAGE=\"${PROJECT_NAME}\" |
69 | -DGETTEXT_LOCALEDIR=\"${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LOCALEDIR}\" |
70 | @@ -5,12 +11,6 @@ |
71 | |
72 | include_directories(${UNITY_SCOPES_INCLUDE_DIRS}) |
73 | |
74 | -add_library(${SO_NAME} SHARED |
75 | - preview.cpp |
76 | - query.cpp |
77 | - scope.cpp |
78 | -) |
79 | - |
80 | find_package(Qt5Core REQUIRED) |
81 | find_package(Qt5Network REQUIRED) |
82 | qt5_use_modules(${SO_NAME} Core Network) |
83 | |
84 | === modified file 'src/query.cpp' |
85 | --- src/query.cpp 2015-09-11 06:24:31 +0000 |
86 | +++ src/query.cpp 2016-02-16 20:55:07 +0000 |
87 | @@ -84,11 +84,14 @@ |
88 | scope_(scope), |
89 | cacheDir_(cacheDir_) |
90 | { |
91 | - qDebug() << "==== cacheDir_ = " << cacheDir_; |
92 | lunarData_filename = QString("%1/LunarData.json").arg(cacheDir_); |
93 | lunarData_f.setFileName(lunarData_filename); |
94 | + sunData_filename = QString("%1/SunData.json").arg(cacheDir_); |
95 | + sunData_f.setFileName(sunData_filename); |
96 | lunarLastRefresh_filename = QString("%1/LastRefresh.txt").arg(cacheDir_); |
97 | lunarLastRefresh_f.setFileName(lunarLastRefresh_filename); |
98 | + sunsLastRefresh_filename = QString("%1/SunLastRefresh.txt").arg(cacheDir_); |
99 | + sunsLastRefresh_f.setFileName(sunsLastRefresh_filename); |
100 | } |
101 | |
102 | Query::~Query() |
103 | @@ -103,66 +106,107 @@ |
104 | return QString("<strong>%1</strong>").arg(str); |
105 | } |
106 | |
107 | -bool Query::useNetwork(QFile &data, QFile &lastRefresh) |
108 | +bool Query::useNetwork(QFile &data, QFile &lastRefresh, data_t t) |
109 | { |
110 | - qDebug() << "==== useNetwork()"; |
111 | + |
112 | + if (!search_metadata().has_location() || !search_metadata().internet_connectivity() == us::QueryMetadata::Connected) |
113 | + return false; |
114 | + |
115 | + if (t == data_t::suns) |
116 | + qDebug() << "==== useNetwork() for suns"; |
117 | + else |
118 | + qDebug() << "==== useNetwork() for lunar"; |
119 | + |
120 | data.close(); |
121 | if (!data.open(QIODevice::ReadOnly)) |
122 | { |
123 | - qWarning() << "===== The JSON file missing!"; |
124 | + qWarning() << "===== The JSON file missing: " << data.fileName(); |
125 | data.close(); |
126 | data.remove(); |
127 | return true; |
128 | } |
129 | - |
130 | lastRefresh.close(); |
131 | if (!lastRefresh.open(QIODevice::ReadOnly)) |
132 | { |
133 | - qWarning() << "===== The TXT file missing!"; |
134 | + qWarning() << "===== The refresh cache file missing: " << lastRefresh.fileName(); |
135 | lastRefresh.remove(); |
136 | return true; |
137 | } |
138 | else |
139 | { |
140 | - // Configure the API request period as 31 days |
141 | QDate today = QDate::currentDate(); |
142 | - QDate oneMonthAgo = today.addDays(-31); |
143 | - QByteArray rDate_ba = lastRefresh.readAll(); |
144 | + QByteArray cachedDate_ba = lastRefresh.readAll(); |
145 | lastRefresh.close(); |
146 | - QString rDate_qstr = QString(rDate_ba); |
147 | - QDate rDate_da = QDate::fromString(rDate_qstr.trimmed(), "yyyy-MM-dd"); |
148 | - if (!rDate_da.isValid()) |
149 | - { |
150 | - qWarning() << "===== Time stamp is invalid!"; |
151 | - lastRefresh.remove(); |
152 | - return true; |
153 | - } |
154 | - if ((rDate_da == oneMonthAgo) || (rDate_da < oneMonthAgo)) |
155 | - { |
156 | - qDebug() << "==== An overdue using of caching result."; |
157 | - lastRefresh.remove(); |
158 | - data.remove(); |
159 | - return true; |
160 | - } |
161 | - |
162 | + QString cachedDate_qstr = QString(cachedDate_ba); |
163 | + QDate cachedDate = QDate::fromString(cachedDate_qstr.trimmed(), "yyyy-MM-dd"); |
164 | + if (!cachedDate.isValid()) |
165 | + { |
166 | + qWarning() << "===== Timestamp in refresh file is invalid: " << lastRefresh.fileName(); |
167 | + lastRefresh.remove(); |
168 | + return true; |
169 | + } |
170 | + if (t == data_t::suns) |
171 | + { |
172 | + if (cachedDate != today) |
173 | + { |
174 | + qDebug() << "==== Suns cache date not today, so use network."; |
175 | + lastRefresh.remove(); |
176 | + data.remove(); |
177 | + return true; |
178 | + } |
179 | + } |
180 | + else if (t == data_t::lunar ) |
181 | + { |
182 | + QDate oneMonthAgo = today.addDays(-31); |
183 | + if ((cachedDate == oneMonthAgo) || (cachedDate < oneMonthAgo)) |
184 | + { |
185 | + qDebug() << "==== Lunar timestamp indicates we need to refresh cache from web."; |
186 | + lastRefresh.remove(); |
187 | + data.remove(); |
188 | + return true; |
189 | + } |
190 | + } |
191 | } |
192 | - |
193 | return false; |
194 | } |
195 | |
196 | -void Query::getFromNetwork(QFile &data, QFile &lastRefresh) |
197 | +void Query::getFromNetwork(QFile &data, QFile &lastRefresh, data_t t) |
198 | { |
199 | - qDebug() << "==== getFromNetwork()"; |
200 | - if (!haveSignature) |
201 | - { |
202 | - makeSignature(); |
203 | - } |
204 | - phase = getLunarPhase(data, lastRefresh); |
205 | + if (t == data_t::suns) |
206 | + qDebug() << "==== useNetwork() for suns"; |
207 | + else |
208 | + qDebug() << "==== useNetwork() for lunar"; |
209 | + |
210 | + if (search_metadata().has_location() && search_metadata().internet_connectivity() == us::QueryMetadata::Connected) |
211 | + { |
212 | + if (!haveSignature) |
213 | + { |
214 | + makeSignature(); |
215 | + } |
216 | + if (t == data_t::suns) |
217 | + { |
218 | + suns = getSuns(data, lastRefresh); |
219 | + } |
220 | + else if (t == data_t::lunar) |
221 | + { |
222 | + phase = getLunarPhase(data, lastRefresh); |
223 | + } |
224 | + } |
225 | + else |
226 | + { |
227 | + qDebug() << "==== DAY. no location"; |
228 | + suns["sunrise"] = QString::fromStdString(_("No Information")); |
229 | + suns["sunset"] = QString::fromStdString(_("No Information")); |
230 | + } |
231 | } |
232 | |
233 | -void Query::getFromLocal(QFile &data, QFile &lastRefresh) |
234 | +void Query::getFromLocal(QFile &data, QFile &lastRefresh, data_t t) |
235 | { |
236 | - qDebug() << "==== getFromLocal()"; |
237 | + if (t == data_t::suns) |
238 | + qDebug() << "==== getFromLocal() for suns"; |
239 | + else |
240 | + qDebug() << "==== getFromLocal() for lunar"; |
241 | + |
242 | data.close(); |
243 | if (data.open(QIODevice::ReadOnly)) |
244 | { |
245 | @@ -172,32 +216,60 @@ |
246 | |
247 | if (err.error != QJsonParseError::NoError) |
248 | { |
249 | - qCritical() << "Failed to parse server data: " << err.errorString(); |
250 | + qWarning() << "Cannot json parse from cache file: " << data.fileName(); |
251 | data.remove(); |
252 | } |
253 | else |
254 | { |
255 | - QJsonObject doc_object = doc.object(); |
256 | - if (doc_object.contains("locations")) |
257 | + if (t == data_t::suns) |
258 | { |
259 | - phase = parseLunarPhase(doc); |
260 | - if (phase.isEmpty()) |
261 | - { |
262 | - qWarning() << "!!!!! Weird case, need to check local cache file."; |
263 | - qWarning() << "===== Request moon phase from network."; |
264 | - getFromNetwork(data, lastRefresh); |
265 | + QJsonObject obj = doc.object(); |
266 | + if (obj.contains("sunrise") && obj.contains("sunset")) |
267 | + { |
268 | + QString sunr = obj["sunrise"].toString(); |
269 | + suns["sunrise"] = sunr.split(" ")[1]; |
270 | + QString sunset = obj["sunset"].toString(); |
271 | + suns["sunset"] = sunset.split(" ")[1]; |
272 | + } |
273 | + else |
274 | + { |
275 | + getFromNetwork(data, lastRefresh, data_t::suns); |
276 | + lastRefresh.remove(); |
277 | + return; |
278 | } |
279 | } |
280 | - else |
281 | + else if (t == data_t::lunar) |
282 | { |
283 | - getFromNetwork(data, lastRefresh); |
284 | - return; |
285 | + QJsonObject doc_object = doc.object(); |
286 | + if (doc_object.contains("locations")) |
287 | + { |
288 | + phase = parseLunarPhase(doc); |
289 | + if (phase.isEmpty()) |
290 | + { |
291 | + qWarning() << "===== Lunar cache file invalid. Request moon phase from network."; |
292 | + getFromNetwork(data, lastRefresh, data_t::lunar); |
293 | + } |
294 | + } |
295 | + else |
296 | + { |
297 | + getFromNetwork(data, lastRefresh, data_t::lunar); |
298 | + lastRefresh.remove(); |
299 | + return; |
300 | + } |
301 | + |
302 | } |
303 | } |
304 | } |
305 | else |
306 | { |
307 | - getFromNetwork(data, lastRefresh); |
308 | + if (t == data_t::suns) |
309 | + { |
310 | + getFromNetwork(data, lastRefresh, data_t::lunar); |
311 | + } |
312 | + else if (t == data_t::lunar) |
313 | + { |
314 | + getFromNetwork(data, lastRefresh, data_t::lunar); |
315 | + } |
316 | } |
317 | } |
318 | |
319 | @@ -221,16 +293,23 @@ |
320 | } |
321 | catch (unity::Exception const &e) |
322 | { |
323 | - qWarning() << "Location object not found: " << QString::fromStdString(e.what()); |
324 | + qWarning() << "Location object not found and exception is caught: " << QString::fromStdString(e.what()); |
325 | } |
326 | |
327 | - if (useNetwork(lunarData_f, lunarLastRefresh_f)) |
328 | - { |
329 | - getFromNetwork(lunarData_f, lunarLastRefresh_f); |
330 | + if (useNetwork(sunData_f, sunsLastRefresh_f, data_t::suns)) |
331 | + { |
332 | + getFromNetwork(sunData_f, sunsLastRefresh_f, data_t::suns); |
333 | + } else { |
334 | + getFromLocal(sunData_f, sunsLastRefresh_f, data_t::suns); |
335 | + } |
336 | + |
337 | + if (useNetwork(lunarData_f, lunarLastRefresh_f, data_t::lunar)) |
338 | + { |
339 | + getFromNetwork(lunarData_f, lunarLastRefresh_f, data_t::lunar); |
340 | } |
341 | else |
342 | { |
343 | - getFromLocal(lunarData_f, lunarLastRefresh_f); |
344 | + getFromLocal(lunarData_f, lunarLastRefresh_f, data_t::lunar); |
345 | } |
346 | |
347 | if (QString::fromStdString(phase.toStdString()).isEmpty()) |
348 | @@ -240,6 +319,23 @@ |
349 | else |
350 | { |
351 | phase = _(phase.toStdString().c_str()); |
352 | + //correct english. Note that current translations provide corrections |
353 | + if (phase == "waningcrescent") |
354 | + phase = "Waning crescent"; |
355 | + else if (phase == "thirdquarter") |
356 | + phase = "Third quarter"; |
357 | + else if (phase == "waninggibbous") |
358 | + phase = "Waning gibbous"; |
359 | + else if (phase == "fullmoon" ) |
360 | + phase = "Full moon"; |
361 | + else if (phase == "waxinggibbous") |
362 | + phase = "Waxing gibbous"; |
363 | + else if (phase == "firstquarter") |
364 | + phase = "First quarter"; |
365 | + else if (phase == "waxingcrescent") |
366 | + phase = "Waxing crescent"; |
367 | + else if (phase == "newmoon") |
368 | + phase = "New moon"; |
369 | } |
370 | |
371 | // Initial date info |
372 | @@ -261,7 +357,6 @@ |
373 | string icon_path = |
374 | QString("%1/images/calendar-app_day-%2.png").arg(QString::fromStdString(scope_.scope_directory())).arg(today.toString("dd")).toStdString(); |
375 | date_res.set_art(icon_path); |
376 | - qDebug() << "====DAY: getSuns. ICON PATH:" << QString::fromStdString(icon_path); |
377 | |
378 | us::VariantBuilder builder; |
379 | builder.add_tuple({ |
380 | @@ -278,9 +373,8 @@ |
381 | |
382 | if (search_metadata().has_location() && search_metadata().internet_connectivity() == us::QueryMetadata::Connected) |
383 | { |
384 | - qDebug() << "==== DAY. has location"; |
385 | + qDebug() << "==== we have location && network"; |
386 | us::Location loc = search_metadata().location(); |
387 | - QMap<QString,QString> suns = getSuns(); |
388 | if (suns.size() >= 2) { |
389 | sunrise_time = suns["sunrise"]; |
390 | sunset_time = suns["sunset"]; |
391 | @@ -321,9 +415,13 @@ |
392 | |
393 | } |
394 | |
395 | -QMap<QString,QString> Query::getSuns() { |
396 | +QMap<QString,QString> Query::getSuns(QFile &cache_f, QFile &lastRefresh) { |
397 | qDebug() << "=== DAY getSuns()"; |
398 | qDebug() << "=== DAY lat long before suns: " << lat << " " << lng; |
399 | + |
400 | + cache_f.remove(); |
401 | + lastRefresh.remove(); |
402 | + |
403 | QString lat_ = lat; |
404 | QString lng_ = lng; |
405 | if (lat_.startsWith("-")) |
406 | @@ -348,7 +446,7 @@ |
407 | QEventLoop loop; |
408 | |
409 | QObject::connect(&manager, SIGNAL(finished(QNetworkReply*)), &loop, SLOT(quit())); |
410 | - QObject::connect(&manager, &QNetworkAccessManager::finished,[&suns, this](QNetworkReply *msg) { |
411 | + QObject::connect(&manager, &QNetworkAccessManager::finished,[&lastRefresh, &cache_f, &suns, this](QNetworkReply *msg) { |
412 | |
413 | QByteArray data = msg->readAll(); |
414 | QJsonParseError err; |
415 | @@ -356,6 +454,18 @@ |
416 | if (err.error != QJsonParseError::NoError) |
417 | qCritical() << "Failed to parse server data: " << err.errorString(); |
418 | |
419 | + if (cache_f.open(QIODevice::ReadWrite | QIODevice::Truncate | QIODevice::Text)) |
420 | + { |
421 | + cache_f.write(doc.toJson()); |
422 | + cache_f.close(); |
423 | + QDate today = QDate::currentDate(); |
424 | + QString today_qstr = today.toString("yyyy-MM-dd"); |
425 | + if (lastRefresh.open(QIODevice::ReadWrite | QIODevice::Truncate)) |
426 | + { |
427 | + lastRefresh.write(today_qstr.toUtf8()); |
428 | + lastRefresh.close(); |
429 | + } |
430 | + } |
431 | QJsonObject obj = doc.object(); |
432 | QString sunr = obj["sunrise"].toString(); |
433 | suns["sunrise"] = sunr.split(" ")[1]; |
Generally, the code looks good to me,
but some suggestions as below,
please take a look there,
thank you.