diff --git a/app-sizer/src/main/kotlin/com/grab/sizer/analyzer/TeamAnalyzer.kt b/app-sizer/src/main/kotlin/com/grab/sizer/analyzer/TeamAnalyzer.kt index cbb5da4..78bc1dd 100644 --- a/app-sizer/src/main/kotlin/com/grab/sizer/analyzer/TeamAnalyzer.kt +++ b/app-sizer/src/main/kotlin/com/grab/sizer/analyzer/TeamAnalyzer.kt @@ -69,7 +69,7 @@ internal class TeamAnalyzer @Inject constructor( resources = wholeProject.noOwnerResources.castToRawFile(), nativeLibs = wholeProject.noOwnerNativeLibs.castToRawFile(), classes = wholeProject.noOwnerClasses.castToClass(), - //others = wholeProject.noOwnerOthers.castToRawFile() + others = wholeProject.noOwnerOthers.castToRawFile() ) // Process all contributors together (modules + libraries) for team analysis @@ -80,12 +80,29 @@ internal class TeamAnalyzer @Inject constructor( dataParser.getJars() ) - return generateTeamReport(allContributorsData.contributors + appContributor) + val moduleContributorPaths = + (dataParser.moduleAars + dataParser.moduleJars).map { it.path }.toSet() + appContributor.path + val libraryContributorPaths = (dataParser.libAars + dataParser.libJars).map { it.path }.toSet() + + return generateTeamReport( + contributors = allContributorsData.contributors + appContributor, + moduleContributorPaths = moduleContributorPaths, + libraryContributorPaths = libraryContributorPaths + ) } - private fun generateTeamReport(contributors: Set): Report { + private fun generateTeamReport( + contributors: Set, + moduleContributorPaths: Set, + libraryContributorPaths: Set + ): Report { // Convert all contributors to teams (handles both modules and libraries) - val teams: List = contributors.toTeams(teamMapping) + val teams: List = contributors.toTeams( + teamMapping = teamMapping, + includeUnowned = true, + moduleContributorPaths = moduleContributorPaths, + libraryContributorPaths = libraryContributorPaths + ) val allTeamRows = teams.sortedBy { it.getDownloadSize() } .flatMap { it.toDetailedReportRows() } @@ -162,4 +179,4 @@ internal fun createAppInfo() = object : BinaryFileInfo { override val name: String = "app" override val path: String = "root/app/build/" override val tag: String = "app" -} \ No newline at end of file +} diff --git a/app-sizer/src/main/kotlin/com/grab/sizer/analyzer/mapper/OtherComponentMapper.kt b/app-sizer/src/main/kotlin/com/grab/sizer/analyzer/mapper/OtherComponentMapper.kt index e271ee8..e846495 100644 --- a/app-sizer/src/main/kotlin/com/grab/sizer/analyzer/mapper/OtherComponentMapper.kt +++ b/app-sizer/src/main/kotlin/com/grab/sizer/analyzer/mapper/OtherComponentMapper.kt @@ -35,10 +35,14 @@ import javax.inject.Inject internal class OtherComponentMapper @Inject constructor() : ComponentMapper { override fun Set.mapTo(aars: Set, jars: Set): ComponentMapperResult { - // Todo: Add logic to map others return ComponentMapperResult( contributors = emptyMap(), - noOwnerData = flatMap { it.others }.toSet() + noOwnerData = flatMap { apk -> + apk.others.map { other -> + // Split APKs can have the same internal path, so include the APK name before storing in a Set. + other.copy(path = "${apk.name}:${other.path}") + } + }.toSet() ) } -} \ No newline at end of file +} diff --git a/app-sizer/src/main/kotlin/com/grab/sizer/analyzer/model/Team.kt b/app-sizer/src/main/kotlin/com/grab/sizer/analyzer/model/Team.kt index e69bcb0..6730cb9 100644 --- a/app-sizer/src/main/kotlin/com/grab/sizer/analyzer/model/Team.kt +++ b/app-sizer/src/main/kotlin/com/grab/sizer/analyzer/model/Team.kt @@ -29,6 +29,7 @@ package com.grab.sizer.analyzer.model import com.grab.sizer.analyzer.ReportItem import com.grab.sizer.analyzer.TeamMapping +import com.grab.sizer.analyzer.NOT_AVAILABLE_VALUE import com.grab.sizer.parser.BinaryFileInfo data class Team( @@ -76,32 +77,58 @@ data class Module( resourcesDownloadSize + nativeLibDownloadSize + assetsDownloadSize + othersDownloadSize + classDownloadSize } -internal fun Set.toTeams(teamMapping: TeamMapping?): List { - return teamMapping?.let { mapping -> - // Single-pass grouping to avoid O(N×T) complexity - val moduleContributorsByTeam = mutableMapOf>() - val libContributorsByTeam = mutableMapOf>() +internal fun Set.toTeams( + teamMapping: TeamMapping?, + includeUnowned: Boolean = false, + moduleContributorPaths: Set = emptySet(), + libraryContributorPaths: Set = emptySet() +): List { + if (teamMapping == null && !includeUnowned) return emptyList() - // Group contributors by team in single pass - this.forEach { contributor -> - mapping.getModuleOwner(contributor.tag)?.let { teamName -> - moduleContributorsByTeam.getOrPut(teamName) { mutableListOf() }.add(contributor) + val moduleContributorsByTeam = mutableMapOf>() + val libContributorsByTeam = mutableMapOf>() + + fun MutableMap>.addContributor(teamName: String, contributor: Contributor) { + getOrPut(teamName) { mutableListOf() }.add(contributor) + } + + this.forEach { contributor -> + val isModuleContributor = contributor.path in moduleContributorPaths + val isLibraryContributor = contributor.path in libraryContributorPaths + + when { + isModuleContributor -> { + val teamName = teamMapping?.getModuleOwner(contributor.tag) + ?: NOT_AVAILABLE_VALUE.takeIf { includeUnowned } + teamName?.let { moduleContributorsByTeam.addContributor(it, contributor) } } - mapping.getLibraryOwner(contributor.tag)?.let { teamName -> - libContributorsByTeam.getOrPut(teamName) { mutableListOf() }.add(contributor) + + isLibraryContributor -> { + val teamName = teamMapping?.getLibraryOwner(contributor.tag) + ?: NOT_AVAILABLE_VALUE.takeIf { includeUnowned } + teamName?.let { libContributorsByTeam.addContributor(it, contributor) } } - } - // Create teams from grouped contributors - val allTeamNames = moduleContributorsByTeam.keys + libContributorsByTeam.keys - allTeamNames.map { teamName -> - Team( - teamName, - moduleContributorsByTeam[teamName] ?: emptyList(), - libContributorsByTeam[teamName] ?: emptyList() - ) + else -> { + val moduleOwner = teamMapping?.getModuleOwner(contributor.tag) + val libraryOwner = teamMapping?.getLibraryOwner(contributor.tag) + moduleOwner?.let { moduleContributorsByTeam.addContributor(it, contributor) } + libraryOwner?.let { libContributorsByTeam.addContributor(it, contributor) } + if (moduleOwner == null && libraryOwner == null && includeUnowned) { + moduleContributorsByTeam.addContributor(NOT_AVAILABLE_VALUE, contributor) + } + } } - } ?: emptyList() + } + + val allTeamNames = moduleContributorsByTeam.keys + libContributorsByTeam.keys + return allTeamNames.map { teamName -> + Team( + teamName, + moduleContributorsByTeam[teamName] ?: emptyList(), + libContributorsByTeam[teamName] ?: emptyList() + ) + } } internal fun List.sort(): List { @@ -138,4 +165,3 @@ internal fun Module.toReportItem(teamMapping: TeamMapping?): ReportItem = assetDownloadSize = assetsDownloadSize, otherDownloadSize = othersDownloadSize ) - diff --git a/app-sizer/src/test/kotlin/com/grab/sizer/analyzer/TeamAnalyzerTest.kt b/app-sizer/src/test/kotlin/com/grab/sizer/analyzer/TeamAnalyzerTest.kt index e655f5d..9652fbe 100644 --- a/app-sizer/src/test/kotlin/com/grab/sizer/analyzer/TeamAnalyzerTest.kt +++ b/app-sizer/src/test/kotlin/com/grab/sizer/analyzer/TeamAnalyzerTest.kt @@ -51,7 +51,7 @@ class TeamAnalyzerTest { // Verify report structure assertEquals("team", report.id) assertEquals("team", report.name) - assertEquals(16, report.rows.size, "Should have 8 breakdown rows per team * 2 teams = 16 total") + assertEquals(24, report.rows.size, "Should have 8 breakdown rows per team * 3 teams = 24 total") // Verify team1 total size (modules: 103 + libraries: libAar1=15 + libJar=7 = 125 total) val team1TotalRow = report.rows.find { row -> @@ -79,20 +79,26 @@ class TeamAnalyzerTest { ownerField?.value == "team2" } assertEquals(8, team2Rows.size, "team2 should have 8 breakdown rows") + + val unownedRows = report.rows.filter { row -> + val ownerField = row.fields.find { it.name == FIELD_KEY_OWNER } + ownerField?.value == NOT_AVAILABLE_VALUE + } + assertEquals(8, unownedRows.size, "unowned contributors should have 8 breakdown rows") } @Test fun testTeamAnalyzerShouldReportCorrectNumberOfRows() { val report = analyzer.process() // Each team should have 8 breakdown rows: total, android-java-libraries, native-libraries, codebase-kotlin-java, codebase-resources, codebase-assets, codebase-native, others - assertEquals(16, report.rows.size, "Project1 report should contain 8 rows per team (2 teams * 8 = 16)") + assertEquals(24, report.rows.size, "Project1 report should contain 8 rows per team (3 teams * 8 = 24)") } @Test fun testTeamAnalyzerShouldReportCorrectRowNames() { val report = analyzer.process() val teamNames = report.rows.map { it.name }.toSet() - val expectedTeamNames = setOf("team1", "team2") + val expectedTeamNames = setOf("team1", "team2", NOT_AVAILABLE_VALUE) assertEquals(expectedTeamNames, teamNames, "Should report the correct team names") // Check that we have the correct tags for each team @@ -113,6 +119,22 @@ class TeamAnalyzerTest { assertEquals(125L, team1TotalRow.fields.find { it.name == FIELD_KEY_SIZE }?.value) } + @Test + fun testTeamAnalyzerShouldReportUnownedApkOthers() { + val report = analyzer.process() + val unownedTotalRow = report.rows.find { row -> + row.name == NOT_AVAILABLE_VALUE && row.fields.find { it.name == FIELD_KEY_CONTRIBUTOR }?.value == "total" + } + assertNotNull(unownedTotalRow, "Project1 report should contain unowned total size row") + assertEquals(20L, unownedTotalRow.fields.find { it.name == FIELD_KEY_SIZE }?.value) + + val unownedOthersRow = report.rows.find { row -> + row.name == NOT_AVAILABLE_VALUE && row.fields.find { it.name == FIELD_KEY_CONTRIBUTOR }?.value == "others" + } + assertNotNull(unownedOthersRow, "Project1 report should contain unowned others size row") + assertEquals(20L, unownedOthersRow.fields.find { it.name == FIELD_KEY_SIZE }?.value) + } + @Test fun testTeamAnalyzerShouldReportCorrectTeam2Size() { val report = analyzer.process() @@ -152,8 +174,8 @@ class TeamAnalyzerTest { val report = analyzerWithLibs.process() - // Verify correct number of rows: 2 teams * 8 breakdown rows each = 16 total - assertEquals(16, report.rows.size) + // Verify correct number of rows: 3 teams * 8 breakdown rows each = 24 total + assertEquals(24, report.rows.size) // Get actual sizes for validation from the total rows val team1TotalRow = report.rows.find { row -> @@ -189,6 +211,19 @@ class TeamAnalyzerTest { // Verify the changes are as expected assertEquals(-7L, team1Size - originalTeam1Size, "team1 should lose libJar1 (7 bytes)") assertEquals(0L, team2Size - originalTeam2Size, "team2 should have same library contribution") + + val unownedTotalRow = report.rows.find { row -> + row.name == NOT_AVAILABLE_VALUE && row.fields.find { it.name == FIELD_KEY_CONTRIBUTOR }?.value == "total" + } + assertNotNull(unownedTotalRow, "Should contain unowned total row") + assertEquals(27L, unownedTotalRow.fields.find { it.name == FIELD_KEY_SIZE }?.value) + + val unownedLibrariesRow = report.rows.find { row -> + row.name == NOT_AVAILABLE_VALUE && + row.fields.find { it.name == FIELD_KEY_CONTRIBUTOR }?.value == "android-java-libraries" + } + assertNotNull(unownedLibrariesRow, "Should contain unowned library contribution row") + assertEquals(7L, unownedLibrariesRow.fields.find { it.name == FIELD_KEY_SIZE }?.value) } @Test @@ -203,14 +238,14 @@ class TeamAnalyzerTest { val report = analyzerLibraryOnly.process() - // Should have 3 teams * 8 breakdown rows each = 24 total rows - assertEquals(24, report.rows.size, "Should include all teams including library-only teams") + // Should have 4 teams * 8 breakdown rows each = 32 total rows + assertEquals(32, report.rows.size, "Should include all teams including library-only and unowned teams") val totalRowNames = report.rows.filter { row -> row.fields.find { it.name == FIELD_KEY_CONTRIBUTOR }?.value == "total" }.map { it.name }.toSet() - val expectedTotalRows = setOf("team1", "team2", "libraryTeam") - assertEquals(expectedTotalRows, totalRowNames, "Should include total rows for all teams including library-only team") + val expectedTotalRows = setOf("team1", "team2", "libraryTeam", NOT_AVAILABLE_VALUE) + assertEquals(expectedTotalRows, totalRowNames, "Should include total rows for all teams including library-only and unowned teams") // Verify library-only team has correct size val libraryOnlyTotalRow = report.rows.find { row -> @@ -235,7 +270,7 @@ class TeamAnalyzerTest { val report = project2Analyzer.process() // Verify basic structure (same as Project1 but with different paths) - assertEquals(16, report.rows.size, "Should have 8 breakdown rows per team * 2 teams = 16 total") + assertEquals(24, report.rows.size, "Should have 8 breakdown rows per team * 3 teams = 24 total") // Verify team total sizes are the same as Project1 (since data should be equivalent including libraries) val team1TotalRow = report.rows.find { row -> @@ -326,4 +361,4 @@ class Project2Data : Project1Data() { get() = super.moduleJar1.copy(path = "jar/moduleJar1.jar") override val moduleJar2: JarFileInfo get() = super.moduleJar2.copy(path = "jar/moduleJar2.jar") -} \ No newline at end of file +} diff --git a/app-sizer/src/test/kotlin/com/grab/sizer/analyzer/mapper/OtherComponentMapperTest.kt b/app-sizer/src/test/kotlin/com/grab/sizer/analyzer/mapper/OtherComponentMapperTest.kt new file mode 100644 index 0000000..9a5f893 --- /dev/null +++ b/app-sizer/src/test/kotlin/com/grab/sizer/analyzer/mapper/OtherComponentMapperTest.kt @@ -0,0 +1,52 @@ +/* + * MIT License + * + * Copyright (c) 2024. Grabtaxi Holdings Pte Ltd (GRAB), All rights reserved. + * + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE + */ + +package com.grab.sizer.analyzer.mapper + +import com.grab.sizer.analyzer.createRawFileInfo +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +class OtherComponentMapperTest { + @Test + fun otherAnalyzerShouldPreserveSamePathOthersFromDifferentSplitApks() { + val baseOther = createRawFileInfo(path = "/resources.arsc", downloadSize = 10, size = 100) + val configOther = createRawFileInfo(path = "/resources.arsc", downloadSize = 20, size = 200) + val baseApk = createEmptyApkInfo("base-master.apk").copy(others = setOf(baseOther)) + val configApk = createEmptyApkInfo("base-en.apk").copy(others = setOf(configOther)) + + val result = OtherComponentMapper().run { + setOf(baseApk, configApk).mapTo(emptySet(), emptySet()) + } + + assertEquals(2, result.noOwnerData.size) + assertEquals(30, result.noOwnerData.sumOf { it.downloadSize }) + assertTrue(result.noOwnerData.any { it.name == "resources.arsc" && it.downloadSize == 10L }) + assertTrue(result.noOwnerData.any { it.name == "resources.arsc" && it.downloadSize == 20L }) + } +} diff --git a/docs/limitation.md b/docs/limitation.md index f532ddb..566bd3c 100644 --- a/docs/limitation.md +++ b/docs/limitation.md @@ -26,8 +26,8 @@ The `resources.arsc` file is a special file in Android APKs containing precompil - For small Android projects, this can disproportionately impact the data, potentially creating the illusion of an inefficient analysis. ### Uncategorized Files -* Any files that cannot be categorized as Java/Kotlin code, resources, native libraries, or assets are automatically distributed to the app module and grouped under the **"Others"** category. -* Any files/classes that cannot find an owner — i.e. files that don't belong to a module owned in `module-owner.yml` and libraries that don't match any pattern in `library-owner.yml` — are automatically distributed to the `app` module. +* APK-level files that cannot be categorized as Java/Kotlin code, resources, native libraries, or assets are attributed to the synthetic `app` module and grouped under the **"Others"** category. The owner of these files is resolved through the `app` entry in `module-owner.yml`. +* Modules and libraries that are present in the analyzed inputs but do not match `module-owner.yml` or `library-owner.yml` are reported as `NA` in team reports. They are not silently reassigned to the `app` module. ## Inline Functions and Classes @@ -50,4 +50,4 @@ Users should keep these limitations in mind when making decisions based on App S [class_size]: https://github.com/JesusFreke/smali/blob/master/dexlib2/src/main/java/org/jf/dexlib2/dexbacked/DexBackedClassDef.java#L505 [inline_functions]: https://kotlinlang.org/docs/inline-functions.html -[inline_class]: https://kotlinlang.org/docs/inline-classes.html \ No newline at end of file +[inline_class]: https://kotlinlang.org/docs/inline-classes.html diff --git a/docs/plugin.md b/docs/plugin.md index 8a4a8a5..6499fe5 100644 --- a/docs/plugin.md +++ b/docs/plugin.md @@ -294,7 +294,7 @@ java.lang.IllegalStateException: Cannot find matching variant for module-name #### Impact on Analysis Results -All classes and resources belonging to skipped modules will be automatically attributed to the app module during analysis, which may impact the accuracy of module-wise and team-based size breakdowns. +Classes and resources belonging to skipped modules may not have module-level ownership metadata available during analysis. In team reports, contributors that cannot be matched to `module-owner.yml` or `library-owner.yml` are reported as `NA` instead of being silently reassigned to the `app` module. ## Troubleshooting diff --git a/sample/README.md b/sample/README.md index 49d3194..38414a8 100644 --- a/sample/README.md +++ b/sample/README.md @@ -108,10 +108,14 @@ Team2: - com.google.guava:* ``` +### Unowned Contributors + +APK-level uncategorized files are grouped under `others` and attributed to the synthetic `app` module, so the `app` entry in `module-owner.yml` determines their team owner. Modules or libraries that do not match either owner file are reported as `NA` in team reports. + ## Additional Resources - [App Sizer Documentation](../docs/index.md) - [Configuring the Gradle Plugin](../docs/plugin.md) - [Understanding the Reports](../docs/report.md) -- [App Sizer Limitations](../docs/limitation.md) \ No newline at end of file +- [App Sizer Limitations](../docs/limitation.md)