From dbbb8374a2c2dea5093041f77579c3994473515a Mon Sep 17 00:00:00 2001
From: Paul Duffin <paulduffin@google.com>
Date: Fri, 1 Apr 2022 15:04:23 +0100
Subject: [PATCH] signature_trie: Avoid unnecessary wrapping and unwrapping of
 values

Previously, Leaf.values() and Leaf.append_values() would wrap the
Leaf's value inside a list before appending it to the list of values.
So, the values list was actually a list of lists of values. The
get_matching_rows method would then use chain.from_iterable() to
flatten that list of list of values into a list of values.

This change removes the initial wrapping in a list and so removes the
need to flatten them into a single list. It also adds a test for the
values() method. Prior to this change the expected value would have
been [[1], ["A"], [{}]].

Bug: 202154151
Test: atest --host analyze_bcpf_test signature_trie_test verify_overlaps_test
Change-Id: Ida78500c9ab4466de127b2c36501b3606d0f3fe5
---
 scripts/hiddenapi/signature_trie.py      | 15 +++++----------
 scripts/hiddenapi/signature_trie_test.py | 21 +++++++++++++++++++++
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/scripts/hiddenapi/signature_trie.py b/scripts/hiddenapi/signature_trie.py
index e813a9781..3650fa159 100644
--- a/scripts/hiddenapi/signature_trie.py
+++ b/scripts/hiddenapi/signature_trie.py
@@ -45,7 +45,9 @@ class Node:
         :return: A list of iterables of all the values associated with
             this node and its children.
         """
-        raise NotImplementedError("Please Implement this method")
+        values = []
+        self.append_values(values, selector)
+        return values
 
     def append_values(self, values, selector):
         """Append the values associated with this node and its children.
@@ -313,12 +315,8 @@ class InteriorNode(Node):
                 node = node.nodes[element]
             else:
                 return []
-        return chain.from_iterable(node.values(selector))
 
-    def values(self, selector):
-        values = []
-        self.append_values(values, selector)
-        return values
+        return node.values(selector)
 
     def append_values(self, values, selector):
         for key, node in self.nodes.items():
@@ -336,11 +334,8 @@ class Leaf(Node):
     # The value associated with this leaf.
     value: typing.Any
 
-    def values(self, selector):
-        return [[self.value]]
-
     def append_values(self, values, selector):
-        values.append([self.value])
+        values.append(self.value)
 
     def child_nodes(self):
         return []
diff --git a/scripts/hiddenapi/signature_trie_test.py b/scripts/hiddenapi/signature_trie_test.py
index 129569107..6d4e660e3 100755
--- a/scripts/hiddenapi/signature_trie_test.py
+++ b/scripts/hiddenapi/signature_trie_test.py
@@ -150,6 +150,27 @@ class TestSignatureToElements(unittest.TestCase):
             str(context.exception))
 
 
+class TestValues(unittest.TestCase):
+    def test_add_then_get(self):
+        trie = signature_trie()
+        trie.add("La/b/C;->l()", 1)
+        trie.add("La/b/C$D;->m()", "A")
+        trie.add("La/b/C$D;->n()", {})
+
+        package_a_node = next(iter(trie.child_nodes()))
+        self.assertEqual("package", package_a_node.type)
+        self.assertEqual("a", package_a_node.selector)
+
+        package_b_node = next(iter(package_a_node.child_nodes()))
+        self.assertEqual("package", package_b_node.type)
+        self.assertEqual("a/b", package_b_node.selector)
+
+        class_c_node = next(iter(package_b_node.child_nodes()))
+        self.assertEqual("class", class_c_node.type)
+        self.assertEqual("a/b/C", class_c_node.selector)
+
+        self.assertEqual([1, "A", {}], class_c_node.values(lambda _: True))
+
 class TestGetMatchingRows(unittest.TestCase):
     extractInput = """
 Ljava/lang/Character$UnicodeScript;->of(I)Ljava/lang/Character$UnicodeScript;