Niedzielski has uploaded a new change for review. https://gerrit.wikimedia.org/r/289241
Change subject: Fix crash when trying to save a page with a colon ...................................................................... Fix crash when trying to save a page with a colon • Request namespace in the MediaWiki page query. In the Content Service and whenever the property is unavailable, continue to derive it. For distinct namespace (not namespace localized name) logic, use Namespace. The previous implementation attempted to convert the localized name to a number which failed when the article's title contained a colon. • Move some of the legacy namespace code into the Namespace class for reusability. Bug: T135523 Change-Id: I0e858b5d456bad150b2ae3313acda1369bd7abb9 --- M app/src/main/java/org/wikipedia/page/Namespace.java M app/src/main/java/org/wikipedia/page/Page.java M app/src/main/java/org/wikipedia/page/PageFragment.java M app/src/main/java/org/wikipedia/page/PageProperties.java M app/src/main/java/org/wikipedia/page/PageTitle.java M app/src/main/java/org/wikipedia/readinglist/page/database/ReadingListDaoProxy.java M app/src/main/java/org/wikipedia/server/PageLeadProperties.java M app/src/main/java/org/wikipedia/server/mwapi/MwPageLead.java M app/src/main/java/org/wikipedia/server/mwapi/MwPageService.java M app/src/main/java/org/wikipedia/server/restbase/RbPageCombo.java M app/src/main/java/org/wikipedia/server/restbase/RbPageLead.java 11 files changed, 108 insertions(+), 46 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/apps/android/wikipedia refs/changes/41/289241/1 diff --git a/app/src/main/java/org/wikipedia/page/Namespace.java b/app/src/main/java/org/wikipedia/page/Namespace.java index 900f337..d17e9cf 100644 --- a/app/src/main/java/org/wikipedia/page/Namespace.java +++ b/app/src/main/java/org/wikipedia/page/Namespace.java @@ -3,15 +3,21 @@ import android.support.annotation.NonNull; import android.support.annotation.Nullable; +import org.wikipedia.Site; import org.wikipedia.model.CodeEnum; import org.wikipedia.model.EnumCode; import org.wikipedia.model.EnumCodeMap; +import org.wikipedia.staticdata.FileAliasData; +import org.wikipedia.staticdata.SpecialAliasData; import org.wikipedia.util.StringUtil; -// https://en.wikipedia.org/wiki/Wikipedia:Namespace -// https://www.mediawiki.org/wiki/Extension_default_namespaces -// https://github.com/wikimedia/wikipedia-ios/blob/master/Wikipedia/Code/NSNumber+MWKTitleNamespace.h -// https://www.mediawiki.org/wiki/Manual:Namespace#Built-in_namespaces +/** An enumeration describing the different possible namespace codes. Do not attempt to use this + * class to preserve URL path information such as Talk: or User: or localization. + * @see <a href='https://en.wikipedia.org/wiki/Wikipedia:Namespace'>Wikipedia:Namespace</a> + * @see <a href='https://www.mediawiki.org/wiki/Extension_default_namespaces'>Extension default namespaces</a> + * @see <a href='https://github.com/wikimedia/wikipedia-ios/blob/master/Wikipedia/Code/NSNumber+MWKTitleNamespace.h'>NSNumber+MWKTitleNamespace.h (iOS implementation)</a> + * @see <a href='https://www.mediawiki.org/wiki/Manual:Namespace#Built-in_namespaces'>Manual:Namespace</a> + */ public enum Namespace implements EnumCode { MEDIA(-2), SPECIAL(-1) { @@ -65,6 +71,8 @@ private final int code; + /** Warning: this method returns an English translation for the current namespace. */ + @Deprecated @Nullable public String toLegacyString() { String string = this == MAIN ? null : this.name(); @@ -72,6 +80,22 @@ StringUtil.capitalizeFirstChar(string.toLowerCase()); } return string; + } + + /** Warning: this method is localized only for File and Special pages. */ + @Deprecated @NonNull public static Namespace fromLegacyString(@NonNull Site site, + @Nullable String name) { + String filePageAlias = FileAliasData.valueFor(site.languageCode()); + if (filePageAlias.equals(name)) { + return Namespace.FILE; + } + + String specialPageAlias = SpecialAliasData.valueFor(site.languageCode()); + if (specialPageAlias.equals(name)) { + return Namespace.SPECIAL; + } + + return Namespace.MAIN; } @NonNull @@ -103,4 +127,4 @@ Namespace(int code) { this.code = code; } -} \ No newline at end of file +} diff --git a/app/src/main/java/org/wikipedia/page/Page.java b/app/src/main/java/org/wikipedia/page/Page.java index 24cb13f..89f13d9 100755 --- a/app/src/main/java/org/wikipedia/page/Page.java +++ b/app/src/main/java/org/wikipedia/page/Page.java @@ -99,7 +99,7 @@ } public boolean couldHaveReadMoreSection() { - return (!isFilePage() && !getTitle().isSpecial() && getTitle().getNamespace() == null); + return getTitle().namespace() == Namespace.MAIN; } @Override @@ -141,7 +141,7 @@ } public boolean isArticle() { - return !isMainPage() && getTitle().getNamespace() == null; + return !isMainPage() && getTitle().namespace() == Namespace.MAIN; } public JSONObject toJSON() { diff --git a/app/src/main/java/org/wikipedia/page/PageFragment.java b/app/src/main/java/org/wikipedia/page/PageFragment.java index 341e92e..046567b 100755 --- a/app/src/main/java/org/wikipedia/page/PageFragment.java +++ b/app/src/main/java/org/wikipedia/page/PageFragment.java @@ -17,7 +17,6 @@ import android.support.v7.app.AppCompatActivity; import android.support.v7.view.ActionMode; import android.text.Html; -import android.text.TextUtils; import android.util.Log; import android.util.TypedValue; import android.view.Gravity; @@ -385,7 +384,7 @@ return; } getPageActivity().dismissBottomSheet(); - if (!TextUtils.isEmpty(title.getNamespace()) || !app.isLinkPreviewEnabled()) { + if (title.namespace() != Namespace.MAIN || !app.isLinkPreviewEnabled()) { HistoryEntry historyEntry = new HistoryEntry(title, HistoryEntry.SOURCE_INTERNAL_LINK); getPageActivity().loadPage(title, historyEntry); new LinkPreviewFunnel(app).logNavigate(); diff --git a/app/src/main/java/org/wikipedia/page/PageProperties.java b/app/src/main/java/org/wikipedia/page/PageProperties.java index 40bfd36..665c77f 100644 --- a/app/src/main/java/org/wikipedia/page/PageProperties.java +++ b/app/src/main/java/org/wikipedia/page/PageProperties.java @@ -3,12 +3,14 @@ import android.location.Location; import android.os.Parcel; import android.os.Parcelable; +import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.text.TextUtils; import android.util.Log; import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; +import org.wikipedia.Site; import org.wikipedia.server.PageLeadProperties; import org.wikipedia.util.StringUtil; @@ -23,8 +25,10 @@ public class PageProperties implements Parcelable { private static final String JSON_NAME_TITLE_PRONUNCIATION_URL = "titlePronunciationUrl"; private static final String JSON_NAME_GEO = "geo"; + private static final String JSON_NAME_NAMESPACE = "namespace"; private final int pageId; + @NonNull private final Namespace namespace; private final long revisionId; private final Date lastModified; private final String displayTitleText; @@ -48,8 +52,9 @@ * Side note: Should later be moved out of this class but I like the similarities with * PageProperties(JSONObject). */ - public PageProperties(PageLeadProperties core) { + public PageProperties(@NonNull Site site, PageLeadProperties core) { pageId = core.getId(); + namespace = core.getNamespace(site); revisionId = core.getRevision(); displayTitleText = StringUtil.emptyIfNull(core.getDisplayTitle()); titlePronunciationUrl = core.getTitlePronunciationUrl(); @@ -80,6 +85,7 @@ */ public PageProperties(JSONObject json) { pageId = json.optInt("id"); + namespace = Namespace.of(json.optInt(JSON_NAME_NAMESPACE)); revisionId = json.optLong("revision"); displayTitleText = json.optString("displaytitle"); titlePronunciationUrl = json.optString(JSON_NAME_TITLE_PRONUNCIATION_URL, null); @@ -120,6 +126,10 @@ public int getPageId() { return pageId; + } + + @NonNull public Namespace getNamespace() { + return namespace; } public long getRevisionId() { @@ -185,6 +195,7 @@ @Override public void writeToParcel(Parcel parcel, int flags) { parcel.writeInt(pageId); + parcel.writeInt(namespace.code()); parcel.writeLong(revisionId); parcel.writeLong(lastModified.getTime()); parcel.writeString(displayTitleText); @@ -201,6 +212,7 @@ private PageProperties(Parcel in) { pageId = in.readInt(); + namespace = Namespace.of(in.readInt()); revisionId = in.readLong(); lastModified = new Date(in.readLong()); displayTitleText = in.readString(); @@ -240,6 +252,7 @@ PageProperties that = (PageProperties) o; return pageId == that.pageId + && namespace == that.namespace && revisionId == that.revisionId && lastModified.equals(that.lastModified) && displayTitleText.equals(that.displayTitleText) @@ -268,6 +281,7 @@ result = 31 * result + (leadImageName != null ? leadImageName.hashCode() : 0); result = 31 * result + (canEdit ? 1 : 0); result = 31 * result + pageId; + result = 31 * result + namespace.code(); result = 31 * result + (int) revisionId; return result; } @@ -281,6 +295,7 @@ JSONObject json = new JSONObject(); try { json.put("id", pageId); + json.put(JSON_NAME_NAMESPACE, namespace.code()); json.put("revision", revisionId); json.put("lastmodified", getIso8601DateFormat().format(getLastModified())); json.put("displaytitle", displayTitleText); diff --git a/app/src/main/java/org/wikipedia/page/PageTitle.java b/app/src/main/java/org/wikipedia/page/PageTitle.java index 7418030..7242c4e 100644 --- a/app/src/main/java/org/wikipedia/page/PageTitle.java +++ b/app/src/main/java/org/wikipedia/page/PageTitle.java @@ -2,15 +2,14 @@ import android.os.Parcel; import android.os.Parcelable; +import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.text.TextUtils; import org.json.JSONException; import org.json.JSONObject; import org.wikipedia.Site; -import org.wikipedia.staticdata.FileAliasData; import org.wikipedia.staticdata.MainPageNameData; -import org.wikipedia.staticdata.SpecialAliasData; import java.io.UnsupportedEncodingException; import java.net.URLEncoder; @@ -47,7 +46,9 @@ * * [[Utilisateur:Deskana]] on frwiki will have a namespace of "Utilisateur", even if you got * to the page by going to [[User:Deskana]] and having MediaWiki automatically redirect you. */ - // TODO: use Namespace. Clients shouldn't have to bear this knowledge to access a String. + // TODO: remove. This legacy code is the localized namespace name (File, Special, Talk, etc) but + // isn't consistent across titles. e.g., articles with colons, such as RTÉ News: Six One, + // are broken. @Nullable private final String namespace; private final String text; private final String fragment; @@ -114,6 +115,15 @@ @Nullable public String getNamespace() { return namespace; + } + + @NonNull public Namespace namespace() { + if (properties != null) { + return properties.getNamespace(); + } + + // Properties has the accurate namespace but it doesn't exist. Guess based on title. + return Namespace.fromLegacyString(site, namespace); } public Site getSite() { @@ -258,35 +268,22 @@ return namespace == null ? getText() : namespace + ":" + getText(); } - - private Boolean isFilePage = null; /** * Check if the Title represents a File: * * @return true if it is a File page, false if not */ public boolean isFilePage() { - if (isFilePage == null) { - String filePageAlias = FileAliasData.valueFor(getSite().languageCode()); - isFilePage = filePageAlias.equals(getNamespace()); - } - - return isFilePage; + return namespace().file(); } - private Boolean isSpecial = null; /** * Check if the Title represents a special page * * @return true if it is a special page, false if not */ public boolean isSpecial() { - if (isSpecial == null) { - String specialPageAlias = SpecialAliasData.valueFor(getSite().languageCode()); - isSpecial = specialPageAlias.equals(getNamespace()); - } - - return isSpecial; + return namespace().special(); } @Override diff --git a/app/src/main/java/org/wikipedia/readinglist/page/database/ReadingListDaoProxy.java b/app/src/main/java/org/wikipedia/readinglist/page/database/ReadingListDaoProxy.java index e4e2229..bfd8e4b 100644 --- a/app/src/main/java/org/wikipedia/readinglist/page/database/ReadingListDaoProxy.java +++ b/app/src/main/java/org/wikipedia/readinglist/page/database/ReadingListDaoProxy.java @@ -3,7 +3,6 @@ import android.support.annotation.NonNull; import android.util.Base64; -import org.wikipedia.page.Namespace; import org.wikipedia.page.PageTitle; import org.wikipedia.readinglist.ReadingList; import org.wikipedia.readinglist.page.ReadingListPage; @@ -35,8 +34,7 @@ .key(key(title)) .listKeys(listKey(list)) .site(title.getSite()) - // TODO: unmarshal namespace when received, not when used. - .namespace(title.getNamespace() == null ? Namespace.MAIN : Namespace.of(Integer.parseInt(title.getNamespace()))) + .namespace(title.namespace()) .title(title.getDisplayText()) .diskPageRevision(title.hasProperties() ? title.getProperties().getRevisionId() : 0) .mtime(now) @@ -65,4 +63,4 @@ } private ReadingListDaoProxy() { } -} \ No newline at end of file +} diff --git a/app/src/main/java/org/wikipedia/server/PageLeadProperties.java b/app/src/main/java/org/wikipedia/server/PageLeadProperties.java index 49076f2..b8a6d6e 100644 --- a/app/src/main/java/org/wikipedia/server/PageLeadProperties.java +++ b/app/src/main/java/org/wikipedia/server/PageLeadProperties.java @@ -1,8 +1,11 @@ package org.wikipedia.server; +import org.wikipedia.Site; +import org.wikipedia.page.Namespace; import org.wikipedia.page.Section; import android.location.Location; +import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.support.annotation.VisibleForTesting; @@ -15,6 +18,8 @@ int getId(); + @NonNull Namespace getNamespace(@NonNull Site site); + long getRevision(); @Nullable diff --git a/app/src/main/java/org/wikipedia/server/mwapi/MwPageLead.java b/app/src/main/java/org/wikipedia/server/mwapi/MwPageLead.java index a3d1624..c4e2a2b 100644 --- a/app/src/main/java/org/wikipedia/server/mwapi/MwPageLead.java +++ b/app/src/main/java/org/wikipedia/server/mwapi/MwPageLead.java @@ -5,6 +5,8 @@ import android.support.annotation.Nullable; import android.support.annotation.VisibleForTesting; +import org.wikipedia.Site; +import org.wikipedia.page.Namespace; import org.wikipedia.page.Page; import org.wikipedia.page.PageProperties; import org.wikipedia.page.PageTitle; @@ -50,7 +52,7 @@ public Page toPage(@NonNull PageTitle title) { return new Page(adjustPageTitle(title), mobileview.getSections(), - mobileview.toPageProperties()); + mobileview.toPageProperties(title.getSite())); } private PageTitle adjustPageTitle(@NonNull PageTitle title) { @@ -99,6 +101,7 @@ */ public static class Mobileview implements PageLeadProperties { private int id; + private int namespace; private long revision; @Nullable private String lastmodified; @Nullable private String displaytitle; @@ -115,13 +118,17 @@ @Nullable private List<Section> sections; /** Converter */ - public PageProperties toPageProperties() { - return new PageProperties(this); + public PageProperties toPageProperties(@NonNull Site site) { + return new PageProperties(site, this); } @Override public int getId() { return id; + } + + @Override @NonNull public Namespace getNamespace(@NonNull Site site) { + return Namespace.of(namespace); } @Override @@ -238,4 +245,4 @@ return url; } } -} \ No newline at end of file +} diff --git a/app/src/main/java/org/wikipedia/server/mwapi/MwPageService.java b/app/src/main/java/org/wikipedia/server/mwapi/MwPageService.java index 3c51020..291fa49 100644 --- a/app/src/main/java/org/wikipedia/server/mwapi/MwPageService.java +++ b/app/src/main/java/org/wikipedia/server/mwapi/MwPageService.java @@ -185,10 +185,10 @@ */ @Headers("x-analytics: pageview=1") @GET("w/api.php?action=mobileview&format=json&formatversion=2&prop=" - + "text%7Csections%7Clanguagecount%7Cthumb%7Cimage%7Cid%7Crevision%7Cdescription" - + "%7Clastmodified%7Cnormalizedtitle%7Cdisplaytitle%7Cprotection%7Ceditable" - + "&onlyrequestedsections=1§ions=0§ionprop=toclevel%7Cline%7Canchor" - + "&noheadings=true") + + "text%7Csections%7Clanguagecount%7Cthumb%7Cimage%7Cid%7Cnamespace%7Crevision" + + "%7Cdescription%7Clastmodified%7Cnormalizedtitle%7Cdisplaytitle%7Cprotection" + + "%7Ceditable&onlyrequestedsections=1§ions=0§ionprop=toclevel%7Cline" + + "%7Canchor&noheadings=true") Call<MwPageLead> pageLead(@Query("page") String title, @Query("thumbsize") int leadImageThumbWidth, @Query("noimages") Boolean noImages); diff --git a/app/src/main/java/org/wikipedia/server/restbase/RbPageCombo.java b/app/src/main/java/org/wikipedia/server/restbase/RbPageCombo.java index c8477cc..8a5284b 100644 --- a/app/src/main/java/org/wikipedia/server/restbase/RbPageCombo.java +++ b/app/src/main/java/org/wikipedia/server/restbase/RbPageCombo.java @@ -1,5 +1,6 @@ package org.wikipedia.server.restbase; +import org.wikipedia.Site; import org.wikipedia.page.Page; import org.wikipedia.page.PageProperties; import org.wikipedia.page.PageTitle; @@ -7,6 +8,7 @@ import org.wikipedia.util.log.L; import android.location.Location; +import android.support.annotation.NonNull; import android.support.annotation.Nullable; @@ -48,7 +50,8 @@ if (lead == null) { throw new RuntimeException("lead is null. Check for errors before use!"); } - Page page = new Page(lead.adjustPageTitle(title), lead.getSections(), toPageProperties()); + Page page = new Page(lead.adjustPageTitle(title), lead.getSections(), + toPageProperties(title.getSite())); if (remaining != null) { page.augmentRemainingSections(remaining.getSections()); } @@ -73,7 +76,7 @@ } /** Converter */ - public PageProperties toPageProperties() { - return new PageProperties(lead); + public PageProperties toPageProperties(@NonNull Site site) { + return new PageProperties(site, lead); } } diff --git a/app/src/main/java/org/wikipedia/server/restbase/RbPageLead.java b/app/src/main/java/org/wikipedia/server/restbase/RbPageLead.java index 50c963c..e12390a 100644 --- a/app/src/main/java/org/wikipedia/server/restbase/RbPageLead.java +++ b/app/src/main/java/org/wikipedia/server/restbase/RbPageLead.java @@ -7,7 +7,9 @@ import com.google.gson.annotations.JsonAdapter; import com.google.gson.annotations.SerializedName; +import org.wikipedia.Site; import org.wikipedia.page.GeoTypeAdapter; +import org.wikipedia.page.Namespace; import org.wikipedia.page.Page; import org.wikipedia.page.PageProperties; import org.wikipedia.page.PageTitle; @@ -15,6 +17,7 @@ import org.wikipedia.server.PageLead; import org.wikipedia.server.PageLeadProperties; import org.wikipedia.server.Protection; +import org.wikipedia.util.StringUtil; import org.wikipedia.util.UriUtil; import org.wikipedia.util.log.L; @@ -70,7 +73,7 @@ public Page toPage(PageTitle title) { return new Page(adjustPageTitle(title), getSections(), - toPageProperties()); + toPageProperties(title.getSite())); } /* package */ PageTitle adjustPageTitle(PageTitle title) { @@ -95,13 +98,17 @@ } /** Converter */ - public PageProperties toPageProperties() { - return new PageProperties(this); + public PageProperties toPageProperties(@NonNull Site site) { + return new PageProperties(site, this); } @Override public int getId() { return id; + } + + @NonNull @Override public Namespace getNamespace(@NonNull Site site) { + return guessNamespace(site, StringUtil.emptyIfNull(normalizedtitle)); } @Override @@ -201,6 +208,13 @@ this.leadImageThumbWidth = leadImageThumbWidth; } + // TODO: remove this method and #getNamespace() Site dependency when T135141 is fixed. + @NonNull private Namespace guessNamespace(@NonNull Site site, @NonNull String title) { + String[] parts = title.split(":", -1); + String name = parts.length > 1 ? parts[0] : null; + return Namespace.fromLegacyString(site, name); + } + /** * For the lead image File: page name */ -- To view, visit https://gerrit.wikimedia.org/r/289241 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0e858b5d456bad150b2ae3313acda1369bd7abb9 Gerrit-PatchSet: 1 Gerrit-Project: apps/android/wikipedia Gerrit-Branch: master Gerrit-Owner: Niedzielski <sniedziel...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
