Fix inefficiency in the setCustomKeys Kotlin extension #6536

mrober · 2024-11-25T21:39:24Z

Fixed inefficiency in the Kotlin FirebaseCrashlytics.setCustomKeys extension.

The old implementation called setCustomKey for every key/value pair, which serializes to disk, and is not an intuitive behaviour for customers. This now calls setCustomKeys once with the entire built map.

This proxies to the existing Java CustomKeysAndValues.Builder but gives nice Kotlin syntax. The old class had a public ctor, I plan to make this internal since customers shouldn't instantiate this class directly. This is done in a way to not break the api, even though the api is internal.

pair=tejasd

github-actions · 2024-11-25T21:39:37Z

Javadoc Changes:

--- /home/runner/diff/original/firebase-kotlindoc/android/com/google/firebase/crashlytics/KeyValueBuilder.html	2024-11-28 21:25:35.766509165 +0000
+++ /home/runner/diff/modified/firebase-kotlindoc/android/com/google/firebase/crashlytics/KeyValueBuilder.html	2024-11-28 21:23:08.959620483 +0000
@@ -11,7 +11,7 @@
       <pre>public final class <a href="/docs/reference/android/com/google/firebase/crashlytics/KeyValueBuilder.html">KeyValueBuilder</a></pre>
     </p>
     <hr>
-    <p>Helper class to enable fluent syntax in <code><a href="/docs/reference/android/com/google/firebase/crashlytics/package-summary.html#(com.google.firebase.crashlytics.FirebaseCrashlytics).setCustomKeys(kotlin.Function1)">setCustomKeys</a></code></p>
+    <p>Helper class to enable convenient syntax in <code><a href="/docs/reference/android/com/google/firebase/crashlytics/package-summary.html#(com.google.firebase.crashlytics.FirebaseCrashlytics).setCustomKeys(kotlin.Function1)">setCustomKeys</a></code></p>
     <h2>Summary</h2>
     <div class="devsite-table-wrapper">
       <table class="responsive">
@@ -27,7 +27,8 @@
         <tbody class="list">
           <tr>
             <td>
-              <div><code><a href="/docs/reference/android/com/google/firebase/crashlytics/KeyValueBuilder.html#KeyValueBuilder(com.google.firebase.crashlytics.FirebaseCrashlytics)">KeyValueBuilder</a>(@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="/docs/reference/android/com/google/firebase/crashlytics/FirebaseCrashlytics.html">FirebaseCrashlytics</a>&nbsp;crashlytics)</code></div>
+              <div><code><span><del><a href="/docs/reference/android/com/google/firebase/crashlytics/KeyValueBuilder.html#KeyValueBuilder(com.google.firebase.crashlytics.FirebaseCrashlytics)">KeyValueBuilder</a></del></span>(@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="/docs/reference/android/com/google/firebase/crashlytics/FirebaseCrashlytics.html">FirebaseCrashlytics</a>&nbsp;crashlytics)</code></div>
+              <p><strong>This method is deprecated.</strong> Do not construct this directly.</p>
             </td>
           </tr>
         </tbody>
@@ -94,7 +95,8 @@
       <h2>Public constructors</h2>
       <div class="api-item"><a name="KeyValueBuilder-com.google.firebase.crashlytics.FirebaseCrashlytics-"></a><a name="keyvaluebuilder"></a>
         <h3 class="api-name" id="KeyValueBuilder(com.google.firebase.crashlytics.FirebaseCrashlytics)">KeyValueBuilder</h3>
-        <pre class="api-signature no-pretty-print">public&nbsp;<a href="/docs/reference/android/com/google/firebase/crashlytics/KeyValueBuilder.html#KeyValueBuilder(com.google.firebase.crashlytics.FirebaseCrashlytics)">KeyValueBuilder</a>(@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="/docs/reference/android/com/google/firebase/crashlytics/FirebaseCrashlytics.html">FirebaseCrashlytics</a>&nbsp;crashlytics)</pre>
+        <pre class="api-signature no-pretty-print">public&nbsp;<span><del><a href="/docs/reference/android/com/google/firebase/crashlytics/KeyValueBuilder.html#KeyValueBuilder(com.google.firebase.crashlytics.FirebaseCrashlytics)">KeyValueBuilder</a></del></span>(@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="/docs/reference/android/com/google/firebase/crashlytics/FirebaseCrashlytics.html">FirebaseCrashlytics</a>&nbsp;crashlytics)</pre>
+        <aside class="caution"><strong>This method is deprecated.</strong><br>Do not construct this directly. Use `setCustomKeys` instead. To be removed in the next major release.</aside>
       </div>
     </div>
     <div class="list">

--- /home/runner/diff/original/firebase-kotlindoc/android/com/google/firebase/crashlytics/package-summary.html	2024-11-28 21:25:35.766509165 +0000
+++ /home/runner/diff/modified/firebase-kotlindoc/android/com/google/firebase/crashlytics/package-summary.html	2024-11-28 21:23:08.959620483 +0000
@@ -37,7 +37,7 @@
           <tr>
             <td><code><a href="/docs/reference/android/com/google/firebase/crashlytics/KeyValueBuilder.html">KeyValueBuilder</a></code></td>
             <td>
-              <p>Helper class to enable fluent syntax in <code><a href="/docs/reference/android/com/google/firebase/crashlytics/package-summary.html#(com.google.firebase.crashlytics.FirebaseCrashlytics).setCustomKeys(kotlin.Function1)">setCustomKeys</a></code></p>
+              <p>Helper class to enable convenient syntax in <code><a href="/docs/reference/android/com/google/firebase/crashlytics/package-summary.html#(com.google.firebase.crashlytics.FirebaseCrashlytics).setCustomKeys(kotlin.Function1)">setCustomKeys</a></code></p>
             </td>
           </tr>
         </tbody>

--- /home/runner/diff/original/firebase-kotlindoc/kotlin/com/google/firebase/crashlytics/KeyValueBuilder.html	2024-11-28 21:25:35.766509165 +0000
+++ /home/runner/diff/modified/firebase-kotlindoc/kotlin/com/google/firebase/crashlytics/KeyValueBuilder.html	2024-11-28 21:23:08.959620483 +0000
@@ -11,7 +11,7 @@
       <pre>class <a href="/docs/reference/kotlin/com/google/firebase/crashlytics/KeyValueBuilder.html">KeyValueBuilder</a></pre>
     </p>
     <hr>
-    <p>Helper class to enable fluent syntax in <code><a href="/docs/reference/kotlin/com/google/firebase/crashlytics/package-summary.html#(com.google.firebase.crashlytics.FirebaseCrashlytics).setCustomKeys(kotlin.Function1)">setCustomKeys</a></code></p>
+    <p>Helper class to enable convenient syntax in <code><a href="/docs/reference/kotlin/com/google/firebase/crashlytics/package-summary.html#(com.google.firebase.crashlytics.FirebaseCrashlytics).setCustomKeys(kotlin.Function1)">setCustomKeys</a></code></p>
     <h2>Summary</h2>
     <div class="devsite-table-wrapper">
       <table class="responsive">
@@ -27,7 +27,8 @@
         <tbody class="list">
           <tr>
             <td>
-              <div><code><a href="/docs/reference/kotlin/com/google/firebase/crashlytics/KeyValueBuilder.html#KeyValueBuilder(com.google.firebase.crashlytics.FirebaseCrashlytics)">KeyValueBuilder</a>(crashlytics:&nbsp;<a href="/docs/reference/kotlin/com/google/firebase/crashlytics/FirebaseCrashlytics.html">FirebaseCrashlytics</a>)</code></div>
+              <div><code><span><del><a href="/docs/reference/kotlin/com/google/firebase/crashlytics/KeyValueBuilder.html#KeyValueBuilder(com.google.firebase.crashlytics.FirebaseCrashlytics)">KeyValueBuilder</a></del></span>(crashlytics:&nbsp;<a href="/docs/reference/kotlin/com/google/firebase/crashlytics/FirebaseCrashlytics.html">FirebaseCrashlytics</a>)</code></div>
+              <p><strong>This function is deprecated.</strong> Do not construct this directly.</p>
             </td>
           </tr>
         </tbody>
@@ -94,7 +95,8 @@
       <h2>Public constructors</h2>
       <div class="api-item"><a name="KeyValueBuilder-com.google.firebase.crashlytics.FirebaseCrashlytics-"></a><a name="keyvaluebuilder"></a>
         <h3 class="api-name" id="KeyValueBuilder(com.google.firebase.crashlytics.FirebaseCrashlytics)">KeyValueBuilder</h3>
-        <pre class="api-signature no-pretty-print"><a href="/docs/reference/kotlin/com/google/firebase/crashlytics/KeyValueBuilder.html#KeyValueBuilder(com.google.firebase.crashlytics.FirebaseCrashlytics)">KeyValueBuilder</a>(crashlytics:&nbsp;<a href="/docs/reference/kotlin/com/google/firebase/crashlytics/FirebaseCrashlytics.html">FirebaseCrashlytics</a>)</pre>
+        <pre class="api-signature no-pretty-print"><span><del><a href="/docs/reference/kotlin/com/google/firebase/crashlytics/KeyValueBuilder.html#KeyValueBuilder(com.google.firebase.crashlytics.FirebaseCrashlytics)">KeyValueBuilder</a></del></span>(crashlytics:&nbsp;<a href="/docs/reference/kotlin/com/google/firebase/crashlytics/FirebaseCrashlytics.html">FirebaseCrashlytics</a>)</pre>
+        <aside class="caution"><strong>This function is deprecated.</strong><br>Do not construct this directly. Use `setCustomKeys` instead. To be removed in the next major release.</aside>
       </div>
     </div>
     <div class="list">

--- /home/runner/diff/original/firebase-kotlindoc/kotlin/com/google/firebase/crashlytics/package-summary.html	2024-11-28 21:25:35.766509165 +0000
+++ /home/runner/diff/modified/firebase-kotlindoc/kotlin/com/google/firebase/crashlytics/package-summary.html	2024-11-28 21:23:08.959620483 +0000
@@ -33,7 +33,7 @@
           <tr>
             <td><code><a href="/docs/reference/kotlin/com/google/firebase/crashlytics/KeyValueBuilder.html">KeyValueBuilder</a></code></td>
             <td>
-              <p>Helper class to enable fluent syntax in <code><a href="/docs/reference/kotlin/com/google/firebase/crashlytics/package-summary.html#(com.google.firebase.crashlytics.FirebaseCrashlytics).setCustomKeys(kotlin.Function1)">setCustomKeys</a></code></p>
+              <p>Helper class to enable convenient syntax in <code><a href="/docs/reference/kotlin/com/google/firebase/crashlytics/package-summary.html#(com.google.firebase.crashlytics.FirebaseCrashlytics).setCustomKeys(kotlin.Function1)">setCustomKeys</a></code></p>
             </td>
           </tr>
         </tbody>

github-actions · 2024-11-25T21:39:44Z

Vertex AI Mock Responses Check ⚠️

A newer major version of the mock responses for Vertex AI unit tests is available. update_responses.sh should be updated to clone the latest version of the responses: v5.2

google-oss-bot · 2024-11-25T21:43:27Z

Coverage Report ¹

Affected Products

firebase-crashlytics
Overall coverage changed from 11.58% (91418a1) to 11.56% (a320b4b) by -0.02%.
Filename Base (91418a1) Merge (a320b4b) Diff
CrashlyticsCore.java 0.61% 0.61% -0.00%

Test Logs

https://storage.googleapis.com/firebase-sdk-metric-reports/sLZMFayJZ4.html

google-oss-bot · 2024-11-25T21:46:07Z

The public api surface has changed for the subproject firebase-crashlytics:
error: Removed constructor com.google.firebase.crashlytics.KeyValueBuilder(com.google.firebase.crashlytics.FirebaseCrashlytics) [RemovedMethod]

Please update the api.txt files for the subprojects being affected by this change by running ./gradlew ${subproject}:generateApiTxtFile. Also perform a major/minor bump accordingly.

github-actions · 2024-11-25T21:48:28Z

Test Results

8 files - 1 028 8 suites - 1 028 19s ⏱️ - 32m 44s
22 tests - 5 851 22 ✅ - 5 829 0 💤 - 22 0 ❌ ±0
44 runs - 11 787 44 ✅ - 11 743 0 💤 - 44 0 ❌ ±0

Results for commit eb6c917. ± Comparison against base commit 91418a1.

This pull request removes 5851 tests.

com.google.android.datatransport.cct.CctBackendFactoryTest ‑ create_returnCCTBackend_WhenBackendNameIsCCT
com.google.android.datatransport.cct.CctDestinationTest ‑ cctDestination_shouldOnlySupportProtoAndJson
com.google.android.datatransport.cct.CctDestinationTest ‑ cctDestination_shouldSupportProtoAndJson
com.google.android.datatransport.cct.CctTransportBackendTest ‑ decorate_whenOffline_shouldProperlyPopulateNetworkInfo
com.google.android.datatransport.cct.CctTransportBackendTest ‑ decorate_whenOnline_shouldProperlyPopulateNetworkInfo
com.google.android.datatransport.cct.CctTransportBackendTest ‑ schedule_shouldAddCookieOnPseudonymousIds
com.google.android.datatransport.cct.CctTransportBackendTest ‑ schedule_shouldDropCookieOnMixedPseudonymousIds
com.google.android.datatransport.cct.CctTransportBackendTest ‑ send_CompressedResponseIsUncompressed
com.google.android.datatransport.cct.CctTransportBackendTest ‑ send_whenBackendRedirectsMoreThan5Times_shouldOnlyRedirect4Times
com.google.android.datatransport.cct.CctTransportBackendTest ‑ send_whenBackendRedirects_shouldCorrectlyFollowTheRedirectViaPost
…

♻️ This comment has been updated with latest results.

google-oss-bot · 2024-11-25T21:49:20Z

Size Report ¹

Affected Products

firebase-crashlytics
Type Base (91418a1) Merge (a320b4b) Diff
aar 411 kB 412 kB +806 B (+0.2%)
apk (release) 5.85 MB 5.85 MB +1.07 kB (+0.0%)

Test Logs

https://storage.googleapis.com/firebase-sdk-metric-reports/diJxJ1bZQU.html

# Conflicts: # firebase-crashlytics/CHANGELOG.md

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.kt

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/KeyValueBuilder.kt

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/CrashlyticsTests.kt

Fix inefficiency in the setCustomKeys Kotlin extension

0e44a52

mrober requested a review from themiswang November 25, 2024 21:39

mrober added the main-merge-ack label Nov 25, 2024

google-oss-bot added the size/M label Nov 25, 2024

Deprecate crashlytics param to KeyValueBuilder

e5a1077

themiswang approved these changes Nov 25, 2024

View reviewed changes

Merge branch 'refs/heads/main' into crashlytics-kts-customKey

8489203

# Conflicts: # firebase-crashlytics/CHANGELOG.md

thatfiredev reviewed Nov 27, 2024

View reviewed changes

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.kt Show resolved Hide resolved

thatfiredev reviewed Nov 28, 2024

View reviewed changes

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/KeyValueBuilder.kt Outdated Show resolved Hide resolved

mrober added 3 commits November 28, 2024 13:00

Merge branch 'refs/heads/main' into crashlytics-kts-customKey

de08936

Clean up ctor

edf81be

Format

3d96c85

mrober requested review from themiswang, tejasd, thatfiredev and rlazo November 28, 2024 19:57

themiswang approved these changes Nov 28, 2024

View reviewed changes

rlazo approved these changes Nov 28, 2024

View reviewed changes

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/KeyValueBuilder.kt Outdated Show resolved Hide resolved

Message

125762d

tejasd approved these changes Nov 28, 2024

View reviewed changes

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/CrashlyticsTests.kt Show resolved Hide resolved

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/CrashlyticsTests.kt Outdated Show resolved Hide resolved

mrober added 4 commits November 28, 2024 15:59

Fix flaky test

4329bc7

Fix another flaky test

d7ecd85

Update test case

2f7eea7

Format

eb6c917

thatfiredev approved these changes Nov 28, 2024

View reviewed changes

mrober merged commit 0743d5a into main Nov 28, 2024
42 checks passed

mrober deleted the crashlytics-kts-customKey branch November 28, 2024 21:39

firebase locked and limited conversation to collaborators Dec 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix inefficiency in the setCustomKeys Kotlin extension #6536

Fix inefficiency in the setCustomKeys Kotlin extension #6536

Uh oh!

mrober commented Nov 25, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Nov 25, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Nov 25, 2024

Uh oh!

google-oss-bot commented Nov 25, 2024 •

edited

Loading

`firebase-crashlytics`

Uh oh!

google-oss-bot commented Nov 25, 2024

Uh oh!

github-actions bot commented Nov 25, 2024 •

edited

Loading

Uh oh!

google-oss-bot commented Nov 25, 2024 •

edited

Loading

`firebase-crashlytics`

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Fix inefficiency in the setCustomKeys Kotlin extension #6536

Fix inefficiency in the setCustomKeys Kotlin extension #6536

Uh oh!

Conversation

mrober commented Nov 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 25, 2024

Vertex AI Mock Responses Check ⚠️

Uh oh!

google-oss-bot commented Nov 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report 1

Affected Products

firebase-crashlytics

Test Logs

Uh oh!

google-oss-bot commented Nov 25, 2024

Uh oh!

github-actions bot commented Nov 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

google-oss-bot commented Nov 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Size Report 1

Affected Products

firebase-crashlytics

Test Logs

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mrober commented Nov 25, 2024 •

edited

Loading

github-actions bot commented Nov 25, 2024 •

edited

Loading

google-oss-bot commented Nov 25, 2024 •

edited

Loading

Coverage Report ¹

`firebase-crashlytics`

github-actions bot commented Nov 25, 2024 •

edited

Loading

google-oss-bot commented Nov 25, 2024 •

edited

Loading

Size Report ¹

`firebase-crashlytics`