diff --git a/Awful.apk/src/main/java/com/ferg/awfulapp/forums/UpdateTask.kt b/Awful.apk/src/main/java/com/ferg/awfulapp/forums/UpdateTask.kt index 2ca2a35bd..a50717448 100644 --- a/Awful.apk/src/main/java/com/ferg/awfulapp/forums/UpdateTask.kt +++ b/Awful.apk/src/main/java/com/ferg/awfulapp/forums/UpdateTask.kt @@ -235,7 +235,7 @@ internal abstract class UpdateTask(protected val context: Context, private val t } - override fun handleError(error: AwfulError, doc: Document): Boolean { + override fun handleCriticalError(error: AwfulError, doc: Document): Boolean { onRequestFailed(error) finishTask(false) return true diff --git a/Awful.apk/src/main/java/com/ferg/awfulapp/network/CookieController.java b/Awful.apk/src/main/java/com/ferg/awfulapp/network/CookieController.java index eeb584a7f..0fdaf7f0c 100644 --- a/Awful.apk/src/main/java/com/ferg/awfulapp/network/CookieController.java +++ b/Awful.apk/src/main/java/com/ferg/awfulapp/network/CookieController.java @@ -72,12 +72,15 @@ public static synchronized boolean restoreLoginCookies(Context ctx) { long expiry = prefs.getLong(Constants.COOKIE_PREF_EXPIRY_DATE, -1); int cookieVersion = prefs.getInt(Constants.COOKIE_PREF_VERSION, 0); - if (useridCookieValue == null || passwordCookieValue == null || expiry == -1) { + long maxAge = expiry - System.currentTimeMillis(); + boolean cookieExpired = maxAge <= 0; + // verify the cookie is valid - if not, we need to clear the cookie and return a failure + if (useridCookieValue == null || passwordCookieValue == null || cookieExpired) { if (Constants.DEBUG) { Timber.w("Unable to restore cookies! Reasons:\n" + (useridCookieValue == null ? "USER_ID is NULL\n" : "") + (passwordCookieValue == null ? "PASSWORD is NULL\n" : "") + - (expiry == -1 ? "EXPIRY is -1" : "")); + (cookieExpired ? "cookie has expired, max age = " + maxAge : "")); } cookie = ""; @@ -90,8 +93,6 @@ public static synchronized boolean restoreLoginCookies(Context ctx) { Constants.COOKIE_NAME_SESSIONID, sessionidCookieValue, Constants.COOKIE_NAME_SESSIONHASH, sessionhashCookieValue); - Date expiryDate = new Date(expiry); - Date now = new Date(); HttpCookie[] allCookies = { new HttpCookie(Constants.COOKIE_NAME_USERID, useridCookieValue), @@ -100,8 +101,6 @@ public static synchronized boolean restoreLoginCookies(Context ctx) { new HttpCookie(Constants.COOKIE_NAME_SESSIONHASH, sessionhashCookieValue) }; - long maxAge = expiryDate.getTime() - now.getTime(); - Timber.e("now.compareTo(expiryDate):%s", maxAge); for (HttpCookie tempCookie : allCookies) { tempCookie.setVersion(cookieVersion); @@ -113,8 +112,8 @@ public static synchronized boolean restoreLoginCookies(Context ctx) { } if (Constants.DEBUG) { - Timber.w("Cookies restored from prefs"); - Timber.w("Cookie dump: %s", TextUtils.join("\n", cookieManager.getCookieStore().getCookies())); + Timber.i("Cookies restored from prefs"); + Timber.i("Cookie dump: %s", TextUtils.join("\n", cookieManager.getCookieStore().getCookies())); } return true; diff --git a/Awful.apk/src/main/java/com/ferg/awfulapp/task/AwfulRequest.kt b/Awful.apk/src/main/java/com/ferg/awfulapp/task/AwfulRequest.kt index 60bd760f9..564aeeba8 100644 --- a/Awful.apk/src/main/java/com/ferg/awfulapp/task/AwfulRequest.kt +++ b/Awful.apk/src/main/java/com/ferg/awfulapp/task/AwfulRequest.kt @@ -47,9 +47,9 @@ import java.io.IOException * do anything with the response (e.g. a fire-and-forget message to the site) you can just set [T] * to a nullable type (Void? makes most sense if there's no meaningful result) and return null here. * - * [handleError] and [customizeProgressListenerError] both have default implementations, but can be + * [handleCriticalError] and [customizeProgressListenerError] both have default implementations, but can be * overridden for special error handling and creating custom notification messages. You probably - * won't want to change [handleError] in most cases, and if you do add any handler code (e.g. if + * won't want to change [handleCriticalError] in most cases, and if you do add any handler code (e.g. if * a network failure requires the app to update some state) you'll probably just want to call the * super method to get the standard error handling when you're done. */ @@ -159,16 +159,17 @@ abstract class AwfulRequest(protected val context: Context, private val baseU /** - * Handler for [error]s thrown by [AwfulError.checkPageErrors] when parsing a response. + * Handler for critical [error]s thrown by [AwfulError.checkPageErrors] when parsing a response. * - * The main response-handler logic calls this when it encounters an [AwfulError], to check whether - * the request implementation will handle it (and processing can proceed to [handleResponse]). - * Returns true if the error was handled. + * The main response-handler logic calls this when it encounters a critical [AwfulError], + * to check whether the request implementation will handle it (and processing can proceed + * to [handleResponse]). Returns true if the error was handled. * - * By default this swallows non-critical errors, and returns false for everything else. If you need - * different behaviour for some reason, override this! + * This is only called for errors that return true for [AwfulError.isCritical], and returns + * false by default. If you want to handle any of these errors, override this and return true + * for those. */ - protected open fun handleError(error: AwfulError, doc: Document): Boolean = !error.isCritical + protected open fun handleCriticalError(error: AwfulError, doc: Document): Boolean = false /** * Customize the error a request delivers in its [ProgressListener.requestEnded] callback. @@ -247,9 +248,9 @@ abstract class AwfulRequest(protected val context: Context, private val baseU try { val doc = parseAsHtml(response) updateProgress(50) - val error = AwfulError.checkPageErrors(doc, preferences) - if (error != null && handleError(error, doc)) { - throw error + // we only pass critical errors for requests to handle - anything else (i.e. probations) gets swallowed + AwfulError.checkPageErrors(doc, preferences)?.let { error -> + if (error.isCritical && !handleCriticalError(error, doc)) throw error } val result = handleResponseDocument(doc) diff --git a/Awful.apk/src/main/java/com/ferg/awfulapp/task/LepersColonyRequest.kt b/Awful.apk/src/main/java/com/ferg/awfulapp/task/LepersColonyRequest.kt index 482301b17..5d1e88bea 100644 --- a/Awful.apk/src/main/java/com/ferg/awfulapp/task/LepersColonyRequest.kt +++ b/Awful.apk/src/main/java/com/ferg/awfulapp/task/LepersColonyRequest.kt @@ -6,7 +6,6 @@ import com.ferg.awfulapp.constants.Constants.* import com.ferg.awfulapp.task.LepersColonyRequest.LepersColonyPage import com.ferg.awfulapp.users.LepersColonyFragment.Companion.FIRST_PAGE import com.ferg.awfulapp.users.Punishment -import com.ferg.awfulapp.util.AwfulError import org.jsoup.nodes.Document /** @@ -56,7 +55,6 @@ class LepersColonyRequest(context: Context, val page: Int = 1, val userId: Strin return LepersColonyPage(punishments, thisPage, lastPage, userId) } - override fun handleError(error: AwfulError, doc: Document) = false /** * Represents the contents and metadata for a page from the Leper's Colony diff --git a/Awful.apk/src/main/java/com/ferg/awfulapp/task/LoginRequest.kt b/Awful.apk/src/main/java/com/ferg/awfulapp/task/LoginRequest.kt index 8fd432aab..59cabeea1 100644 --- a/Awful.apk/src/main/java/com/ferg/awfulapp/task/LoginRequest.kt +++ b/Awful.apk/src/main/java/com/ferg/awfulapp/task/LoginRequest.kt @@ -26,8 +26,8 @@ class LoginRequest(context: Context, private val username: String, password: Str @Throws(AwfulError::class) override fun handleResponse(doc: Document): Boolean = validateLoginState() - override fun handleError(error: AwfulError, doc: Document): Boolean = - error.networkResponse?.isRedirect == true || !error.isCritical + override fun handleCriticalError(error: AwfulError, doc: Document): Boolean = + error.networkResponse?.isRedirect == true private val NetworkResponse.isRedirect get() = this.statusCode == 302 diff --git a/Awful.apk/src/main/java/com/ferg/awfulapp/util/AwfulError.java b/Awful.apk/src/main/java/com/ferg/awfulapp/util/AwfulError.java index d4e5747c8..44a3e8274 100644 --- a/Awful.apk/src/main/java/com/ferg/awfulapp/util/AwfulError.java +++ b/Awful.apk/src/main/java/com/ferg/awfulapp/util/AwfulError.java @@ -32,7 +32,6 @@ * requests we make can focus on looking for specific problems. */ public class AwfulError extends VolleyError { - private static final String TAG = "AwfulError"; public static final int ERROR_LOGGED_OUT = 0x00000001; public static final int ERROR_FORUM_CLOSED = 0x00000002; @@ -40,6 +39,8 @@ public class AwfulError extends VolleyError { public static final int ERROR_GENERIC_FAILURE = 0x00000008; public static final int ERROR_ACCESS_DENIED = 0x00000010; + private static final Pattern PROBATION_MESSAGE_REGEX = Pattern.compile("(.*)until\\s(([\\s\\w:,])+).\\sYou(.*)"); + private final int errorCode; @Nullable private final String errorMessage; @@ -60,7 +61,6 @@ public AwfulError(@Nullable String message) { public AwfulError(int code, @Nullable String message) { errorCode = code; errorMessage = message; - Timber.e("Error: " + code + " - " + getMessage()); } /** @@ -157,7 +157,7 @@ public static AwfulError checkPageErrors(Document page, AwfulPreferences prefs) } } - // you're probed! + // handle probation status by looking for the probation message (or lack of it) Element probation = page.getElementById("probation_warn"); if (probation == null) { // clear any probation @@ -171,9 +171,8 @@ public static AwfulError checkPageErrors(Document page, AwfulPreferences prefs) } // try to parse the probation date - default to 1 day in case we can't parse it (not too scary) - Long probTimestamp = System.currentTimeMillis() + TimeUnit.DAYS.toMillis(1); - Pattern p = Pattern.compile("(.*)until\\s(([\\s\\w:,])+).\\sYou(.*)"); - Matcher m = p.matcher(probation.text()); + long probTimestamp = System.currentTimeMillis() + TimeUnit.DAYS.toMillis(1); + Matcher m = PROBATION_MESSAGE_REGEX.matcher(probation.text()); if (m.find()) { String date = m.group(2); //for example January 11, 2013 10:35 AM CST @@ -182,10 +181,10 @@ public static AwfulError checkPageErrors(Document page, AwfulPreferences prefs) //TODO this might have timezone issues? probTimestamp = probationFormat.parse(date).getTime(); } catch (ParseException e) { - Timber.tag(TAG).w(e, "checkPageErrors: couldn't parse probation date text: %s", date); + Timber.w(e, "checkPageErrors: couldn't parse probation date text: %s", date); } } else { - Timber.tag(TAG).w("checkPageErrors: couldn't find expected probation date text!\nFull text: %s", probation.text()); + Timber.w("checkPageErrors: couldn't find expected probation date text!\nFull text: %s", probation.text()); } prefs.setPreference(Keys.PROBATION_TIME, probTimestamp);