Refactored WallpaperXMLParser to contain all logic for category fetching This CL aims at refactoring WallpaperXMLParser such that we have all the logic for category fetching in a single place. Bug: 338278751 Test: Tested by building and installing picker on local, also by unit tests. Flag: ACONFIG com.android.wallpaper.refactor_wallpaper_category_flag DEVELOPMENT Change-Id: I15724ebd982e4be8cb3ed681bb059894314526c0
diff --git a/src/com/android/wallpaper/model/WallpaperCategory.java b/src/com/android/wallpaper/model/WallpaperCategory.java index bea8d4b..97e68e3 100755 --- a/src/com/android/wallpaper/model/WallpaperCategory.java +++ b/src/com/android/wallpaper/model/WallpaperCategory.java
@@ -173,6 +173,15 @@ } /** + * Adds the given list of {@link WallpaperInfo} to this category + * @return this for chaining + */ + public Builder addWallpapers(List<WallpaperInfo> wallpapers) { + mWallpapers.addAll(wallpapers); + return this; + } + + /** * If no priority was parsed from the XML attributes for this category, set the priority to * the given value. * @return this for chaining
diff --git a/src/com/android/wallpaper/picker/category/client /DefaultWallpaperCategoryClient.kt b/src/com/android/wallpaper/picker/category/client /DefaultWallpaperCategoryClient.kt index d2d3a39..2176aba 100644 --- a/src/com/android/wallpaper/picker/category/client /DefaultWallpaperCategoryClient.kt +++ b/src/com/android/wallpaper/picker/category/client /DefaultWallpaperCategoryClient.kt
@@ -17,9 +17,7 @@ package com.android.wallpaper.picker.category.client import android.content.Context -import android.util.Log import com.android.wallpaper.R -import com.android.wallpaper.model.Category import com.android.wallpaper.model.DefaultWallpaperInfo import com.android.wallpaper.model.ImageCategory import com.android.wallpaper.model.LegacyPartnerWallpaperInfo @@ -29,14 +27,12 @@ import com.android.wallpaper.module.DefaultCategoryProvider import com.android.wallpaper.module.PartnerProvider import com.android.wallpaper.picker.data.category.CategoryModel -import com.android.wallpaper.util.WallpaperXMLParser +import com.android.wallpaper.util.WallpaperXMLParserInterface import com.android.wallpaper.util.converter.category.CategoryFactory import dagger.hilt.android.qualifiers.ApplicationContext -import java.io.IOException import java.util.Locale import javax.inject.Inject -import org.xmlpull.v1.XmlPullParser -import org.xmlpull.v1.XmlPullParserException +import javax.inject.Singleton /** * This class is responsible for fetching wallpaper categories, listed as follows: @@ -45,13 +41,14 @@ * wallpapers, modern way is described below) * 3. System categories on device (modern way of pre-loading wallpapers on device) */ +@Singleton class DefaultWallpaperCategoryClient @Inject constructor( @ApplicationContext val context: Context, private val partnerProvider: PartnerProvider, private val categoryFactory: CategoryFactory, - private val wallpaperXMLParser: WallpaperXMLParser + private val wallpaperXMLParser: WallpaperXMLParserInterface ) { /** This method is used for fetching and creating the MyPhotos category tile. */ @@ -105,7 +102,6 @@ suspend fun getSystemCategories(): List<CategoryModel> { val partnerRes = partnerProvider.resources val packageName = partnerProvider.packageName - val categories = mutableListOf<Category>() val categoryModels = mutableListOf<CategoryModel>() if (partnerRes == null || packageName == null) { return categoryModels @@ -119,29 +115,8 @@ return categoryModels } - try { - val parser = partnerRes.getXml(wallpapersResId) - val depth = parser.depth - var type: Int - while ( - parser.next().also { type = it } != XmlPullParser.END_TAG || - parser.depth > depth && type != XmlPullParser.END_DOCUMENT - ) { - if (type == XmlPullParser.START_TAG && WallpaperCategory.TAG_NAME == parser.name) { - val category = wallpaperXMLParser.parseCategory(parser) - category?.let { categories.add(it) } - } - } - } catch (e: Exception) { - when (e) { - is IOException, - is XmlPullParserException -> { - Log.w(TAG, "Couldn't read system wallpapers definition", e) - return emptyList() - } - else -> throw e - } - } + val categories = + wallpaperXMLParser.parseSystemCategories(partnerRes.getXml(wallpapersResId)) return categories.map { category -> categoryFactory.getCategoryModel(context, category) } }
diff --git a/src/com/android/wallpaper/util/WallpaperXMLParser.kt b/src/com/android/wallpaper/util/WallpaperXMLParser.kt index 5a0e75f..8136dc0 100644 --- a/src/com/android/wallpaper/util/WallpaperXMLParser.kt +++ b/src/com/android/wallpaper/util/WallpaperXMLParser.kt
@@ -18,15 +18,19 @@ import android.content.Context import android.content.res.XmlResourceParser +import android.util.Log import android.util.Xml import com.android.wallpaper.model.LiveWallpaperInfo import com.android.wallpaper.model.SystemStaticWallpaperInfo import com.android.wallpaper.model.WallpaperCategory import com.android.wallpaper.model.WallpaperInfo import com.android.wallpaper.module.PartnerProvider +import dagger.hilt.android.qualifiers.ApplicationContext +import java.io.IOException import javax.inject.Inject import javax.inject.Singleton import org.xmlpull.v1.XmlPullParser +import org.xmlpull.v1.XmlPullParserException /** * Utility class for parsing an XML file containing information about a list of wallpapers. The @@ -36,61 +40,91 @@ @Singleton class WallpaperXMLParser @Inject -constructor(private val context: Context, private val partnerProvider: PartnerProvider) : - WallpaperXMLParserInterface { +constructor( + @ApplicationContext private val context: Context, + private val partnerProvider: PartnerProvider +) : WallpaperXMLParserInterface { - override fun parseCategory(parser: XmlResourceParser): WallpaperCategory? { - val categoryBuilder = - WallpaperCategory.Builder(partnerProvider.resources, Xml.asAttributeSet(parser)) - categoryBuilder.setPriorityIfEmpty(PRIORITY_SYSTEM) - var publishedPlaceholder = false - val pair = parseXML(parser, parser.depth, categoryBuilder, false) - publishedPlaceholder = pair.first - val category = categoryBuilder.build() - return if (category.unmodifiableWallpapers.isNotEmpty()) category else null + /** This method is responsible for generating list of system categories from the XML file. */ + override fun parseSystemCategories(parser: XmlResourceParser): List<WallpaperCategory> { + val categories = mutableListOf<WallpaperCategory>() + try { + var priorityTracker = 0 + val depth = parser.depth + var type: Int + while ( + (parser.next().also { type = it } != XmlPullParser.END_TAG || + parser.depth > depth) && type != XmlPullParser.END_DOCUMENT + ) { + if (type == XmlPullParser.START_TAG && WallpaperCategory.TAG_NAME == parser.name) { + val categoryBuilder = + WallpaperCategory.Builder( + partnerProvider.resources, + Xml.asAttributeSet(parser) + ) + categoryBuilder.setPriorityIfEmpty(PRIORITY_SYSTEM + priorityTracker++) + categoryBuilder.addWallpapers( + parseXmlForWallpapersForASingleCategory(parser, categoryBuilder.id) + ) + val category = categoryBuilder.build() + category?.let { categories.add(it) } + } + } + } catch (e: Exception) { + when (e) { + is IOException, + is XmlPullParserException -> { + Log.w(TAG, "Failed to parse the XML file of system wallpapers", e) + return emptyList() + } + else -> throw e + } + } + return categories } - private fun parseXML( + /** + * This method is responsible for parsing the XML for a single category and returning a list of + * WallpaperInfo objects. + */ + private fun parseXmlForWallpapersForASingleCategory( parser: XmlResourceParser, - categoryDepth: Int, - categoryBuilder: WallpaperCategory.Builder, - publishedPlaceholder: Boolean - ): Pair<Boolean, Int> { - var type1 = parser.eventType - var publishedPlaceholder1 = publishedPlaceholder + categoryId: String + ): List<WallpaperInfo> { + val outputWallpaperInfo = mutableListOf<WallpaperInfo>() + val categoryDepth = parser.depth + var type: Int while ( - parser.next().also { type1 = it } != XmlPullParser.END_TAG || - parser.depth > categoryDepth + (parser.next().also { type = it } != XmlPullParser.END_TAG || + parser.depth > categoryDepth) && type != XmlPullParser.END_DOCUMENT ) { - if (type1 == XmlPullParser.START_TAG) { + if (type == XmlPullParser.START_TAG) { var wallpaper: WallpaperInfo? = null if (SystemStaticWallpaperInfo.TAG_NAME == parser.name) { wallpaper = SystemStaticWallpaperInfo.fromAttributeSet( partnerProvider.packageName, - categoryBuilder.id, + categoryId, Xml.asAttributeSet(parser) ) } else if (LiveWallpaperInfo.TAG_NAME == parser.name) { wallpaper = LiveWallpaperInfo.fromAttributeSet( context, - categoryBuilder.id, + categoryId, Xml.asAttributeSet(parser) ) } if (wallpaper != null) { - categoryBuilder.addWallpaper(wallpaper) - if (!publishedPlaceholder1) { - publishedPlaceholder1 = true - } + outputWallpaperInfo.add(wallpaper) } } } - return Pair(publishedPlaceholder1, type1) + return outputWallpaperInfo } companion object { - private const val PRIORITY_SYSTEM = 100 + const val PRIORITY_SYSTEM = 100 + private const val TAG = "WallpaperXMLParser" } }
diff --git a/src/com/android/wallpaper/util/WallpaperXMLParserInterface.kt b/src/com/android/wallpaper/util/WallpaperXMLParserInterface.kt index 13a85f5..ecb2531 100644 --- a/src/com/android/wallpaper/util/WallpaperXMLParserInterface.kt +++ b/src/com/android/wallpaper/util/WallpaperXMLParserInterface.kt
@@ -17,8 +17,8 @@ package com.android.wallpaper.util import android.content.res.XmlResourceParser -import com.android.wallpaper.model.WallpaperCategory +import com.android.wallpaper.model.Category interface WallpaperXMLParserInterface { - fun parseCategory(parser: XmlResourceParser): WallpaperCategory? + fun parseSystemCategories(parser: XmlResourceParser): List<Category> }
diff --git a/tests/common/Android.bp b/tests/common/Android.bp index 94c011c..7da2718 100644 --- a/tests/common/Android.bp +++ b/tests/common/Android.bp
@@ -28,6 +28,7 @@ "src/**/*.java", "src/**/*.kt", ], + resource_dirs: ["res"], static_libs: [ "WallpaperPicker2Lib", "androidx.annotation_annotation",
diff --git a/tests/common/res/xml/exception_wallpapers.xml b/tests/common/res/xml/exception_wallpapers.xml new file mode 100644 index 0000000..4c0beb4 --- /dev/null +++ b/tests/common/res/xml/exception_wallpapers.xml
@@ -0,0 +1,22 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- + ~ Copyright (C) 2024 The Android Open Source Project + ~ + ~ Licensed under the Apache License, Version 2.0 (the "License"); + ~ you may not use this file except in compliance with the License. + ~ You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, software + ~ distributed under the License is distributed on an "AS IS" BASIS, + ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + ~ See the License for the specific language governing permissions and + ~ limitations under the License. + --> + +<wallpapers> + <category title="Category 1"> <!-- Missing 'id' attribute --> + <static-wallpaper id="wallpaper1" src="wallpaper1.jpg" /> + </category> +</wallpapers> \ No newline at end of file
diff --git a/tests/common/res/xml/invalid_wallpapers.xml b/tests/common/res/xml/invalid_wallpapers.xml new file mode 100644 index 0000000..4021aff --- /dev/null +++ b/tests/common/res/xml/invalid_wallpapers.xml
@@ -0,0 +1,22 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- + ~ Copyright (C) 2024 The Android Open Source Project + ~ + ~ Licensed under the Apache License, Version 2.0 (the "License"); + ~ you may not use this file except in compliance with the License. + ~ You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, software + ~ distributed under the License is distributed on an "AS IS" BASIS, + ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + ~ See the License for the specific language governing permissions and + ~ limitations under the License. + --> + +<wallpapers> + <invalid-tag> <!-- Invalid tag --> + <static-wallpaper id="wallpaper2" src="wallpaper2.jpg" /> + </invalid-tag> +</wallpapers>
diff --git a/tests/common/res/xml/wallpapers.xml b/tests/common/res/xml/wallpapers.xml new file mode 100644 index 0000000..982749c --- /dev/null +++ b/tests/common/res/xml/wallpapers.xml
@@ -0,0 +1,23 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- + ~ Copyright (C) 2024 The Android Open Source Project + ~ + ~ Licensed under the Apache License, Version 2.0 (the "License"); + ~ you may not use this file except in compliance with the License. + ~ You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, software + ~ distributed under the License is distributed on an "AS IS" BASIS, + ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + ~ See the License for the specific language governing permissions and + ~ limitations under the License. + --> + +<wallpapers> + <category id="category1" title="Category 1"> + <static-wallpaper id="wallpaper1" src="wallpaper1.jpg" /> + <static-wallpaper id="wallpaper2" src="wallpaper2.jpg" /> + </category> +</wallpapers> \ No newline at end of file
diff --git a/tests/common/src/com/android/wallpaper/testing/FakeWallpaperXMLParser.kt b/tests/common/src/com/android/wallpaper/testing/FakeWallpaperXMLParser.kt index acbee88..8ab5748 100644 --- a/tests/common/src/com/android/wallpaper/testing/FakeWallpaperXMLParser.kt +++ b/tests/common/src/com/android/wallpaper/testing/FakeWallpaperXMLParser.kt
@@ -17,19 +17,22 @@ package com.android.wallpaper.testing import android.content.res.XmlResourceParser +import com.android.wallpaper.model.Category import com.android.wallpaper.model.SystemStaticWallpaperInfo import com.android.wallpaper.model.WallpaperCategory import com.android.wallpaper.util.WallpaperXMLParserInterface +import javax.inject.Inject import javax.inject.Singleton @Singleton -class FakeWallpaperXMLParser : WallpaperXMLParserInterface { - override fun parseCategory(parser: XmlResourceParser): WallpaperCategory { - // Return a hardcoded WallpaperCategory for testing +class FakeWallpaperXMLParser @Inject constructor() : WallpaperXMLParserInterface { + override fun parseSystemCategories(parser: XmlResourceParser): List<Category> { val wallpapers = listOf(fakeWallpaper) - - return WallpaperCategory("Fake Category Title", "Fake CollectionID", 1, wallpapers, 1) + return listOf( + WallpaperCategory("Fake Category Title", "Fake CollectionID", 1, wallpapers, 1) + ) } + companion object FakeWallpaperData { val fakeWallpaper = SystemStaticWallpaperInfo(
diff --git a/tests/common/src/com/android/wallpaper/testing/TestPartnerProvider.java b/tests/common/src/com/android/wallpaper/testing/TestPartnerProvider.java index 8a1333f..d6e6e19 100644 --- a/tests/common/src/com/android/wallpaper/testing/TestPartnerProvider.java +++ b/tests/common/src/com/android/wallpaper/testing/TestPartnerProvider.java
@@ -30,6 +30,8 @@ @Singleton public class TestPartnerProvider implements PartnerProvider { private File mLegacyWallpaperDirectory; + private String mPackageName; + private Resources mResources; @Inject public TestPartnerProvider() { @@ -37,9 +39,15 @@ @Override public Resources getResources() { - return null; + return mResources; } + public void setPackageName(String packageName) { + this.mPackageName = packageName; + } + public void setResources(Resources mResources) { + this.mResources = mResources; + } @Override public File getLegacyWallpaperDirectory() { return mLegacyWallpaperDirectory; @@ -56,7 +64,7 @@ @Override public String getPackageName() { - return null; + return mPackageName; } @Override
diff --git a/tests/robotests/src/com/android/wallpaper/util/WallpaperXMLParserTest.kt b/tests/robotests/src/com/android/wallpaper/util/WallpaperXMLParserTest.kt new file mode 100644 index 0000000..8a5f70e --- /dev/null +++ b/tests/robotests/src/com/android/wallpaper/util/WallpaperXMLParserTest.kt
@@ -0,0 +1,128 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.wallpaper.util + +import android.content.Context +import android.content.res.XmlResourceParser +import androidx.annotation.XmlRes +import androidx.test.core.app.ApplicationProvider +import com.android.wallpaper.module.PartnerProvider +import com.android.wallpaper.testing.TestPartnerProvider +import com.google.common.truth.Truth.assertThat +import dagger.hilt.android.testing.HiltAndroidRule +import dagger.hilt.android.testing.HiltAndroidTest +import dagger.hilt.android.testing.HiltTestApplication +import javax.inject.Inject +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.setMain +import org.junit.Assert.assertThrows +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.robolectric.shadows.ShadowDisplayManager + +@HiltAndroidTest +@RunWith(RobolectricTestRunner::class) +@Config(shadows = [ShadowDisplayManager::class]) +class WallpaperXMLParserTest { + + @get:Rule var hiltRule = HiltAndroidRule(this) + var context: Context = ApplicationProvider.getApplicationContext<HiltTestApplication>() + @Inject lateinit var partnerProvider: TestPartnerProvider + @Inject lateinit var wallpaperXMLParser: WallpaperXMLParser + private val testDispatcher: CoroutineDispatcher = StandardTestDispatcher() + val testScope = TestScope(testDispatcher) + + @Before + fun setup() { + hiltRule.inject() + Dispatchers.setMain(testDispatcher) + wallpaperXMLParser = WallpaperXMLParser(context, partnerProvider) + } + + /** + * This test uses the file wallpapers.xml that is defined in the resources folder to make sure + * that we parse categories correctly. + */ + @Test + fun parseXMLForSystemCategories_shouldReturnCategories() { + val resources = context.resources + partnerProvider.resources = resources + val packageName = context.packageName + partnerProvider.packageName = packageName + @XmlRes + val wallpapersResId: Int = + resources.getIdentifier(PartnerProvider.WALLPAPER_RES_ID, "xml", packageName) + assertThat(wallpapersResId).isNotEqualTo(0) + val parser: XmlResourceParser = resources.getXml(wallpapersResId) + + val categories = wallpaperXMLParser.parseSystemCategories(parser) + + assertThat(categories).hasSize(1) + assertThat(categories[0].collectionId).isEqualTo("category1") + } + + /** + * This test uses the file invalid_wallpapers.xml that is defined in the resources folder where + * if incorrect tags are defined, we return empty categories. + */ + @Test + fun parseInvalidXMLForSystemCategories_shouldReturnEmptyCategories() { + val resources = context.resources + partnerProvider.resources = resources + val packageName = context.packageName + partnerProvider.packageName = packageName + @XmlRes + val wallpapersResId: Int = resources.getIdentifier("invalid_wallpapers", "xml", packageName) + assertThat(wallpapersResId).isNotEqualTo(0) + val parser: XmlResourceParser = resources.getXml(wallpapersResId) + + val categories = wallpaperXMLParser.parseSystemCategories(parser) + + assertThat(categories).hasSize(0) + } + + /** + * This test uses the file exception_wallpapers.xml that is defined in the resources folder + * where if some mandatory attributes aren't defined, an exception will be thrown. + */ + @Test + fun parseInvalidXMLForSystemCategories_shouldThrowException() { + val resources = context.resources + partnerProvider.resources = resources + val packageName = context.packageName + partnerProvider.packageName = packageName + @XmlRes + val wallpapersResId: Int = + resources.getIdentifier("exception_wallpapers", "xml", packageName) + assertThat(wallpapersResId).isNotEqualTo(0) + val parser: XmlResourceParser = resources.getXml(wallpapersResId) + + assertThat( + assertThrows(NullPointerException::class.java) { + wallpaperXMLParser.parseSystemCategories(parser) + } + ) + .isNotNull() + } +}