BearND has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/251667

Change subject: Version saved pages
......................................................................

Version saved pages

Pages saved that were downloaded from MW API still need all the original DOM 
transformations,
irrespective of the current "useRestBase" setting.
At the very least, we wouldn't want to move the first paragraph in the lead 
section up twice.
Then the content would be displayed in the wrong order. We want this to happen 
exactly once.

Conversely, pages saved which were downloaded from the Mobile Content Service 
should
only get some of the DOM transformations.

Change-Id: Iadc31079e54c83488c7bf74d792b5b70e06c102f
---
M app/src/main/java/org/wikipedia/page/JsonPageLoadStrategy.java
M app/src/main/java/org/wikipedia/page/Page.java
A app/src/test/java/org/wikipedia/page/PageTest.java
M app/src/test/java/org/wikipedia/server/BasePageLeadTest.java
4 files changed, 95 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/apps/android/wikipedia 
refs/changes/67/251667/1

diff --git a/app/src/main/java/org/wikipedia/page/JsonPageLoadStrategy.java 
b/app/src/main/java/org/wikipedia/page/JsonPageLoadStrategy.java
index becee7c..48172f6 100644
--- a/app/src/main/java/org/wikipedia/page/JsonPageLoadStrategy.java
+++ b/app/src/main/java/org/wikipedia/page/JsonPageLoadStrategy.java
@@ -17,7 +17,6 @@
 import org.wikipedia.pageimages.PageImagesTask;
 import org.wikipedia.savedpages.LoadSavedPageTask;
 import org.wikipedia.search.SearchBarHideHandler;
-import org.wikipedia.settings.Prefs;
 import org.wikipedia.util.DimenUtil;
 import org.wikipedia.util.L10nUtil;
 import org.wikipedia.util.PageLoadUtil;
@@ -572,7 +571,7 @@
                     .put("isBeta", app.isPreProdRelease()) // True for any 
non-production release type
                     .put("siteLanguage", 
model.getTitle().getSite().getLanguageCode())
                     .put("isMainPage", page.isMainPage())
-                    .put("fromRestBase", Prefs.useRestBase())
+                    .put("fromRestBase", page.isFromRestBase())
                     .put("apiLevel", Build.VERSION.SDK_INT);
         } catch (JSONException e) {
             throw new RuntimeException(e);
diff --git a/app/src/main/java/org/wikipedia/page/Page.java 
b/app/src/main/java/org/wikipedia/page/Page.java
index abf31f3..5102cfd 100755
--- a/app/src/main/java/org/wikipedia/page/Page.java
+++ b/app/src/main/java/org/wikipedia/page/Page.java
@@ -4,6 +4,10 @@
 import org.json.JSONException;
 import org.json.JSONObject;
 import org.wikipedia.page.gallery.GalleryCollection;
+import org.wikipedia.settings.Prefs;
+
+import android.support.annotation.NonNull;
+import android.support.annotation.VisibleForTesting;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -12,6 +16,11 @@
  * Represents a particular page along with its full contents.
  */
 public class Page {
+    @VisibleForTesting
+    static final int MW_VERSION = 0;
+    @VisibleForTesting
+    static final int RESTBASE_VERSION = 1;
+
     private final PageTitle title;
     private final List<Section> sections;
     private final PageProperties pageProperties;
@@ -23,6 +32,15 @@
      * activity will then be able to retrieve the page's gallery collection 
from cache.
      */
     private GalleryCollection galleryCollection;
+
+    /**
+     * An indicator what payload version the page content was originally 
retrieved from.
+     * If it's set to RESTBASE_VERSION the it came from the Mobile Content 
Service
+     * (via RESTBase). This is esp. useful for saved pages, so that an older 
saved page will get the
+     * correct kind of DOM transformations applied.
+     */
+    private int version = MW_VERSION;
+
     public GalleryCollection getGalleryCollection() {
         return galleryCollection;
     }
@@ -31,17 +49,39 @@
     }
 
     /** Regular constructor */
-    public Page(PageTitle title, List<Section> sections, PageProperties 
pageProperties) {
+    public Page(@NonNull PageTitle title, @NonNull List<Section> sections,
+                @NonNull PageProperties pageProperties) {
+        if (Prefs.useRestBase()) {
+            this.version = RESTBASE_VERSION;
+        }
+        this.title = title;
+        this.sections = sections;
+        this.pageProperties = pageProperties;
+    }
+
+    @VisibleForTesting
+    Page(@NonNull PageTitle title, @NonNull List<Section> sections,
+         @NonNull PageProperties pageProperties, int version) {
+        this.version = version;
         this.title = title;
         this.sections = sections;
         this.pageProperties = pageProperties;
     }
 
     /** Copy constructor */
-    public Page(Page orig) {
+    public Page(@NonNull Page orig) {
+        this.version = orig.version;
         this.title = orig.title;
         this.sections = orig.sections;
         this.pageProperties = orig.pageProperties;
+    }
+
+    /**
+     * This could also be called getVersion but since there are only two 
different versions
+     * I like to call it isFromRestBase to make it clearer.
+     */
+    public boolean isFromRestBase() {
+        return version == RESTBASE_VERSION;
     }
 
     public PageTitle getTitle() {
@@ -90,6 +130,7 @@
                 + "title=" + title
                 + ", sections=" + sections
                 + ", pageProperties=" + pageProperties
+                + ", version=" + version
                 + '}';
     }
 
@@ -108,6 +149,9 @@
     public JSONObject toJSON() {
         JSONObject json = new JSONObject();
         try {
+            if (version > MW_VERSION) {
+                json.putOpt("version", version);
+            }
             json.putOpt("title", getTitle().toJSON());
             JSONArray sectionsJSON = new JSONArray();
             for (Section section : getSections()) {
@@ -126,6 +170,7 @@
     }
 
     public Page(JSONObject json) {
+        version = json.optInt("version");
         title = new PageTitle(json.optJSONObject("title"));
         JSONArray sectionsJSON = json.optJSONArray("sections");
         sections = new ArrayList<>(sectionsJSON.length());
diff --git a/app/src/test/java/org/wikipedia/page/PageTest.java 
b/app/src/test/java/org/wikipedia/page/PageTest.java
new file mode 100644
index 0000000..ed7d586
--- /dev/null
+++ b/app/src/test/java/org/wikipedia/page/PageTest.java
@@ -0,0 +1,46 @@
+package org.wikipedia.page;
+
+import org.wikipedia.Site;
+import org.wikipedia.server.BasePageLeadTest;
+import org.wikipedia.test.TestRunner;
+
+import org.json.JSONObject;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import java.util.ArrayList;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+/** Unit tests for Page. */
+@RunWith(TestRunner.class)
+public class PageTest {
+    private static final Site SITE = new Site("en.wikipedia.org");
+
+    private PageTitle title;
+    private PageProperties props;
+
+    @Before
+    public void setUp() throws Exception {
+        title = new PageTitle("Main page", SITE, "//foo/thumb.jpg");
+        props = new PageProperties(new 
JSONObject(BasePageLeadTest.getEnglishMainPageJson()));
+    }
+
+    @Test
+    public void testVersion0Marshalling() throws Exception {
+        Page page = new Page(title, new ArrayList<Section>(), props, 
Page.MW_VERSION);
+        assertThat(page.isFromRestBase(), is(false));
+        Page pageClone = new Page(page.toJSON());
+        assertThat(pageClone.isFromRestBase(), is(false));
+    }
+
+    @Test
+    public void testVersion1Marshalling() throws Exception {
+        Page page = new Page(title, new ArrayList<Section>(), props, 
Page.RESTBASE_VERSION);
+        assertThat(page.isFromRestBase(), is(true));
+        Page pageClone = new Page(page.toJSON());
+        assertThat(pageClone.isFromRestBase(), is(true));
+    }
+}
diff --git a/app/src/test/java/org/wikipedia/server/BasePageLeadTest.java 
b/app/src/test/java/org/wikipedia/server/BasePageLeadTest.java
index 094ddc0..7b811bb 100644
--- a/app/src/test/java/org/wikipedia/server/BasePageLeadTest.java
+++ b/app/src/test/java/org/wikipedia/server/BasePageLeadTest.java
@@ -20,7 +20,7 @@
     protected static final String MAIN_PAGE = "Main Page";
 
     @NonNull
-    protected String getEnglishMainPageJson() {
+    public static String getEnglishMainPageJson() {
         return "{"
                 + "\"lastmodified\":\"" + LAST_MODIFIED_DATE + "\","
                 + "\"revision\":" + REVISION + ","

-- 
To view, visit https://gerrit.wikimedia.org/r/251667
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iadc31079e54c83488c7bf74d792b5b70e06c102f
Gerrit-PatchSet: 1
Gerrit-Project: apps/android/wikipedia
Gerrit-Branch: master
Gerrit-Owner: BearND <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to