From 6ea7367bf3fd6fefa62400022d513047a7132554 Mon Sep 17 00:00:00 2001 From: misaochan Date: Mon, 22 Jan 2018 16:47:42 +1000 Subject: [PATCH 1/8] Add check for moveToFirst() --- .../commons/modifications/ModificationsSyncAdapter.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java b/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java index 85bfedcfa..6ffe52eb1 100644 --- a/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java +++ b/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java @@ -99,9 +99,13 @@ public class ModificationsSyncAdapter extends AbstractThreadedSyncAdapter { } catch (RemoteException e) { throw new RuntimeException(e); } - contributionCursor.moveToFirst(); - contrib = contributionDao.fromCursor(contributionCursor); + if (contributionCursor != null) { + contributionCursor.moveToFirst(); + } + + contrib = contributionDao.fromCursor(contributionCursor); + if (contrib.getState() == Contribution.STATE_COMPLETED) { String pageContent; try { From 2d677d9dad2f40667c2ae726e20f306c2198a967 Mon Sep 17 00:00:00 2001 From: misaochan Date: Mon, 22 Jan 2018 16:48:33 +1000 Subject: [PATCH 2/8] Add null check to line that causes crash --- .../nrw/commons/modifications/ModificationsSyncAdapter.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java b/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java index 6ffe52eb1..045681435 100644 --- a/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java +++ b/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java @@ -105,8 +105,8 @@ public class ModificationsSyncAdapter extends AbstractThreadedSyncAdapter { } contrib = contributionDao.fromCursor(contributionCursor); - - if (contrib.getState() == Contribution.STATE_COMPLETED) { + + if (contrib != null && contrib.getState() == Contribution.STATE_COMPLETED) { String pageContent; try { pageContent = mwApi.revisionsByFilename(contrib.getFilename()); From 895d64dce97c1c265c857a35d2340a5e885c58c8 Mon Sep 17 00:00:00 2001 From: misaochan Date: Mon, 22 Jan 2018 16:50:47 +1000 Subject: [PATCH 3/8] Prevent NPE from string literal comparison --- .../free/nrw/commons/modifications/ModificationsSyncAdapter.java | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java b/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java index 045681435..c594306aa 100644 --- a/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java +++ b/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java @@ -128,7 +128,6 @@ public class ModificationsSyncAdapter extends AbstractThreadedSyncAdapter { Timber.d("Response is %s", editResult); - if (!editResult.equals("Success")) { // FIXME: Log this somewhere else Timber.d("Non success result! %s", editResult); } else { From 417013dcd8ea5196f637ff828adab464929193dc Mon Sep 17 00:00:00 2001 From: misaochan Date: Mon, 22 Jan 2018 16:51:02 +1000 Subject: [PATCH 4/8] Prevent NPE from string literal comparison --- .../free/nrw/commons/modifications/ModificationsSyncAdapter.java | 1 + 1 file changed, 1 insertion(+) diff --git a/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java b/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java index c594306aa..aef4a9b7a 100644 --- a/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java +++ b/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java @@ -128,6 +128,7 @@ public class ModificationsSyncAdapter extends AbstractThreadedSyncAdapter { Timber.d("Response is %s", editResult); + if (!"Success".equals(editResult)) { // FIXME: Log this somewhere else Timber.d("Non success result! %s", editResult); } else { From 00eb55692eb27505068ba439b20430409ff955a1 Mon Sep 17 00:00:00 2001 From: misaochan Date: Mon, 22 Jan 2018 16:54:22 +1000 Subject: [PATCH 5/8] Make error logs more polite --- .../nrw/commons/modifications/ModificationsSyncAdapter.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java b/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java index aef4a9b7a..ee0447519 100644 --- a/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java +++ b/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java @@ -111,7 +111,7 @@ public class ModificationsSyncAdapter extends AbstractThreadedSyncAdapter { try { pageContent = mwApi.revisionsByFilename(contrib.getFilename()); } catch (IOException e) { - Timber.d("Network fuckup on modifications sync!"); + Timber.d("Network messed up on modifications sync!"); continue; } @@ -122,7 +122,7 @@ public class ModificationsSyncAdapter extends AbstractThreadedSyncAdapter { try { editResult = mwApi.edit(editToken, processedPageContent, contrib.getFilename(), sequence.getEditSummary()); } catch (IOException e) { - Timber.d("Network fuckup on modifications sync!"); + Timber.d("Network messed up on modifications sync!"); continue; } From c80d1066364d1bd4c60703f06d112066ddba1289 Mon Sep 17 00:00:00 2001 From: misaochan Date: Mon, 22 Jan 2018 16:55:32 +1000 Subject: [PATCH 6/8] Add TODO for sole remaining flagged NPE --- .../free/nrw/commons/modifications/ModificationsSyncAdapter.java | 1 + 1 file changed, 1 insertion(+) diff --git a/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java b/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java index ee0447519..24df60513 100644 --- a/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java +++ b/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java @@ -95,6 +95,7 @@ public class ModificationsSyncAdapter extends AbstractThreadedSyncAdapter { Cursor contributionCursor; try { + //TODO: How do we prevent this NPE? contributionCursor = contributionsClient.query(sequence.getMediaUri(), null, null, null, null); } catch (RemoteException e) { throw new RuntimeException(e); From b42340b03a7c5f6b2208f3de8152233b401b80ae Mon Sep 17 00:00:00 2001 From: misaochan Date: Wed, 31 Jan 2018 23:18:28 +1000 Subject: [PATCH 7/8] Return early if contributionsClient == null --- .../commons/modifications/ModificationsSyncAdapter.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java b/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java index 24df60513..5fcaffc99 100644 --- a/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java +++ b/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java @@ -92,10 +92,13 @@ public class ModificationsSyncAdapter extends AbstractThreadedSyncAdapter { while (!allModifications.isAfterLast()) { ModifierSequence sequence = modifierSequenceDao.fromCursor(allModifications); Contribution contrib; - Cursor contributionCursor; + + if (contributionsClient == null) { + return; + } + try { - //TODO: How do we prevent this NPE? contributionCursor = contributionsClient.query(sequence.getMediaUri(), null, null, null, null); } catch (RemoteException e) { throw new RuntimeException(e); From 87fc6903b8bd4a207c4c8a2031a7bf880f184766 Mon Sep 17 00:00:00 2001 From: misaochan Date: Wed, 31 Jan 2018 23:25:06 +1000 Subject: [PATCH 8/8] Added error logging --- .../free/nrw/commons/modifications/ModificationsSyncAdapter.java | 1 + 1 file changed, 1 insertion(+) diff --git a/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java b/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java index 5fcaffc99..fa20be595 100644 --- a/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java +++ b/app/src/main/java/fr/free/nrw/commons/modifications/ModificationsSyncAdapter.java @@ -95,6 +95,7 @@ public class ModificationsSyncAdapter extends AbstractThreadedSyncAdapter { Cursor contributionCursor; if (contributionsClient == null) { + Timber.e("ContributionsClient is null. This should not happen!"); return; }