Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust Functions getters to be Kotlin source compatible #6530

Merged
merged 7 commits into from
Nov 25, 2024

Conversation

emilypgoogle
Copy link
Collaborator

No description provided.

@emilypgoogle emilypgoogle requested a review from rlazo November 22, 2024 21:11
Copy link
Contributor

github-actions bot commented Nov 22, 2024

Javadoc Changes:
--- /home/runner/diff/original/firebase-kotlindoc/android/com/google/firebase/functions/HttpsCallableResult.html	2024-11-25 21:12:58.307279339 +0000
+++ /home/runner/diff/modified/firebase-kotlindoc/android/com/google/firebase/functions/HttpsCallableResult.html	2024-11-25 21:10:28.590811067 +0000
@@ -21,6 +21,28 @@
         </colgroup>
         <thead>
           <tr>
+            <th colspan="100%"><h3>Public fields</h3></th>
+          </tr>
+        </thead>
+        <tbody class="list">
+          <tr>
+            <td><code>final <a href="https://developer.android.com/reference/kotlin/java/lang/Object.html">Object</a></code></td>
+            <td>
+              <div><code><a href="/docs/reference/android/com/google/firebase/functions/HttpsCallableResult.html#data()">data</a></code></div>
+              <p>The data that was returned from the Callable HTTPS trigger.</p>
+            </td>
+          </tr>
+        </tbody>
+      </table>
+    </div>
+    <div class="devsite-table-wrapper">
+      <table class="responsive">
+        <colgroup>
+          <col width="40%">
+          <col>
+        </colgroup>
+        <thead>
+          <tr>
             <th colspan="100%"><h3>Public methods</h3></th>
           </tr>
         </thead>
@@ -36,12 +58,21 @@
       </table>
     </div>
     <div class="list">
+      <h2>Public fields</h2>
+      <div class="api-item"><a name="getData()"></a><a name="setData()"></a><a name="getData--"></a><a name="setData--"></a>
+        <h3 class="api-name" id="data()">data</h3>
+        <pre class="api-signature no-pretty-print">public&nbsp;final&nbsp;<a href="https://developer.android.com/reference/kotlin/java/lang/Object.html">Object</a>&nbsp;<a href="/docs/reference/android/com/google/firebase/functions/HttpsCallableResult.html#data()">data</a></pre>
+        <p>The data that was returned from the Callable HTTPS trigger.</p>
+        <p>The data is in the form of native Java objects. For example, if your trigger returned an array, this object would be a <code>List&lt;Object&gt;</code>. If your trigger returned a JavaScript object with keys and values, this object would be a <code>Map&lt;String, Object&gt;</code>.</p>
+      </div>
+    </div>
+    <div class="list">
       <h2>Public methods</h2>
       <div class="api-item"><a name="getData--"></a><a name="getdata"></a>
         <h3 class="api-name" id="getData()">getData</h3>
         <pre class="api-signature no-pretty-print">public&nbsp;final&nbsp;<a href="https://developer.android.com/reference/kotlin/java/lang/Object.html">Object</a>&nbsp;<a href="/docs/reference/android/com/google/firebase/functions/HttpsCallableResult.html#getData()">getData</a>()</pre>
         <p>Returns the data that was returned from the Callable HTTPS trigger.</p>
-        <p>The data is in the form of native Java objects. For example, if your trigger returned an array, this object would be a List<Object>. If your trigger returned a JavaScript object with keys and values, this object would be a Map.</p>
+        <p>The data is in the form of native Java objects. For example, if your trigger returned an array, this object would be a <code>List&lt;Object&gt;</code>. If your trigger returned a JavaScript object with keys and values, this object would be a <code>Map&lt;String, Object&gt;</code>.</p>
       </div>
     </div>
   </body>
--- /home/runner/diff/original/firebase-kotlindoc/kotlin/com/google/firebase/functions/HttpsCallableResult.html	2024-11-25 21:12:58.295279301 +0000
+++ /home/runner/diff/modified/firebase-kotlindoc/kotlin/com/google/firebase/functions/HttpsCallableResult.html	2024-11-25 21:10:28.578811030 +0000
@@ -35,13 +35,44 @@
         </tbody>
       </table>
     </div>
+    <div class="devsite-table-wrapper">
+      <table class="responsive">
+        <colgroup>
+          <col width="40%">
+          <col>
+        </colgroup>
+        <thead>
+          <tr>
+            <th colspan="100%"><h3>Public properties</h3></th>
+          </tr>
+        </thead>
+        <tbody class="list">
+          <tr>
+            <td><code><a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-any/index.html">Any</a>?</code></td>
+            <td>
+              <div><code><a href="/docs/reference/kotlin/com/google/firebase/functions/HttpsCallableResult.html#data()">data</a></code></div>
+              <p>The data that was returned from the Callable HTTPS trigger.</p>
+            </td>
+          </tr>
+        </tbody>
+      </table>
+    </div>
     <div class="list">
       <h2>Public functions</h2>
       <div class="api-item"><a name="getData--"></a><a name="getdata"></a>
         <h3 class="api-name" id="getData()">getData</h3>
         <pre class="api-signature no-pretty-print">fun&nbsp;<a href="/docs/reference/kotlin/com/google/firebase/functions/HttpsCallableResult.html#getData()">getData</a>():&nbsp;<a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-any/index.html">Any</a>?</pre>
         <p>Returns the data that was returned from the Callable HTTPS trigger.</p>
-        <p>The data is in the form of native Java objects. For example, if your trigger returned an array, this object would be a List<Object>. If your trigger returned a JavaScript object with keys and values, this object would be a Map.</p>
+        <p>The data is in the form of native Java objects. For example, if your trigger returned an array, this object would be a <code>List&lt;Object&gt;</code>. If your trigger returned a JavaScript object with keys and values, this object would be a <code>Map&lt;String, Object&gt;</code>.</p>
+      </div>
+    </div>
+    <div class="list">
+      <h2>Public properties</h2>
+      <div class="api-item"><a name="getData()"></a><a name="setData()"></a><a name="getData--"></a><a name="setData--"></a>
+        <h3 class="api-name" id="data()">data</h3>
+        <pre class="api-signature no-pretty-print">val&nbsp;<a href="/docs/reference/kotlin/com/google/firebase/functions/HttpsCallableResult.html#data()">data</a>:&nbsp;<a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-any/index.html">Any</a>?</pre>
+        <p>The data that was returned from the Callable HTTPS trigger.</p>
+        <p>The data is in the form of native Java objects. For example, if your trigger returned an array, this object would be a <code>List&lt;Object&gt;</code>. If your trigger returned a JavaScript object with keys and values, this object would be a <code>Map&lt;String, Object&gt;</code>.</p>
       </div>
     </div>
   </body>

Copy link
Contributor

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
Copy link
Contributor

google-oss-bot commented Nov 22, 2024

Copy link
Contributor

github-actions bot commented Nov 22, 2024

Test Results

20 files   -  1 016  20 suites   - 1 016   36s ⏱️ - 36m 8s
23 tests  -  5 850  23 ✅  -  5 828  0 💤  - 22  0 ❌ ±0 
68 runs   - 11 763  68 ✅  - 11 719  0 💤  - 44  0 ❌ ±0 

Results for commit a1b1cfb. ± Comparison against base commit 1b24b82.

This pull request removes 5850 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
Copy link
Contributor

google-oss-bot commented Nov 22, 2024

Copy link
Collaborator

@rlazo rlazo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just need to fix the instrumentation tests

@rlazo
Copy link
Collaborator

rlazo commented Nov 23, 2024

Also, please reference #6522

@daymxn
Copy link
Member

daymxn commented Nov 25, 2024

To avoid a breaking change, we'd have to use @JvmField with a custom getter, and make sure that the data field is public. We'd also have to duplicate our javadcos (or just add a kdoc link that points to getData).

/** The result of calling a HttpsCallableReference function. */
public class HttpsCallableResult
internal constructor(
  /** Backing field for [getData]. */
  @JvmSynthetic @JvmField public val data: Any?
) {
  /**
   * Returns the data that was returned from the Callable HTTPS trigger.
   *
   * The data is in the form of native Java objects. For example, if your trigger returned an array,
   * this object would be a List<Object>. If your trigger returned a JavaScript object with keys and
   * values, this object would be a Map<String, Object>.
   */
  public fun getData(): Any? {
    return data
  }
}

This will allow kotlin consumers to call .data and .getData().

But Java consumers can also call .data and .getData().

We can block java consumers from using it by using @JvmSynthetic, but this can negatively impact stack traces. We can also do @JvmName("--data"), and that'll also hide it without hurting stack traces- but it's a bandaid if anything.

Seeing as how removing .data in the future would be a breaking change regardless, I'd argue that it's fine to expose it to java consumers. So just using @JvmField should be fine. wdyt @rlazo ?

@rlazo
Copy link
Collaborator

rlazo commented Nov 25, 2024

To avoid a breaking change, we'd have to use @JvmField with a custom getter, and make sure that the data field is public. We'd also have to duplicate our javadcos (or just add a kdoc link that points to getData).

/** The result of calling a HttpsCallableReference function. */
public class HttpsCallableResult
internal constructor(
  /** Backing field for [getData]. */
  @JvmSynthetic @JvmField public val data: Any?
) {
  /**
   * Returns the data that was returned from the Callable HTTPS trigger.
   *
   * The data is in the form of native Java objects. For example, if your trigger returned an array,
   * this object would be a List<Object>. If your trigger returned a JavaScript object with keys and
   * values, this object would be a Map<String, Object>.
   */
  public fun getData(): Any? {
    return data
  }
}

This will allow kotlin consumers to call .data and .getData().

But Java consumers can also call .data and .getData().

We can block java consumers from using it by using @JvmSynthetic, but this can negatively impact stack traces. We can also do @JvmName("--data"), and that'll also hide it without hurting stack traces- but it's a bandaid if anything.

Seeing as how removing .data in the future would be a breaking change regardless, I'd argue that it's fine to expose it to java consumers. So just using @JvmField should be fine. wdyt @rlazo ?

SGTM

@google-oss-bot
Copy link
Contributor

The public api surface has changed for the subproject firebase-functions:
error: Added field com.google.firebase.functions.HttpsCallableResult.data [AddedField]

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.

@emilypgoogle emilypgoogle enabled auto-merge (squash) November 25, 2024 21:07
@emilypgoogle emilypgoogle merged commit 04bfe55 into main Nov 25, 2024
33 checks passed
@emilypgoogle emilypgoogle deleted the ep/functions-getter branch November 25, 2024 21:14
@firebase firebase locked and limited conversation to collaborators Dec 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants