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.

Sign up for GitHub

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

Jump to bottom

Add support for mutable messages in Wire's Kotlin Generator. #3217

Merged
merged 6 commits into from
Dec 24, 2024
Merged

Add support for mutable messages in Wire's Kotlin Generator. #3217

merged 6 commits into from
Dec 24, 2024

Conversation

tikurahul
Copy link
Contributor

@tikurahul tikurahul commented Dec 17, 2024 •
edited
Loading

  • Sometimes its very useful to avoid needing to create new object graph instances when all we need to do is to serialize the object graph into a ProtoSink.
  • Mutable messages solves for this, for code that is particularly performance sensitive where a large number of objects would have otherwise been created which adds unnecesssary GC pressure.
  • Mutable messages should be used very carefully. There are no guarantees of safety whatsoever.
  • Mutable messages don't use Builders and so newBuilder() throws an UnsupportedOperationException().
  • Mutable messages don't support copy(...).
  • When using mutable messages, hashCode() is no longer cached.
  • Mutable messages don't support redact().

Test: Added a unit test in KotlinGeneratorTest.

@tikurahul tikurahul requested a review from oldergod December 18, 2024 23:10
tikurahul and others added 6 commits December 20, 2024 13:00
@tikurahul
Add support for mutable messages in Wire's Kotlin Generator.
fd9bfb9 
* Sometimes its very useful to avoid needing to create new object graph instances when all we need to do is to serialize the object graph into a `ProtoSink`.
* Mutable messages solves for this, for code that is particularly performance sensitive where a large number of objects would have otherwise been created which adds unnecesssary GC pressure.
* Mutable messages should be **used very carefully**. There are **no guarantees of safety whatsoever**.
* When using mutable messages, `hashCode()` is no longer cached.
* Mutable messages don't support `redact()`.

Test: Added a unit test in KotlinGeneratorTest
@oldergod @tikurahul
Add mutableTypes to Wire plugin and set golden file
00a9852
@tikurahul
Address comments in the PR.
93fe2e8 
* `newBuilder()` throws an `UnsupportedOperationException` instead. Thanks @oldergod.
* Updated the `KotlinGeneratorTest` to use the renamed option in the `KotlinGenerator`.
* Update goldens to include the new error message.
* Update API files for `wire-compiler`.
@tikurahul
Open unknownFields for Mutable messages.
dc31133 
* `Mutable` messsages now have access to mutable `unknownFields`.

Test: Updated KotlinGeneratorTest and the golden tests.
@tikurahul
Open unknownFields for Mutable messages.
ad6d312 
* `Mutable` messsages now have access to mutable `unknownFields`.

Test: Updated KotlinGeneratorTest and the golden tests.
@tikurahul
Bring back referential equality check.
88666f2 
* Regenerate golden files.
* Update API files for `wire-gradle-plugin`.
* Update API files for `wire-kotlin-generator`.
* Spotless checks.
squarejesse
squarejesse reviewed Dec 23, 2024
View reviewed changes
wire-golden-files/src/main/kotlin/squareup/wire/mutable/MutableHeader.kt
)
override fun newBuilder(): Nothing = throw UnsupportedOperationException("newBuilder() is unsupported for mutable message types")

override fun equals(other: Any?): Boolean {
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be by-identity rather than by-value? Equals is weird on mutable things

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did you mean that we should only support referential equality ?

I think it's also okay for us to also compare the values. Otherwise it might just end up being confusing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good.

squarejesse
squarejesse approved these changes Dec 23, 2024
View reviewed changes
Copy link
Contributor

@squarejesse squarejesse left a comment

Choose a reason for hiding this comment

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

This is rad

@JakeWharton JakeWharton merged commit 54e032b into square:master Dec 24, 2024
11 checks passed
svc-squareup-copybara pushed a commit to cashapp/misk that referenced this pull request Jan 24, 2025
@github-cash-renovate-jvm-2 @svc-squareup-copybara
This PR contains the following updates:
afc3ab9 
| Package | Type | Package file | Manager | Update | Change |
|---|---|---|---|---|---|
|
[org.jetbrains:annotations](https://github.com/JetBrains/java-annotations)
| dependencies | misk/gradle/libs.versions.toml | gradle | patch |
`26.0.1` -> `26.0.2` |
| [io.grpc:grpc-stub](https://github.com/grpc/grpc-java) | dependencies
| misk/gradle/libs.versions.toml | gradle | minor | `1.69.1` -> `1.70.0`
|
| [io.grpc:grpc-protobuf](https://github.com/grpc/grpc-java) |
dependencies | misk/gradle/libs.versions.toml | gradle | minor |
`1.69.1` -> `1.70.0` |
| [io.grpc:grpc-netty](https://github.com/grpc/grpc-java) | dependencies
| misk/gradle/libs.versions.toml | gradle | minor | `1.69.1` -> `1.70.0`
|
| [io.grpc:protoc-gen-grpc-java](https://github.com/grpc/grpc-java) |
dependencies | misk/gradle/libs.versions.toml | gradle | minor |
`1.69.1` -> `1.70.0` |
| [io.grpc:grpc-bom](https://github.com/grpc/grpc-java) | dependencies |
misk/gradle/libs.versions.toml | gradle | minor | `1.69.1` -> `1.70.0` |
| [io.grpc:grpc-api](https://github.com/grpc/grpc-java) | dependencies |
misk/gradle/libs.versions.toml | gradle | minor | `1.69.1` -> `1.70.0` |
|
[com.google.api-client:google-api-client](https://github.com/googleapis/google-api-java-client)
| dependencies | misk/gradle/libs.versions.toml | gradle | patch |
`2.7.1` -> `2.7.2` |
| [com.squareup.wire:wire-schema](https://github.com/square/wire) |
dependencies | misk/gradle/libs.versions.toml | gradle | minor | `5.0.0`
-> `5.2.1` |
| [com.squareup.wire:wire-runtime](https://github.com/square/wire) |
dependencies | misk/gradle/libs.versions.toml | gradle | minor | `5.0.0`
-> `5.2.1` |
| [com.squareup.wire:wire-reflector](https://github.com/square/wire) |
dependencies | misk/gradle/libs.versions.toml | gradle | minor | `5.0.0`
-> `5.2.1` |
| [com.squareup.wire:wire-moshi-adapter](https://github.com/square/wire)
| dependencies | misk/gradle/libs.versions.toml | gradle | minor |
`5.0.0` -> `5.2.1` |
| [com.squareup.wire:wire-grpc-client](https://github.com/square/wire) |
dependencies | misk/gradle/libs.versions.toml | gradle | minor | `5.0.0`
-> `5.2.1` |
| [com.squareup.wire:wire-gradle-plugin](https://github.com/square/wire)
| dependencies | misk/gradle/libs.versions.toml | gradle | minor |
`5.0.0` -> `5.2.1` |
| [com.squareup.wire:wire-bom](https://github.com/square/wire) |
dependencies | misk/gradle/libs.versions.toml | gradle | minor | `5.0.0`
-> `5.2.1` |
|
[com.google.auth:google-auth-library-oauth2-http](https://github.com/googleapis/google-auth-library-java)
| dependencies | misk/gradle/libs.versions.toml | gradle | minor |
`1.30.1` -> `1.31.0` |
|
[com.google.auth:google-auth-library-credentials](https://github.com/googleapis/google-auth-library-java)
| dependencies | misk/gradle/libs.versions.toml | gradle | minor |
`1.30.1` -> `1.31.0` |
| [com.datadoghq:dd-trace-api](https://github.com/datadog/dd-trace-java)
| dependencies | misk/gradle/libs.versions.toml | gradle | patch |
`1.45.1` -> `1.45.2` |
| [com.datadoghq:dd-trace-ot](https://github.com/datadog/dd-trace-java)
| dependencies | misk/gradle/libs.versions.toml | gradle | patch |
`1.45.1` -> `1.45.2` |
| [software.amazon.awssdk:sdk-core](https://aws.amazon.com/sdkforjava) |
dependencies | misk/gradle/libs.versions.toml | gradle | patch |
`2.30.2` -> `2.30.4` |
|
[software.amazon.awssdk:dynamodb-enhanced](https://aws.amazon.com/sdkforjava)
| dependencies | misk/gradle/libs.versions.toml | gradle | patch |
`2.30.2` -> `2.30.4` |
| [software.amazon.awssdk:dynamodb](https://aws.amazon.com/sdkforjava) |
dependencies | misk/gradle/libs.versions.toml | gradle | patch |
`2.30.2` -> `2.30.4` |
| [software.amazon.awssdk:aws-core](https://aws.amazon.com/sdkforjava) |
dependencies | misk/gradle/libs.versions.toml | gradle | patch |
`2.30.2` -> `2.30.4` |
| [software.amazon.awssdk:bom](https://aws.amazon.com/sdkforjava) |
dependencies | misk/gradle/libs.versions.toml | gradle | patch |
`2.30.2` -> `2.30.4` |
| [software.amazon.awssdk:auth](https://aws.amazon.com/sdkforjava) |
dependencies | misk/gradle/libs.versions.toml | gradle | patch |
`2.30.2` -> `2.30.4` |

---

### Release Notes

<details>
<summary>JetBrains/java-annotations
(org.jetbrains:annotations)</summary>

###
[`v26.0.2`](https://github.com/JetBrains/java-annotations/blob/HEAD/CHANGELOG.md#Version-2602)

[Compare
Source](JetBrains/java-annotations@26.0.1...26.0.2)

-   Fixed missing klibs for apple artifacts.

</details>

<details>
<summary>googleapis/google-api-java-client
(com.google.api-client:google-api-client)</summary>

###
[`v2.7.2`](https://github.com/googleapis/google-api-java-client/blob/HEAD/CHANGELOG.md#272-2025-01-22)

##### Bug Fixes

- Add warnings to users about using credentials from external sources
([#&#8203;2551](googleapis/google-api-java-client#2551))
([3bb2879](googleapis/google-api-java-client@3bb2879))

</details>

<details>
<summary>square/wire (com.squareup.wire:wire-schema)</summary>

###
[`v5.2.1`](https://github.com/square/wire/blob/HEAD/CHANGELOG.md#Version-521)

[Compare Source](square/wire@5.2.0...5.2.1)

*2025-01-07*

##### JVM generation

- Fix support for mutable messages in Wire's Kotlin Generator.
([#&#8203;3233](square/wire#3233) by \[Rahul
Ravikumar]\[tikurahul])

###
[`v5.2.0`](https://github.com/square/wire/blob/HEAD/CHANGELOG.md#Version-520)

[Compare Source](square/wire@5.1.0...5.2.0)

*2025-01-06*

##### Common

- Enforce recursion limit when parsing nested groups.
([#&#8203;3119](square/wire#3119))

##### CLI `wire-compiler`

- It is now possible to set multiple targets.
([#&#8203;3106](square/wire#3106) &
[#&#8203;3107](square/wire#3107))
- The option `opaque_types` introduced in `4.9.2` for the Wire Gradle
plugin is now available on CLI.
([#&#8203;3147](square/wire#3147))

##### JVM generation

- [KotlinPoet has been updated to
`2.0.0`](https://square.github.io/kotlinpoet/changelog/#version-200)
which dramatically changes how generated Kotlin files are wrapped. This
is neither a source nor a binary breaking changes.
- A new `@WireEnclosingType` annotation is now applied to generated
types so R8 doesn't prune too much.
([#&#8203;3123](square/wire#3123))
- Split the redact method into chunks when a type has more than 100
fields to avoid compilation error.
([#&#8203;3214](square/wire#3214) by \[Damian
Wieczorek]\[damianw])
- Add support for mutable messages in Wire's Kotlin Generator.
([#&#8203;3217](square/wire#3217) by \[Rahul
Ravikumar]\[tikurahul])
- You can opt-in by adding `mutableTypes = true` on your Kotlin target.
This is unsafe and we do not recommend that you use it unless you have a
sound use-case for it.
-   Wire is now using Palantir's JavaPoet instead of Square's JavaPoet.

##### Swift

- Fix buffer overflow and data corruption when a type has more than 5
layers of nesting
([#&#8203;3203](square/wire#3203) by \[Eric
Amorde]\[amorde])

###
[`v5.1.0`](https://github.com/square/wire/blob/HEAD/CHANGELOG.md#Version-510)

[Compare Source](square/wire@5.0.0...5.1.0)

*2024-09-11*

##### Common

- Support for Kotlin `2.0.20`.
([#&#8203;3093](square/wire#3093))
- `srcDir(String)` has been undeprecated.
([#&#8203;3039](square/wire#3039))
- Some loggings now happen at the debug level, instead of info.
([#&#8203;3041](square/wire#3041))
- Remove some unactionable warnings on Kotlin/JS
([#&#8203;3047](square/wire#3047))
- Propagate the deprecated flag on EnumType after pruning by
wire-gradle-plugin
([#&#8203;3076](square/wire#3076) by \[Aaron
Edwards]\[aaron-edwards])
- Introduce `ProtoReader32`, a specialization for Kotlin/JS
([#&#8203;3077](square/wire#3077))
This is an alternative to `ProtoReader`, which uses `Long` as a cursor.
It originates as an optimization for Kotlin/JS, where `Long` cursors are
prohibitively expensive.
- Fix Gradle project isolation issue when reading a property
([#&#8203;3078](square/wire#3078) by
\[Aurimas]\[liutikas])
- Change the recursion limit to match grpc's default
([#&#8203;3091](square/wire#3091))

##### Kotlin

- New enum option `enum_mode` to take precedence over the `enumMode`
option added in `5.0.0-alpha02`. Use this if you want to migrate your
enums granularly.
([#&#8203;2993](square/wire#2993))
- Don't throw if reading trailers fail
([#&#8203;3087](square/wire#3087))

##### Swift

- Avoid crash when parsing an empty repeated `[packed=true]` for
fixed-length types.
([#&#8203;3044](square/wire#3044) by \[Sasha
Weiss]\[sashaweiss-signal])

</details>

<details>
<summary>googleapis/google-auth-library-java
(com.google.auth:google-auth-library-oauth2-http)</summary>

###
[`v1.31.0`](https://github.com/googleapis/google-auth-library-java/blob/HEAD/CHANGELOG.md#1310-2025-01-22)

##### Features

- ImpersonatedCredentials to support universe domain for idtoken and
signblob
([#&#8203;1566](googleapis/google-auth-library-java#1566))
([adc2ff3](googleapis/google-auth-library-java@adc2ff3))
- Support transport and binding-enforcement MDS parameters.
([#&#8203;1558](googleapis/google-auth-library-java#1558))
([9828a8e](googleapis/google-auth-library-java@9828a8e))

##### Documentation

- Promote use of bill of materials in quickstart documentation
([#&#8203;1620](googleapis/google-auth-library-java#1620))
([fc20d9c](googleapis/google-auth-library-java@fc20d9c)),
closes
[#&#8203;1552](googleapis/google-auth-library-java#1552)

</details>

<details>
<summary>datadog/dd-trace-java (com.datadoghq:dd-trace-api)</summary>

###
[`v1.45.2`](https://github.com/DataDog/dd-trace-java/releases/tag/v1.45.2):
1.45.2

##### Components

##### Application Security Management (WAF)

- 🐛 🍒 8258 - Prevents a NPE when there is no subscriber for user
events
([#&#8203;8260](DataDog/dd-trace-java#8260) -
[@&#8203;manuel-alvarez-alvarez](https://github.com/manuel-alvarez-alvarez))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 6pm every weekday,before 2am
every weekday" in timezone Australia/Melbourne, Automerge - At any time
(no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://github.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://github.com/renovatebot/renovate).

GitOrigin-RevId: 12db0f59db2e6ebf55203c87fccab042d495106a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JakeWharton JakeWharton JakeWharton approved these changes

@squarejesse squarejesse squarejesse approved these changes

@oldergod oldergod Awaiting requested review from oldergod

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tikurahul @JakeWharton @oldergod @squarejesse