From 31588fa21bc3b91209bb4122807e9e0a9ba5fcbf Mon Sep 17 00:00:00 2001 From: Tobias Preuss Date: Thu, 15 Feb 2024 21:56:33 +0100 Subject: [PATCH 1/3] Improve name of boolean variable. --- .../fahrplan/congress/repositories/AppRepository.kt | 6 +++--- .../AppRepositoryLoadAndParseScheduleTest.kt | 2 +- .../repositories/RealScheduleNetworkRepository.kt | 4 ++-- .../repositories/ScheduleNetworkRepository.kt | 2 +- .../network/serialization/FahrplanParser.java | 12 ++++++------ 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepository.kt b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepository.kt index 388f0fcfeb..80fdb00e93 100644 --- a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepository.kt +++ b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepository.kt @@ -391,9 +391,9 @@ object AppRepository { val validMeta = meta.validate() updateMeta(validMeta) }, - onParsingDone = { result: Boolean, version: String -> - val parseResult = ParseScheduleResult(result, version) - val parseScheduleStatus = if (result) ParseSuccess else ParseFailure(parseResult) + onParsingDone = { isSuccess: Boolean, version: String -> + val parseResult = ParseScheduleResult(isSuccess, version) + val parseScheduleStatus = if (isSuccess) ParseSuccess else ParseFailure(parseResult) mutableLoadScheduleState.tryEmit(parseScheduleStatus) onParsingDone(parseResult) loadShifts(onLoadingShiftsDone) diff --git a/app/src/test/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepositoryLoadAndParseScheduleTest.kt b/app/src/test/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepositoryLoadAndParseScheduleTest.kt index 84d82eb2f9..5cc8945f1b 100644 --- a/app/src/test/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepositoryLoadAndParseScheduleTest.kt +++ b/app/src/test/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepositoryLoadAndParseScheduleTest.kt @@ -351,7 +351,7 @@ class AppRepositoryLoadAndParseScheduleTest { httpHeader: HttpHeader, onUpdateSessions: (sessions: List) -> Unit, onUpdateMeta: (meta: NetworkMeta) -> Unit, - onParsingDone: (result: Boolean, version: String) -> Unit + onParsingDone: (isSuccess: Boolean, version: String) -> Unit ) { this.onUpdateSessions = onUpdateSessions this.onUpdateMeta = onUpdateMeta diff --git a/network/src/main/java/info/metadude/android/eventfahrplan/network/repositories/RealScheduleNetworkRepository.kt b/network/src/main/java/info/metadude/android/eventfahrplan/network/repositories/RealScheduleNetworkRepository.kt index 4a8b774646..077dd6c8ed 100644 --- a/network/src/main/java/info/metadude/android/eventfahrplan/network/repositories/RealScheduleNetworkRepository.kt +++ b/network/src/main/java/info/metadude/android/eventfahrplan/network/repositories/RealScheduleNetworkRepository.kt @@ -30,11 +30,11 @@ class RealScheduleNetworkRepository( httpHeader: HttpHeader, onUpdateSessions: (sessions: List) -> Unit, onUpdateMeta: (meta: Meta) -> Unit, - onParsingDone: (result: Boolean, version: String) -> Unit) { + onParsingDone: (isSuccess: Boolean, version: String) -> Unit) { parser.setListener(object : FahrplanParser.OnParseCompleteListener { override fun onUpdateSessions(sessions: List) = onUpdateSessions.invoke(sessions) override fun onUpdateMeta(meta: Meta) = onUpdateMeta.invoke(meta) - override fun onParseDone(result: Boolean, version: String) = onParsingDone.invoke(result, version) + override fun onParseDone(isSuccess: Boolean, version: String) = onParsingDone.invoke(isSuccess, version) }) parser.parse(scheduleXml, httpHeader) } diff --git a/network/src/main/java/info/metadude/android/eventfahrplan/network/repositories/ScheduleNetworkRepository.kt b/network/src/main/java/info/metadude/android/eventfahrplan/network/repositories/ScheduleNetworkRepository.kt index b95b0de761..5b6fc278da 100644 --- a/network/src/main/java/info/metadude/android/eventfahrplan/network/repositories/ScheduleNetworkRepository.kt +++ b/network/src/main/java/info/metadude/android/eventfahrplan/network/repositories/ScheduleNetworkRepository.kt @@ -17,6 +17,6 @@ interface ScheduleNetworkRepository { httpHeader: HttpHeader, onUpdateSessions: (sessions: List) -> Unit, onUpdateMeta: (meta: Meta) -> Unit, - onParsingDone: (result: Boolean, version: String) -> Unit) + onParsingDone: (isSuccess: Boolean, version: String) -> Unit) } diff --git a/network/src/main/java/info/metadude/android/eventfahrplan/network/serialization/FahrplanParser.java b/network/src/main/java/info/metadude/android/eventfahrplan/network/serialization/FahrplanParser.java index 79b4293ded..2fa231bcc2 100644 --- a/network/src/main/java/info/metadude/android/eventfahrplan/network/serialization/FahrplanParser.java +++ b/network/src/main/java/info/metadude/android/eventfahrplan/network/serialization/FahrplanParser.java @@ -33,7 +33,7 @@ public interface OnParseCompleteListener { void onUpdateMeta(@NonNull Meta meta); - void onParseDone(Boolean result, String version); + void onParseDone(Boolean isSuccess, String version); } @NonNull @@ -80,7 +80,7 @@ class ParserTask extends AsyncTask { private boolean completed; - private boolean result; + private boolean isSuccess; ParserTask(@NonNull Logging logging, FahrplanParser.OnParseCompleteListener listener) { this.logging = logging; @@ -109,17 +109,17 @@ protected Boolean doInBackground(String... args) { } private void notifyActivity() { - if (result) { + if (isSuccess) { listener.onUpdateMeta(meta); listener.onUpdateSessions(sessions); } - listener.onParseDone(result, meta.getVersion()); + listener.onParseDone(isSuccess, meta.getVersion()); completed = false; } - protected void onPostExecute(Boolean result) { + protected void onPostExecute(Boolean isSuccess) { completed = true; - this.result = result; + this.isSuccess = isSuccess; if (listener != null) { notifyActivity(); From c15ed97cf292f16d7a5c98310bb8121572986e6e Mon Sep 17 00:00:00 2001 From: Tobias Preuss Date: Thu, 15 Feb 2024 22:53:27 +0100 Subject: [PATCH 2/3] Clean up code. --- .../fahrplan/congress/repositories/AppRepository.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepository.kt b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepository.kt index 80fdb00e93..06aed3de2b 100644 --- a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepository.kt +++ b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepository.kt @@ -362,10 +362,10 @@ object AppRepository { val parsingStatus = if (meta.numDays == 0) InitialParsing else Parsing mutableLoadScheduleState.tryEmit(parsingStatus) parseSchedule( - fetchScheduleResult.scheduleXml, - fetchScheduleResult.httpHeader, - onParsingDone, - onLoadingShiftsDone + scheduleXml = fetchScheduleResult.scheduleXml, + httpHeader = fetchScheduleResult.httpHeader, + onParsingDone = onParsingDone, + onLoadingShiftsDone = onLoadingShiftsDone ) } else if (fetchResult.isNotModified) { loadShifts(onLoadingShiftsDone) From 0a5eaa51b82b11e8d520c80d6a8e36ba73ea6702 Mon Sep 17 00:00:00 2001 From: Tobias Preuss Date: Thu, 15 Feb 2024 22:53:39 +0100 Subject: [PATCH 3/3] Continue to display parsing error when schedule data cannot be parsed. + Previously, loading malformed schedule data would result in an error message shown once. Each subsequent failed load would not be shown as an error message, but as "Schedule is up to date". This happened because the ETag or Last-Modified header from the HTTP response remained in the database. + Now the ETag and Last-Modified headers are cleared when the parsing fails. + Related: https://github.com/pretalx/pretalx/issues/1685 --- .../congress/repositories/AppRepository.kt | 5 ++++ .../AppRepositoryLoadAndParseScheduleTest.kt | 23 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepository.kt b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepository.kt index 06aed3de2b..b63d15119e 100644 --- a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepository.kt +++ b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepository.kt @@ -364,6 +364,7 @@ object AppRepository { parseSchedule( scheduleXml = fetchScheduleResult.scheduleXml, httpHeader = fetchScheduleResult.httpHeader, + oldMeta = meta, onParsingDone = onParsingDone, onLoadingShiftsDone = onLoadingShiftsDone ) @@ -375,6 +376,7 @@ object AppRepository { private fun parseSchedule(scheduleXml: String, httpHeader: HttpHeader, + oldMeta: Meta, onParsingDone: (parseScheduleResult: ParseResult) -> Unit, onLoadingShiftsDone: (loadShiftsResult: LoadShiftsResult) -> Unit) { scheduleNetworkRepository.parseSchedule(scheduleXml, httpHeader, @@ -392,6 +394,9 @@ object AppRepository { updateMeta(validMeta) }, onParsingDone = { isSuccess: Boolean, version: String -> + if (!isSuccess) { + updateMeta(oldMeta.copy(httpHeader = HttpHeader(eTag = "", lastModified = ""))) + } val parseResult = ParseScheduleResult(isSuccess, version) val parseScheduleStatus = if (isSuccess) ParseSuccess else ParseFailure(parseResult) mutableLoadScheduleState.tryEmit(parseScheduleStatus) diff --git a/app/src/test/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepositoryLoadAndParseScheduleTest.kt b/app/src/test/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepositoryLoadAndParseScheduleTest.kt index 5cc8945f1b..d174741f7c 100644 --- a/app/src/test/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepositoryLoadAndParseScheduleTest.kt +++ b/app/src/test/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepositoryLoadAndParseScheduleTest.kt @@ -298,6 +298,29 @@ class AppRepositoryLoadAndParseScheduleTest { testableAppRepository.loadScheduleState.test { assertThat(awaitItem()).isEqualTo(ParseFailure(ParseScheduleResult(false, "1.0.0"))) } + // Reset ETag &b Last-Modified if parsing failed + verify(metaDatabaseRepository, times(2)).insert(any()) + } + + @Test + fun `loadScheduleState emits ParseFailure when initial parsing finished with an error`() = + runTest { + whenever(metaDatabaseRepository.query()) doReturn DatabaseMeta(numDays = 0) + val success = createFetchScheduleResult(NetworkHttpStatus.HTTP_OK) + val onParsingDone: OnParsingDone = { result -> + assertThat(result).isEqualTo(ParseScheduleResult(isSuccess = false, "1.0.0")) + } + testableAppRepository.loadSchedule(isUserRequest = false, onParsingDone = onParsingDone) + scheduleNetworkRepository.onFetchScheduleFinished(success) + + // onParsingDone + whenever(sharedPreferencesRepository.getEngelsystemShiftsUrl()) doReturn EMPTY_ENGELSYSTEM_URL // early exit to bypass here + scheduleNetworkRepository.onParsingDone(false, "1.0.0") + testableAppRepository.loadScheduleState.test { + assertThat(awaitItem()).isEqualTo(ParseFailure(ParseScheduleResult(false, "1.0.0"))) + } + // Reset ETag &b Last-Modified if parsing failed + verify(metaDatabaseRepository, times(2)).insert(any()) } private fun createFetchScheduleResult(httpStatus: NetworkHttpStatus) =