Merge lp:~day-scope-team/day-scope/fixes-1500486-cache-sun-info into lp:day-scope

Proposed by Kyle Nitzsche
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
Reviewer Review Type Date Requested Status
Jin (community) Approve
Review via email: mp+285126@code.launchpad.net

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.

To post a comment you must log in.
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.

Revision history for this message
Jin (jindallo) :
Revision history for this message
Jin (jindallo) :
Revision history for this message
Jin (jindallo) :
Revision history for this message
Jin (jindallo) wrote :

Generally, the code looks good to me,
but some suggestions as below,
please take a look there,
thank you.

Revision history for this message
Jin (jindallo) :
review: Approve
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 getLunarFromNetwork() and getSunsFromNetwork() 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

Revision history for this message
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 getLunarFromNetwork() and getSunsFromNetwork() into single method:
  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

Revision history for this message
Jin (jindallo) wrote :

Hello Kyle,

Oh super!

The code looks great to me,
I can't wait for this landing!

Many thanks.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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];

Subscribers

People subscribed via source and target branches

to all changes: