From 7e311e55679e67b21dab98eea12e881137e6608b Mon Sep 17 00:00:00 2001 From: Mauricio Colli Date: Sat, 19 Oct 2019 21:31:14 -0300 Subject: [PATCH] Fix mess with tab handling and enable ignored tests again - Fix typo in a string resource - Reorder tabs so the default kiosk is on top of the others --- .../newpipe/fragments/MainFragment.java | 22 ++---- .../list/kiosk/DefaultKioskFragment.java | 51 +++++++++++++ .../settings/tabs/ChooseTabsFragment.java | 24 +++--- .../org/schabi/newpipe/settings/tabs/Tab.java | 75 ++++++++----------- .../newpipe/settings/tabs/TabsJsonHelper.java | 27 ++++--- app/src/main/res/values/strings.xml | 2 +- .../schabi/newpipe/settings/tabs/TabTest.java | 2 - .../settings/tabs/TabsJsonHelperTest.java | 24 +++--- 8 files changed, 127 insertions(+), 100 deletions(-) create mode 100644 app/src/main/java/org/schabi/newpipe/fragments/list/kiosk/DefaultKioskFragment.java diff --git a/app/src/main/java/org/schabi/newpipe/fragments/MainFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/MainFragment.java index e0661a49f..085056302 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/MainFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/MainFragment.java @@ -159,27 +159,17 @@ public class MainFragment extends BaseFragment implements TabLayout.OnTabSelecte pagerAdapter.notifyDataSetChanged(); viewPager.setOffscreenPageLimit(pagerAdapter.getCount()); - updateTabsIcon(); - updateTabsContentDescription(); + updateTabsIconAndDescription(); updateCurrentTitle(); } - private void updateTabsIcon() { + private void updateTabsIconAndDescription() { for (int i = 0; i < tabsList.size(); i++) { final TabLayout.Tab tabToSet = tabLayout.getTabAt(i); if (tabToSet != null) { - tabToSet.setIcon(tabsList.get(i).getTabIconRes(activity)); - } - } - } - - private void updateTabsContentDescription() { - for (int i = 0; i < tabsList.size(); i++) { - final TabLayout.Tab tabToSet = tabLayout.getTabAt(i); - if (tabToSet != null) { - final Tab t = tabsList.get(i); - tabToSet.setIcon(t.getTabIconRes(activity)); - tabToSet.setContentDescription(t.getTabName(activity)); + final Tab tab = tabsList.get(i); + tabToSet.setIcon(tab.getTabIconRes(requireContext())); + tabToSet.setContentDescription(tab.getTabName(requireContext())); } } } @@ -217,7 +207,7 @@ public class MainFragment extends BaseFragment implements TabLayout.OnTabSelecte Throwable throwable = null; Fragment fragment = null; try { - fragment = tab.getFragment(); + fragment = tab.getFragment(requireContext()); } catch (ExtractionException e) { throwable = e; } diff --git a/app/src/main/java/org/schabi/newpipe/fragments/list/kiosk/DefaultKioskFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/list/kiosk/DefaultKioskFragment.java new file mode 100644 index 000000000..35b68b094 --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/fragments/list/kiosk/DefaultKioskFragment.java @@ -0,0 +1,51 @@ +package org.schabi.newpipe.fragments.list.kiosk; + +import android.os.Bundle; + +import org.schabi.newpipe.extractor.NewPipe; +import org.schabi.newpipe.extractor.exceptions.ExtractionException; +import org.schabi.newpipe.extractor.kiosk.KioskList; +import org.schabi.newpipe.report.UserAction; +import org.schabi.newpipe.util.KioskTranslator; +import org.schabi.newpipe.util.ServiceHelper; + +public class DefaultKioskFragment extends KioskFragment { + + @Override + public void onCreate(Bundle savedInstanceState) { + super.onCreate(savedInstanceState); + + if (serviceId < 0) { + updateSelectedDefaultKiosk(); + } + } + + @Override + public void onResume() { + super.onResume(); + + if (serviceId != ServiceHelper.getSelectedServiceId(requireContext())) { + if (currentWorker != null) currentWorker.dispose(); + updateSelectedDefaultKiosk(); + reloadContent(); + } + } + + private void updateSelectedDefaultKiosk() { + try { + serviceId = ServiceHelper.getSelectedServiceId(requireContext()); + + final KioskList kioskList = NewPipe.getService(serviceId).getKioskList(); + kioskId = kioskList.getDefaultKioskId(); + url = kioskList.getListLinkHandlerFactoryByType(kioskId).fromId(kioskId).getUrl(); + + kioskTranslatedName = KioskTranslator.getTranslatedKioskName(kioskId, requireContext()); + name = kioskTranslatedName; + + currentInfo = null; + currentNextPageUrl = null; + } catch (ExtractionException e) { + onUnrecoverableError(e, UserAction.REQUESTED_KIOSK, "none", "Loading default kiosk from selected service", 0); + } + } +} diff --git a/app/src/main/java/org/schabi/newpipe/settings/tabs/ChooseTabsFragment.java b/app/src/main/java/org/schabi/newpipe/settings/tabs/ChooseTabsFragment.java index 4297fb13e..6aba2783f 100644 --- a/app/src/main/java/org/schabi/newpipe/settings/tabs/ChooseTabsFragment.java +++ b/app/src/main/java/org/schabi/newpipe/settings/tabs/ChooseTabsFragment.java @@ -231,7 +231,7 @@ public class ChooseTabsFragment extends Fragment { break; case DEFAULT_KIOSK: if (!tabList.contains(tab)) { - returnList.add(new ChooseTabListItem(tab.getTabId(), getString(R.string.default_kiosk_page_sumatry), + returnList.add(new ChooseTabListItem(tab.getTabId(), getString(R.string.default_kiosk_page_summary), ThemeHelper.resolveResourceIdFromAttr(context, R.attr.ic_hot))); } break; @@ -305,23 +305,25 @@ public class ChooseTabsFragment extends Fragment { return; } - String tabName = tab.getTabName(requireContext()); + final String tabName; switch (type) { case BLANK: - tabName = requireContext().getString(R.string.blank_page_summary); - break; - case KIOSK: - tabName = NewPipe.getNameOfService(((Tab.KioskTab) tab).getKioskServiceId()) + "/" + tabName; - break; - case CHANNEL: - tabName = NewPipe.getNameOfService(((Tab.ChannelTab) tab).getChannelServiceId()) + "/" + tabName; + tabName = getString(R.string.blank_page_summary); break; case DEFAULT_KIOSK: - tabName = requireContext().getString(R.string.default_kiosk_page_sumatry); + tabName = getString(R.string.default_kiosk_page_summary); + break; + case KIOSK: + tabName = NewPipe.getNameOfService(((Tab.KioskTab) tab).getKioskServiceId()) + "/" + tab.getTabName(requireContext()); + break; + case CHANNEL: + tabName = NewPipe.getNameOfService(((Tab.ChannelTab) tab).getChannelServiceId()) + "/" + tab.getTabName(requireContext()); + break; + default: + tabName = tab.getTabName(requireContext()); break; } - tabNameView.setText(tabName); tabIconView.setImageResource(tab.getTabIconRes(requireContext())); } diff --git a/app/src/main/java/org/schabi/newpipe/settings/tabs/Tab.java b/app/src/main/java/org/schabi/newpipe/settings/tabs/Tab.java index 64ba3683b..f80a8bb7f 100644 --- a/app/src/main/java/org/schabi/newpipe/settings/tabs/Tab.java +++ b/app/src/main/java/org/schabi/newpipe/settings/tabs/Tab.java @@ -1,6 +1,7 @@ package org.schabi.newpipe.settings.tabs; import android.content.Context; + import androidx.annotation.DrawableRes; import androidx.annotation.NonNull; import androidx.annotation.Nullable; @@ -9,18 +10,20 @@ import androidx.fragment.app.Fragment; import com.grack.nanojson.JsonObject; import com.grack.nanojson.JsonSink; -import org.jsoup.helper.StringUtil; -import org.schabi.newpipe.App; import org.schabi.newpipe.R; import org.schabi.newpipe.extractor.NewPipe; +import org.schabi.newpipe.extractor.StreamingService; import org.schabi.newpipe.extractor.exceptions.ExtractionException; import org.schabi.newpipe.fragments.BlankFragment; import org.schabi.newpipe.fragments.list.channel.ChannelFragment; +import org.schabi.newpipe.fragments.list.kiosk.DefaultKioskFragment; import org.schabi.newpipe.fragments.list.kiosk.KioskFragment; import org.schabi.newpipe.local.bookmark.BookmarkFragment; import org.schabi.newpipe.local.feed.FeedFragment; import org.schabi.newpipe.local.history.StatisticsPlaylistFragment; import org.schabi.newpipe.local.subscription.SubscriptionFragment; +import org.schabi.newpipe.report.ErrorActivity; +import org.schabi.newpipe.report.UserAction; import org.schabi.newpipe.util.KioskTranslator; import org.schabi.newpipe.util.ServiceHelper; import org.schabi.newpipe.util.ThemeHelper; @@ -40,7 +43,7 @@ public abstract class Tab { /** * Return a instance of the fragment that this tab represent. */ - public abstract Fragment getFragment() throws ExtractionException; + public abstract Fragment getFragment(Context context) throws ExtractionException; @Override public boolean equals(Object obj) { @@ -115,12 +118,6 @@ public abstract class Tab { return new KioskTab(jsonObject); case CHANNEL: return new ChannelTab(jsonObject); - case DEFAULT_KIOSK: - DefaultKioskTab tab = new DefaultKioskTab(); - if(!StringUtil.isBlank(tab.getKioskId())){ - return tab; - } - return null; } } @@ -133,13 +130,13 @@ public abstract class Tab { public enum Type { BLANK(new BlankTab()), + DEFAULT_KIOSK(new DefaultKioskTab()), SUBSCRIPTIONS(new SubscriptionsTab()), FEED(new FeedTab()), BOOKMARKS(new BookmarksTab()), HISTORY(new HistoryTab()), KIOSK(new KioskTab()), - CHANNEL(new ChannelTab()), - DEFAULT_KIOSK(new DefaultKioskTab()); + CHANNEL(new ChannelTab()); private Tab tab; @@ -176,7 +173,7 @@ public abstract class Tab { } @Override - public BlankFragment getFragment() { + public BlankFragment getFragment(Context context) { return new BlankFragment(); } } @@ -201,7 +198,7 @@ public abstract class Tab { } @Override - public SubscriptionFragment getFragment() { + public SubscriptionFragment getFragment(Context context) { return new SubscriptionFragment(); } @@ -227,7 +224,7 @@ public abstract class Tab { } @Override - public FeedFragment getFragment() { + public FeedFragment getFragment(Context context) { return new FeedFragment(); } } @@ -252,7 +249,7 @@ public abstract class Tab { } @Override - public BookmarkFragment getFragment() { + public BookmarkFragment getFragment(Context context) { return new BookmarkFragment(); } } @@ -277,7 +274,7 @@ public abstract class Tab { } @Override - public StatisticsPlaylistFragment getFragment() { + public StatisticsPlaylistFragment getFragment(Context context) { return new StatisticsPlaylistFragment(); } } @@ -327,7 +324,7 @@ public abstract class Tab { } @Override - public KioskFragment getFragment() throws ExtractionException { + public KioskFragment getFragment(Context context) throws ExtractionException { return KioskFragment.getInstance(kioskServiceId, kioskId); } @@ -394,7 +391,7 @@ public abstract class Tab { } @Override - public ChannelFragment getFragment() { + public ChannelFragment getFragment(Context context) { return ChannelFragment.getInstance(channelServiceId, channelUrl, channelName); } @@ -428,22 +425,6 @@ public abstract class Tab { public static class DefaultKioskTab extends Tab { public static final int ID = 7; - private int kioskServiceId; - private String kioskId; - - protected DefaultKioskTab() { - initKiosk(); - } - - public void initKiosk() { - this.kioskServiceId = ServiceHelper.getSelectedServiceId(App.getApp()); - try { - this.kioskId = NewPipe.getService(this.kioskServiceId).getKioskList().getDefaultKioskId(); - } catch (ExtractionException e) { - this.kioskId = ""; - } - } - @Override public int getTabId() { return ID; @@ -451,27 +432,31 @@ public abstract class Tab { @Override public String getTabName(Context context) { - return KioskTranslator.getTranslatedKioskName(kioskId, context); + return KioskTranslator.getTranslatedKioskName(getDefaultKioskId(context), context); } @DrawableRes @Override public int getTabIconRes(Context context) { - final int kioskIcon = KioskTranslator.getKioskIcons(kioskId, context); - - if (kioskIcon <= 0) { - throw new IllegalStateException("Kiosk ID is not valid: \"" + kioskId + "\""); - } - - return kioskIcon; + return KioskTranslator.getKioskIcons(getDefaultKioskId(context), context); } @Override - public KioskFragment getFragment() throws ExtractionException { - return KioskFragment.getInstance(kioskServiceId, kioskId); + public DefaultKioskFragment getFragment(Context context) throws ExtractionException { + return new DefaultKioskFragment(); } - public String getKioskId() { + private String getDefaultKioskId(Context context) { + final int kioskServiceId = ServiceHelper.getSelectedServiceId(context); + + String kioskId = ""; + try { + final StreamingService service = NewPipe.getService(kioskServiceId); + kioskId = service.getKioskList().getDefaultKioskId(); + } catch (ExtractionException e) { + ErrorActivity.reportError(context, e, null, null, + ErrorActivity.ErrorInfo.make(UserAction.REQUESTED_KIOSK, "none", "Loading default kiosk from selected service", 0)); + } return kioskId; } } diff --git a/app/src/main/java/org/schabi/newpipe/settings/tabs/TabsJsonHelper.java b/app/src/main/java/org/schabi/newpipe/settings/tabs/TabsJsonHelper.java index 9553e47e1..9f54d59f6 100644 --- a/app/src/main/java/org/schabi/newpipe/settings/tabs/TabsJsonHelper.java +++ b/app/src/main/java/org/schabi/newpipe/settings/tabs/TabsJsonHelper.java @@ -1,7 +1,5 @@ package org.schabi.newpipe.settings.tabs; -import androidx.annotation.Nullable; - import com.grack.nanojson.JsonArray; import com.grack.nanojson.JsonObject; import com.grack.nanojson.JsonParser; @@ -9,18 +7,25 @@ import com.grack.nanojson.JsonParserException; import com.grack.nanojson.JsonStringWriter; import com.grack.nanojson.JsonWriter; -import org.jsoup.helper.StringUtil; - import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; +import androidx.annotation.Nullable; + /** * Class to get a JSON representation of a list of tabs, and the other way around. */ public class TabsJsonHelper { private static final String JSON_TABS_ARRAY_KEY = "tabs"; + private static final List FALLBACK_INITIAL_TABS_LIST = Collections.unmodifiableList(Arrays.asList( + Tab.Type.DEFAULT_KIOSK.getTab(), + Tab.Type.SUBSCRIPTIONS.getTab(), + Tab.Type.BOOKMARKS.getTab() + )); + public static class InvalidJsonException extends Exception { private InvalidJsonException() { super(); @@ -83,16 +88,6 @@ public class TabsJsonHelper { return returnTabs; } - public static List getDefaultTabs(){ - List tabs = new ArrayList<>(); - Tab.DefaultKioskTab tab = new Tab.DefaultKioskTab(); - if(!StringUtil.isBlank(tab.getKioskId())){ - tabs.add(tab); - } - tabs.add(Tab.Type.SUBSCRIPTIONS.getTab()); - tabs.add(Tab.Type.BOOKMARKS.getTab()); - return Collections.unmodifiableList(tabs); - } /** * Get a JSON representation from a list of tabs. * @@ -112,4 +107,8 @@ public class TabsJsonHelper { jsonWriter.end(); return jsonWriter.done(); } + + public static List getDefaultTabs(){ + return FALLBACK_INITIAL_TABS_LIST; + } } \ No newline at end of file diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 6449833ea..a34b00ea9 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -379,7 +379,7 @@ Selection Blank Page Kiosk Page - Default Kiosk + Default Kiosk Subscription Page Feed Page Channel Page diff --git a/app/src/test/java/org/schabi/newpipe/settings/tabs/TabTest.java b/app/src/test/java/org/schabi/newpipe/settings/tabs/TabTest.java index 4b3fbc2e0..45c7c0fff 100644 --- a/app/src/test/java/org/schabi/newpipe/settings/tabs/TabTest.java +++ b/app/src/test/java/org/schabi/newpipe/settings/tabs/TabTest.java @@ -1,6 +1,5 @@ package org.schabi.newpipe.settings.tabs; -import org.junit.Ignore; import org.junit.Test; import java.util.HashSet; @@ -9,7 +8,6 @@ import java.util.Set; import static org.junit.Assert.assertTrue; public class TabTest { - @Ignore @Test public void checkIdDuplication() { final Set usedIds = new HashSet<>(); diff --git a/app/src/test/java/org/schabi/newpipe/settings/tabs/TabsJsonHelperTest.java b/app/src/test/java/org/schabi/newpipe/settings/tabs/TabsJsonHelperTest.java index 20785e548..1f951159f 100644 --- a/app/src/test/java/org/schabi/newpipe/settings/tabs/TabsJsonHelperTest.java +++ b/app/src/test/java/org/schabi/newpipe/settings/tabs/TabsJsonHelperTest.java @@ -5,7 +5,6 @@ import com.grack.nanojson.JsonObject; import com.grack.nanojson.JsonParser; import com.grack.nanojson.JsonParserException; -import org.junit.Ignore; import org.junit.Test; import java.util.Arrays; @@ -21,19 +20,19 @@ public class TabsJsonHelperTest { private static final String JSON_TABS_ARRAY_KEY = "tabs"; private static final String JSON_TAB_ID_KEY = "tab_id"; - @Ignore @Test public void testEmptyAndNullRead() throws TabsJsonHelper.InvalidJsonException { + final List defaultTabs = TabsJsonHelper.getDefaultTabs(); + final String emptyTabsJson = "{\"" + JSON_TABS_ARRAY_KEY + "\":[]}"; List items = TabsJsonHelper.getTabsFromJson(emptyTabsJson); - assertTrue(!items.isEmpty()); + assertEquals(items, defaultTabs); final String nullSource = null; items = TabsJsonHelper.getTabsFromJson(nullSource); - assertTrue(!items.isEmpty()); + assertEquals(items, defaultTabs); } - @Ignore @Test public void testInvalidIdRead() throws TabsJsonHelper.InvalidJsonException { final int blankTabId = Tab.Type.BLANK.getTabId(); @@ -84,17 +83,17 @@ public class TabsJsonHelperTest { return jsonObject.getArray(JSON_TABS_ARRAY_KEY).size() == 0; } - @Ignore @Test public void testSaveAndReading() throws JsonParserException { // Saving final Tab.BlankTab blankTab = new Tab.BlankTab(); + final Tab.DefaultKioskTab defaultKioskTab = new Tab.DefaultKioskTab(); final Tab.SubscriptionsTab subscriptionsTab = new Tab.SubscriptionsTab(); final Tab.ChannelTab channelTab = new Tab.ChannelTab(666, "https://example.org", "testName"); final Tab.KioskTab kioskTab = new Tab.KioskTab(123, "trending_key"); - final List tabs = Arrays.asList(blankTab, subscriptionsTab, channelTab, kioskTab); - String returnedJson = TabsJsonHelper.getJsonToSave(tabs); + final List tabs = Arrays.asList(blankTab, defaultKioskTab, subscriptionsTab, channelTab, kioskTab); + final String returnedJson = TabsJsonHelper.getJsonToSave(tabs); // Reading final JsonObject jsonObject = JsonParser.object().from(returnedJson); @@ -106,16 +105,19 @@ public class TabsJsonHelperTest { final Tab.BlankTab blankTabFromReturnedJson = requireNonNull((Tab.BlankTab) Tab.from(((JsonObject) tabsFromArray.get(0)))); assertEquals(blankTab.getTabId(), blankTabFromReturnedJson.getTabId()); - final Tab.SubscriptionsTab subscriptionsTabFromReturnedJson = requireNonNull((Tab.SubscriptionsTab) Tab.from(((JsonObject) tabsFromArray.get(1)))); + final Tab.DefaultKioskTab defaultKioskTabFromReturnedJson = requireNonNull((Tab.DefaultKioskTab) Tab.from(((JsonObject) tabsFromArray.get(1)))); + assertEquals(defaultKioskTab.getTabId(), defaultKioskTabFromReturnedJson.getTabId()); + + final Tab.SubscriptionsTab subscriptionsTabFromReturnedJson = requireNonNull((Tab.SubscriptionsTab) Tab.from(((JsonObject) tabsFromArray.get(2)))); assertEquals(subscriptionsTab.getTabId(), subscriptionsTabFromReturnedJson.getTabId()); - final Tab.ChannelTab channelTabFromReturnedJson = requireNonNull((Tab.ChannelTab) Tab.from(((JsonObject) tabsFromArray.get(2)))); + final Tab.ChannelTab channelTabFromReturnedJson = requireNonNull((Tab.ChannelTab) Tab.from(((JsonObject) tabsFromArray.get(3)))); assertEquals(channelTab.getTabId(), channelTabFromReturnedJson.getTabId()); assertEquals(channelTab.getChannelServiceId(), channelTabFromReturnedJson.getChannelServiceId()); assertEquals(channelTab.getChannelUrl(), channelTabFromReturnedJson.getChannelUrl()); assertEquals(channelTab.getChannelName(), channelTabFromReturnedJson.getChannelName()); - final Tab.KioskTab kioskTabFromReturnedJson = requireNonNull((Tab.KioskTab) Tab.from(((JsonObject) tabsFromArray.get(3)))); + final Tab.KioskTab kioskTabFromReturnedJson = requireNonNull((Tab.KioskTab) Tab.from(((JsonObject) tabsFromArray.get(4)))); assertEquals(kioskTab.getTabId(), kioskTabFromReturnedJson.getTabId()); assertEquals(kioskTab.getKioskServiceId(), kioskTabFromReturnedJson.getKioskServiceId()); assertEquals(kioskTab.getKioskId(), kioskTabFromReturnedJson.getKioskId());