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

Firestore: update javadocs of Exclude, PropertyName, and ServerTimestamp to explicitly document using the @get and/or @set use-site targets on Kotlin properties #6612

Merged
merged 6 commits into from
Jan 3, 2025

Conversation

dconeybe
Copy link
Contributor

@dconeybe dconeybe commented Jan 2, 2025

Fixed #6601

…amp to explicitly document using the `@get` and/or `@set` use-site targets on Kotlin properties.
Copy link
Contributor

github-actions bot commented Jan 2, 2025

Javadoc Changes:
--- /home/runner/diff/original/firebase-kotlindoc/android/com/google/firebase/firestore/DocumentId.html	2025-01-03 16:36:21.480169669 +0000
+++ /home/runner/diff/modified/firebase-kotlindoc/android/com/google/firebase/firestore/DocumentId.html	2025-01-03 16:33:24.225285841 +0000
@@ -20,6 +20,13 @@
       <li></li>
     </ul>
     <p>When using a POJO to write to a document (via <code><a href="/docs/reference/android/com/google/firebase/firestore/DocumentReference.html#set(java.lang.Object)">set</a></code> or @<code><a href="/docs/reference/android/com/google/firebase/firestore/WriteBatch.html#set(com.google.firebase.firestore.DocumentReference,java.lang.Object)">set</a></code>), the property annotated by <code>@DocumentId</code> is ignored, which allows writing the POJO back to any document, even if it's not the origin of the POJO.</p>
+    <h3>Kotlin Note</h3>
+ When applying this annotation to a property of a Kotlin class, the <code>@set</code> use-site target should always be used. There is no need to use the <code>@get</code> use-site target as this annotation is <em>only</em> considered when <em>reading</em> instances from Firestore, and is ignored when <em>writing</em> instances into Firestore. 
+    <p> Here is an example of a class that can both be written into and read from Firestore whose <code>foo</code> property will be populated with the Document ID when being read and will be ignored when being written into Firestore: </p>
+    <pre class="prettyprint">data class Pojo(@set:DocumentId var foo: String? = null) {
+  constructor() : this(null) // Used by Firestore to create new instances
+}
+</pre>
   </body>
 </html>
 
--- /home/runner/diff/original/firebase-kotlindoc/android/com/google/firebase/firestore/Exclude.html	2025-01-03 16:36:21.484169667 +0000
+++ /home/runner/diff/modified/firebase-kotlindoc/android/com/google/firebase/firestore/Exclude.html	2025-01-03 16:33:24.225285841 +0000
@@ -12,6 +12,16 @@
     </p>
     <hr>
     <p>Marks a field as excluded from the database instance.</p>
-  </body>
+    <h3>Kotlin Note</h3>
+ When applying this annotation to a property of a Kotlin class, the <code>@get</code> use-site target should always be used. There is no need to use the <code>@set</code> use-site target as this annotation is <em>only</em> considered when <em>writing</em> instances into Firestore, and is ignored when <em>reading</em> instances from Firestore. 
+    <p> Here is an example of a class that can both be written into and read from Firestore whose <code>bar</code> property will never be written into Firestore: </p>
+    <pre class="prettyprint">data class Pojo(var foo: String? = null, @get:Exclude var bar: String? = null) {
+  constructor() : this(null, null) // Used by Firestore to create new instances
+}
+</pre>
+    <p> If the class only needs to be <em>written</em> into Firestore (and not read from Firestore) then the class can be simplified as follows: </p>
+    <pre class="prettyprint">data class Pojo(val foo: String? = null, @get:Exclude val bar: String? = null)
+</pre>
+ That is, <code>var</code> can be tightened to <code>val</code> and the secondary no-argument constructor can be omitted.</body>
 </html>
 
--- /home/runner/diff/original/firebase-kotlindoc/android/com/google/firebase/firestore/PropertyName.html	2025-01-03 16:36:21.455169684 +0000
+++ /home/runner/diff/modified/firebase-kotlindoc/android/com/google/firebase/firestore/PropertyName.html	2025-01-03 16:33:24.221285845 +0000
@@ -12,6 +12,17 @@
     </p>
     <hr>
     <p>Marks a field to be renamed when serialized.</p>
+    <h3>Kotlin Note</h3>
+ When applying this annotation to a property of a Kotlin class, both the <code>@get</code> and <code>@set</code> use-site targets should be used. 
+    <p> Here is an example of a class that can both be written into and read from Firestore whose <code>foo</code> property will be stored into and read from a field named <code>my_foo</code> in the Firestore document: </p>
+    <pre class="prettyprint">data class Pojo(@get:PropertyName(&quot;my_foo&quot;) @set:PropertyName(&quot;my_foo&quot;) var foo: String? = null) {
+  constructor() : this(null) // Used by Firestore to create new instances
+}
+</pre>
+    <p> If the class only needs to be <em>written</em> into Firestore (and not read from Firestore) then the class can be simplified as follows: </p>
+    <pre class="prettyprint">data class Pojo(@get:PropertyName(&quot;my_foo&quot;) val foo: String? = null)
+</pre>
+ That is, <code>var</code> can be tightened to <code>val</code>, the secondary no-argument constructor can be omitted, and the <code>@set</code> use-site target can be omitted.
     <h2>Summary</h2>
     <div class="devsite-table-wrapper">
       <table class="responsive">
--- /home/runner/diff/original/firebase-kotlindoc/android/com/google/firebase/firestore/ServerTimestamp.html	2025-01-03 16:36:21.484169667 +0000
+++ /home/runner/diff/modified/firebase-kotlindoc/android/com/google/firebase/firestore/ServerTimestamp.html	2025-01-03 16:33:24.225285841 +0000
@@ -12,6 +12,16 @@
     </p>
     <hr>
     <p>Annotation used to mark a timestamp field to be populated with a server timestamp. If a POJO being written contains <code>null</code> for a @ServerTimestamp-annotated field, it will be replaced with a server-generated timestamp.</p>
-  </body>
+    <h3>Kotlin Note</h3>
+ When applying this annotation to a property of a Kotlin class, the <code>@get</code> use-site target should always be used. There is no need to use the <code>@set</code> use-site target as this annotation is <em>only</em> considered when <em>writing</em> instances into Firestore, and is ignored when <em>reading</em> instances from Firestore. 
+    <p> Here is an example of a class that can both be written into and read from Firestore whose <code>foo</code> property will be populated with the server timestamp in Firestore if its value is null: </p>
+    <pre class="prettyprint">data class Pojo(@get:ServerTimestamp var foo: Timestamp? = null) {
+  constructor() : this(null) // Used by Firestore to create new instances
+}
+</pre>
+    <p> If the class only needs to be <em>written</em> into Firestore (and not read from Firestore) then the class can be simplified as follows: </p>
+    <pre class="prettyprint">data class Pojo(@get:ServerTimestamp val foo: Timestamp? = null)
+</pre>
+ That is, <code>var</code> can be tightened to <code>val</code> and the secondary no-argument constructor can be omitted.</body>
 </html>
 
--- /home/runner/diff/original/firebase-kotlindoc/kotlin/com/google/firebase/firestore/DocumentId.html	2025-01-03 16:36:21.525169642 +0000
+++ /home/runner/diff/modified/firebase-kotlindoc/kotlin/com/google/firebase/firestore/DocumentId.html	2025-01-03 16:33:24.236285833 +0000
@@ -20,6 +20,13 @@
       <li></li>
     </ul>
     <p>When using a POJO to write to a document (via <code><a href="/docs/reference/kotlin/com/google/firebase/firestore/DocumentReference.html#set(java.lang.Object)">set</a></code> or @<code><a href="/docs/reference/kotlin/com/google/firebase/firestore/WriteBatch.html#set(com.google.firebase.firestore.DocumentReference,java.lang.Object)">set</a></code>), the property annotated by <code>@DocumentId</code> is ignored, which allows writing the POJO back to any document, even if it's not the origin of the POJO.</p>
+    <h3>Kotlin Note</h3>
+ When applying this annotation to a property of a Kotlin class, the <code>@set</code> use-site target should always be used. There is no need to use the <code>@get</code> use-site target as this annotation is <em>only</em> considered when <em>reading</em> instances from Firestore, and is ignored when <em>writing</em> instances into Firestore. 
+    <p> Here is an example of a class that can both be written into and read from Firestore whose <code>foo</code> property will be populated with the Document ID when being read and will be ignored when being written into Firestore: </p>
+    <pre class="prettyprint">data class Pojo(@set:DocumentId var foo: String? = null) {
+  constructor() : this(null) // Used by Firestore to create new instances
+}
+</pre>
   </body>
 </html>
 
--- /home/runner/diff/original/firebase-kotlindoc/kotlin/com/google/firebase/firestore/Exclude.html	2025-01-03 16:36:21.525169642 +0000
+++ /home/runner/diff/modified/firebase-kotlindoc/kotlin/com/google/firebase/firestore/Exclude.html	2025-01-03 16:33:24.236285833 +0000
@@ -12,6 +12,16 @@
     </p>
     <hr>
     <p>Marks a field as excluded from the database instance.</p>
-  </body>
+    <h3>Kotlin Note</h3>
+ When applying this annotation to a property of a Kotlin class, the <code>@get</code> use-site target should always be used. There is no need to use the <code>@set</code> use-site target as this annotation is <em>only</em> considered when <em>writing</em> instances into Firestore, and is ignored when <em>reading</em> instances from Firestore. 
+    <p> Here is an example of a class that can both be written into and read from Firestore whose <code>bar</code> property will never be written into Firestore: </p>
+    <pre class="prettyprint">data class Pojo(var foo: String? = null, @get:Exclude var bar: String? = null) {
+  constructor() : this(null, null) // Used by Firestore to create new instances
+}
+</pre>
+    <p> If the class only needs to be <em>written</em> into Firestore (and not read from Firestore) then the class can be simplified as follows: </p>
+    <pre class="prettyprint">data class Pojo(val foo: String? = null, @get:Exclude val bar: String? = null)
+</pre>
+ That is, <code>var</code> can be tightened to <code>val</code> and the secondary no-argument constructor can be omitted.</body>
 </html>
 
--- /home/runner/diff/original/firebase-kotlindoc/kotlin/com/google/firebase/firestore/PropertyName.html	2025-01-03 16:36:21.498169658 +0000
+++ /home/runner/diff/modified/firebase-kotlindoc/kotlin/com/google/firebase/firestore/PropertyName.html	2025-01-03 16:33:24.229285838 +0000
@@ -12,6 +12,17 @@
     </p>
     <hr>
     <p>Marks a field to be renamed when serialized.</p>
+    <h3>Kotlin Note</h3>
+ When applying this annotation to a property of a Kotlin class, both the <code>@get</code> and <code>@set</code> use-site targets should be used. 
+    <p> Here is an example of a class that can both be written into and read from Firestore whose <code>foo</code> property will be stored into and read from a field named <code>my_foo</code> in the Firestore document: </p>
+    <pre class="prettyprint">data class Pojo(@get:PropertyName(&quot;my_foo&quot;) @set:PropertyName(&quot;my_foo&quot;) var foo: String? = null) {
+  constructor() : this(null) // Used by Firestore to create new instances
+}
+</pre>
+    <p> If the class only needs to be <em>written</em> into Firestore (and not read from Firestore) then the class can be simplified as follows: </p>
+    <pre class="prettyprint">data class Pojo(@get:PropertyName(&quot;my_foo&quot;) val foo: String? = null)
+</pre>
+ That is, <code>var</code> can be tightened to <code>val</code>, the secondary no-argument constructor can be omitted, and the <code>@set</code> use-site target can be omitted.
     <h2>Summary</h2>
     <div class="devsite-table-wrapper">
       <table class="responsive">
--- /home/runner/diff/original/firebase-kotlindoc/kotlin/com/google/firebase/firestore/ServerTimestamp.html	2025-01-03 16:36:21.525169642 +0000
+++ /home/runner/diff/modified/firebase-kotlindoc/kotlin/com/google/firebase/firestore/ServerTimestamp.html	2025-01-03 16:33:24.236285833 +0000
@@ -12,6 +12,16 @@
     </p>
     <hr>
     <p>Annotation used to mark a timestamp field to be populated with a server timestamp. If a POJO being written contains <code>null</code> for a @ServerTimestamp-annotated field, it will be replaced with a server-generated timestamp.</p>
-  </body>
+    <h3>Kotlin Note</h3>
+ When applying this annotation to a property of a Kotlin class, the <code>@get</code> use-site target should always be used. There is no need to use the <code>@set</code> use-site target as this annotation is <em>only</em> considered when <em>writing</em> instances into Firestore, and is ignored when <em>reading</em> instances from Firestore. 
+    <p> Here is an example of a class that can both be written into and read from Firestore whose <code>foo</code> property will be populated with the server timestamp in Firestore if its value is null: </p>
+    <pre class="prettyprint">data class Pojo(@get:ServerTimestamp var foo: Timestamp? = null) {
+  constructor() : this(null) // Used by Firestore to create new instances
+}
+</pre>
+    <p> If the class only needs to be <em>written</em> into Firestore (and not read from Firestore) then the class can be simplified as follows: </p>
+    <pre class="prettyprint">data class Pojo(@get:ServerTimestamp val foo: Timestamp? = null)
+</pre>
+ That is, <code>var</code> can be tightened to <code>val</code> and the secondary no-argument constructor can be omitted.</body>
 </html>
 

Copy link
Contributor

github-actions bot commented Jan 2, 2025

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.3

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jan 2, 2025

Coverage Report 1

Affected Products

  • firebase-firestore

    Overall coverage changed from 45.73% (320a3ed) to 45.72% (feb597e) by -0.01%.

    FilenameBase (320a3ed)Merge (feb597e)Diff
    DeleteMutation.java95.24%90.48%-4.76%
    LruGarbageCollector.java97.27%93.64%-3.64%
    PatchMutation.java98.39%100.00%+1.61%
    SetMutation.java94.44%97.22%+2.78%

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/3XGxBTy8tJ.html

Copy link
Contributor

github-actions bot commented Jan 2, 2025

Test Results

  186 files  +  172    186 suites  +172   4m 37s ⏱️ + 4m 25s
1 234 tests +1 211  1 218 ✅ +1 195  16 💤 +16  0 ❌ ±0 
2 492 runs  +2 446  2 460 ✅ +2 414  32 💤 +32  0 ❌ ±0 

Results for commit 98d526a. ± Comparison against base commit 320a3ed.

This pull request removes 23 and adds 1234 tests. Note that renamed tests count towards both.
com.google.firebase.functions.AppCheckLimitedUseTest ‑ FirebaseFunctions#getHttpsCallable should build callable with FAC settings (when false)
com.google.firebase.functions.AppCheckLimitedUseTest ‑ FirebaseFunctions#getHttpsCallable should build callable with FAC settings (when true)
com.google.firebase.functions.AppCheckLimitedUseTest ‑ FirebaseFunctions#getHttpsCallable should not use limited-use tokens by default
com.google.firebase.functions.AppCheckLimitedUseTest ‑ FirebaseFunctions#getHttpsCallableFromUrl callable with FAC settings (when false)
com.google.firebase.functions.AppCheckLimitedUseTest ‑ FirebaseFunctions#getHttpsCallableFromUrl callable with FAC settings (when true)
com.google.firebase.functions.AppCheckLimitedUseTest ‑ FirebaseFunctions#getHttpsCallableFromUrl should not use limited-use tokens by default
com.google.firebase.functions.FunctionsRegistrarTest ‑ getComponents_publishesLibVersionComponent
com.google.firebase.functions.FunctionsTests ‑ Firebase#functions should delegate to FirebaseFunctions#getInstance(FirebaseApp, region)
com.google.firebase.functions.FunctionsTests ‑ Firebase#functions should delegate to FirebaseFunctions#getInstance(region)
com.google.firebase.functions.FunctionsTests ‑ FirebaseApp#functions should delegate to FirebaseFunctions#getInstance(FirebaseApp)
…
com.google.firebase.firestore.AggregateQuerySnapshotTest ‑ createWithCountShouldReturnInstanceWithTheGivenQueryAndCount
com.google.firebase.firestore.AggregateQueryTest ‑ testSourceMustNotBeNull
com.google.firebase.firestore.BlobTest ‑ testComparison
com.google.firebase.firestore.BlobTest ‑ testEquals
com.google.firebase.firestore.BlobTest ‑ testMutableBytes
com.google.firebase.firestore.CollectionReferenceTest ‑ testEquals
com.google.firebase.firestore.DocumentChangeTest ‑ randomTests
com.google.firebase.firestore.DocumentChangeTest ‑ testAdditions
com.google.firebase.firestore.DocumentChangeTest ‑ testChangesWithSortOrderChange
com.google.firebase.firestore.DocumentChangeTest ‑ testDeletions
…

♻️ This comment has been updated with latest results.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jan 2, 2025

@dconeybe dconeybe merged commit f024090 into main Jan 3, 2025
32 checks passed
@dconeybe dconeybe deleted the dconeybe/firestore/AnnotationKtDocs branch January 3, 2025 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Firestore Document annotations are confusing (fail) when used together
3 participants