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

implement a converter to dynamic message #212

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
2dd4541
implement a converter to dynamic message
andrewparmet Dec 30, 2023
dd96e8e
api dump
andrewparmet Dec 30, 2023
72fbb9f
fix
andrewparmet Dec 30, 2023
da3ff2e
internal
andrewparmet Dec 30, 2023
58b1d4c
reorganize shared but unpublished code
andrewparmet Jan 2, 2024
e28dd17
lint
andrewparmet Jan 2, 2024
3659cb2
move over what is needed to infer protobuf type names
andrewparmet Jan 2, 2024
cd757ca
map value well known wrapper types do not work correctly
andrewparmet Jan 2, 2024
21d9534
maps work
andrewparmet Jan 2, 2024
283d20d
compilation
andrewparmet Jan 2, 2024
eca1983
more tests
andrewparmet Jan 2, 2024
8e96790
api dump
andrewparmet Jan 2, 2024
f980afa
fix field presence lookup
andrewparmet Jan 3, 2024
1036d0e
api dump
andrewparmet Jan 3, 2024
b2b0e1a
more api cleanup
andrewparmet Jan 3, 2024
786ee1d
fix map wrapper types
andrewparmet Jan 3, 2024
9d426aa
some cleanup
andrewparmet Jan 3, 2024
5f6a280
mostly clean up map wrapper types
andrewparmet Jan 3, 2024
3689bc8
isolate more and get tests passing
andrewparmet Jan 3, 2024
c783115
fix
andrewparmet Jan 3, 2024
437909a
fix tags in codegen for large field numbers
andrewparmet Jan 4, 2024
d1a4417
api dump
andrewparmet Jan 4, 2024
deddc2d
Merge branch 'fix-tags-for-large-numbers' into implement-converter-to…
andrewparmet Jan 4, 2024
d357e38
change strategy for sharing potentially unpublished code
andrewparmet Jan 4, 2024
e2e0379
lint
andrewparmet Jan 4, 2024
c679e4f
Merge branch 'change-strategy-for-common-unpublished-code' into imple…
andrewparmet Jan 4, 2024
33206b3
fix map wrapper types for well known wrappers
andrewparmet Jan 5, 2024
dbcd52c
Merge branch 'fix-map-wrapper-types' into implement-converter-to-dyna…
andrewparmet Jan 5, 2024
ed9c87b
remove guava
andrewparmet Jan 6, 2024
294fb53
rm guava
andrewparmet Jan 6, 2024
3af2204
rm line
andrewparmet Jan 6, 2024
d150db2
reflect need not be compatible with android
andrewparmet Jan 6, 2024
0e82a08
turns out this line was needed
andrewparmet Jan 6, 2024
301a9e2
Update build.gradle.kts
andrewparmet Jan 9, 2024
d50dd74
Merge branch 'main' into implement-converter-to-dynamic-message
andrewparmet Feb 7, 2024
af1af55
Update FieldTypeExt.kt
andrewparmet Feb 7, 2024
a171c1e
Merge branch 'main' into implement-converter-to-dynamic-message
andrewparmet Mar 17, 2024
ad92a9f
no data objects
andrewparmet Mar 17, 2024
ae1c609
don't allow conversion of null
andrewparmet Apr 22, 2024
dc621f5
Merge branch 'main' into implement-converter-to-dynamic-message
andrewparmet May 8, 2024
27ff3cc
classpath scan only in tests
andrewparmet May 8, 2024
5eaae73
apidump
andrewparmet May 8, 2024
c7e643e
update readme
andrewparmet May 8, 2024
bc8cea8
space
andrewparmet May 8, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
map value well known wrapper types do not work correctly
  • Loading branch information
andrewparmet committed Jan 2, 2024
commit cd757cabcb3b89ac05251944f224c4f278de0750
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import com.squareup.kotlinpoet.asTypeName
import protokt.v1.Collections
import protokt.v1.SizeCodecs
import protokt.v1.codegen.generate.CodeGenerator.Context
import protokt.v1.reflect.inferClassName
import kotlin.reflect.KClass
import kotlin.reflect.KFunction1

Expand Down Expand Up @@ -87,12 +88,4 @@ fun CodeBlock.Builder.endControlFlowWithoutNewline() {

internal fun inferClassName(className: String, ctx: Context) =
inferClassName(className, ctx.info.kotlinPackage)

fun inferClassName(className: String, pkg: String): ClassName {
val inferred = ClassName.bestGuess(className)
return if (inferred.packageName == "") {
ClassName(pkg, className.split("."))
} else {
inferred
}
}
.let { (pkg, names) -> ClassName(pkg, names) }
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ private class OneofGenerator(
val implements =
oneof.options.protokt.implements
.takeIf { it.isNotEmpty() }
?.let { inferClassName(it, ctx.info.kotlinPackage) }
?.let { inferClassName(it, ctx) }

TypeSpec.classBuilder(oneof.name)
.addModifiers(KModifier.SEALED)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import protokt.v1.codegen.util.StandardField
import protokt.v1.reflect.ClassLookup
import protokt.v1.reflect.ConverterDetails
import protokt.v1.reflect.FieldType
import protokt.v1.reflect.inferClassName
import kotlin.reflect.KFunction2

internal object Wrapper {
Expand All @@ -56,8 +57,9 @@ internal object Wrapper {
className.canonicalName,
type,
fieldName
).let(ClassName::bestGuess),
inferClassName(wrap, ctx.kotlinPackage),
),
inferClassName(wrap, ctx.kotlinPackage)
.let { (pkg, names) -> ClassName(pkg, names).canonicalName },
ctx
)
)
Expand Down Expand Up @@ -184,6 +186,6 @@ internal object Wrapper {
ifWrapped
)

private fun converter(protoClassName: ClassName, kotlinClassName: ClassName, ctx: GeneratorContext) =
ctx.classLookup.converter(protoClassName.canonicalName, kotlinClassName.canonicalName)
private fun converter(protoClassName: String, kotlinClassName: String, ctx: GeneratorContext) =
ctx.classLookup.converter(protoClassName, kotlinClassName)
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package protokt.v1.codegen.util

import com.google.protobuf.DescriptorProtos.FileDescriptorProto
import protokt.v1.reflect.PROTOKT_V1
import protokt.v1.reflect.resolvePackage

const val DOT_GOOGLE_PROTOBUF = ".google.protobuf"
val PROTOKT_V1_GOOGLE_PROTO = PROTOKT_V1 + DOT_GOOGLE_PROTOBUF
Expand All @@ -25,8 +26,4 @@ fun packagesByFileName(protoFileList: List<FileDescriptorProto>) =
protoFileList.associate { it.name to resolvePackage(it) }

fun resolvePackage(fdp: FileDescriptorProto) =
if (fdp.`package`.startsWith(PROTOKT_V1)) {
fdp.`package`
} else {
"$PROTOKT_V1." + fdp.`package`
}
resolvePackage(fdp.`package`)
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ import protokt.v1.KtGeneratedMessage
import protokt.v1.KtMessage
import protokt.v1.reflect.ClassLookup
import protokt.v1.reflect.FieldType
import protokt.v1.reflect.inferClassName
import protokt.v1.reflect.resolvePackage
import protokt.v1.reflect.typeName
import kotlin.Any
import kotlin.reflect.KClass
Expand Down Expand Up @@ -64,7 +66,7 @@ class RuntimeContext internal constructor(
else -> value
}

internal fun unwrap(value: Any, field: FieldDescriptor): Any {
internal fun unwrap(value: Any, field: FieldDescriptor, wrap: String): Any {
val proto = field.toProto()
val type = FieldType.from(proto.type)
val converterDetails =
Expand All @@ -75,8 +77,8 @@ class RuntimeContext internal constructor(
type,
field.name
),
// todo: ths is not right; have to infer kotlin name like we do in codegen
value::class.qualifiedName!!
inferClassName(wrap, resolvePackage(field.file.`package`))
.let { (pkg, names) -> pkg + "." + names.joinToString(".") }
)

@Suppress("UNCHECKED_CAST")
Expand Down Expand Up @@ -138,6 +140,8 @@ private fun toDynamicMessage(
.apply {
descriptor.fields.forEach { field ->
ProtoktReflect.getField(message, field)?.let { value ->
val wrap = wrap(field)

setField(
field,
when {
Expand All @@ -154,10 +158,16 @@ private fun toDynamicMessage(

// todo: unwrap elements if wrapped
field.isRepeated ->
(value as List<*>).map(context::convertValue)
(value as List<*>).map {
if (isWrapped(field, wrap)) {
context.convertValue(context.unwrap(it!!, field, wrap!!))
} else {
context.convertValue(it)
}
}

isWrapped(field) ->
context.convertValue(context.unwrap(value, field))
isWrapped(field, wrap) ->
context.convertValue(context.unwrap(value, field, wrap!!))

else -> context.convertValue(value)
},
Expand All @@ -182,11 +192,19 @@ private val WRAPPER_TYPE_NAMES =
"google.protobuf.BytesValue"
)

private fun isWrapped(field: FieldDescriptor): Boolean {
private fun isWrapped(field: FieldDescriptor, wrap: String?): Boolean {
if (field.type == FieldDescriptor.Type.MESSAGE && field.messageType.fullName in WRAPPER_TYPE_NAMES) {
return true
}

return wrap != null
}

private fun wrap(field: FieldDescriptor): String? {
if (field.type == FieldDescriptor.Type.MESSAGE && field.messageType.fullName in WRAPPER_TYPE_NAMES) {
return field.messageType.fullName
}

val options = field.toProto().options

val propertyOptions =
Expand All @@ -199,12 +217,12 @@ private fun isWrapped(field: FieldDescriptor): Boolean {
.last()
)
} else {
return false
return null
}

return propertyOptions.wrap.isNotEmpty() ||
propertyOptions.keyWrap.isNotEmpty() ||
propertyOptions.valueWrap.isNotEmpty()
return propertyOptions.wrap.takeIf { it.isNotEmpty() }
?: propertyOptions.keyWrap.takeIf { it.isNotEmpty() }
?: propertyOptions.valueWrap.takeIf { it.isNotEmpty() }
}

private fun convertMap(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class DynamicMessageTest {
fun `dynamic message serialization`() {
val message = getAllTypesAllSet()

; verifyMessage(message)
verifyMessage(message)
}

@Test
Expand All @@ -52,6 +52,7 @@ class DynamicMessageTest {
optionalUuid = UUID.randomUUID()
// optionalIpAddress = InetAddress.getByName("127.0.0.2")
optionalLocalDate = LocalDate.now().minusDays(2)
optionalString = "foo"
}

verifyMessage(message)
Expand All @@ -70,7 +71,7 @@ class DynamicMessageTest {
verifyMessage(message2)

val message3 =
OneofWrappers { wrappedOneof = OneofWrappers.WrappedOneof.SocketAddressOneof(InetSocketAddress.createUnresolved("127.0.0.1", 2319)) }
OneofWrappers { wrappedOneof = OneofWrappers.WrappedOneof.SocketAddressOneof(InetSocketAddress("127.0.0.1", 2319)) }

verifyMessage(message3)

Expand Down Expand Up @@ -98,6 +99,11 @@ class DynamicMessageTest {
OneofWrappers { wrappedOneof = OneofWrappers.WrappedOneof.NullableLocalDateOneof(LocalDate.now()) }

verifyMessage(message8)

val message9 =
OneofWrappers { wrappedOneof = OneofWrappers.WrappedOneof.OptionalString("foo") }

verifyMessage(message9)
}

@Test
Expand All @@ -106,6 +112,7 @@ class DynamicMessageTest {
RepeatedWrappers {
uuids = listOf(UUID.randomUUID(), UUID.randomUUID())
uuidsWrapped = listOf(UUID.randomUUID(), UUID.randomUUID())
strings = listOf("foo")
}

verifyMessage(message)
Expand All @@ -126,6 +133,10 @@ class DynamicMessageTest {
LocalDate.now() to InetSocketAddress.createUnresolved("127.0.0.1", 2319),
LocalDate.now().minusDays(1) to InetSocketAddress.createUnresolved("127.0.0.1", 2320)
)

// todo: this needs to wrap properly
mapStringStringValue =
mapOf("foo" to StringValue { value = "bar" })
}

verifyMessage(message)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ message Wrappers {
optional string optional_local_date = 14 [
(.protokt.v1.property).wrap = "java.time.LocalDate"
];

google.protobuf.StringValue optional_string = 15;
}

message OneofWrappers {
Expand Down Expand Up @@ -106,6 +108,8 @@ message OneofWrappers {
google.protobuf.StringValue nullable_local_date_oneof = 10 [
(.protokt.v1.property).wrap = "java.time.LocalDate"
];

google.protobuf.StringValue optional_string = 11;
}
}

Expand All @@ -117,6 +121,8 @@ message RepeatedWrappers {
repeated google.protobuf.BytesValue uuids_wrapped = 2 [
(.protokt.v1.property).wrap = "java.util.UUID"
];

repeated google.protobuf.StringValue strings = 3;
}

message MapWrappers {
Expand All @@ -129,4 +135,6 @@ message MapWrappers {
(.protokt.v1.property).key_wrap = "java.time.LocalDate",
(.protokt.v1.property).value_wrap = "java.net.InetSocketAddress"
];

map<string, google.protobuf.StringValue> map_string_string_value = 9;
}
2 changes: 1 addition & 1 deletion unpublished/src/reflect/protokt/v1/reflect/ClassLookup.kt
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ private fun <T : Any> tryDeserializeDefaultValue(converter: Converter<T, *>): Th
return if (protoDefault == null) null else tryWrap(protoDefault as T)
}

class ConverterDetails(
internal class ConverterDetails(
val converter: Converter<*, *>,
val kotlinCanonicalClassName: String,
val optimizedSizeof: Boolean,
Expand Down
38 changes: 38 additions & 0 deletions unpublished/src/reflect/protokt/v1/reflect/ClassNames.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright (c) 2024 Toast, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http:https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package protokt.v1.reflect

internal fun inferClassName(className: String, pkg: String): Pair<String, List<String>> {
val inferred = bestGuessPackageName(className)
return if (inferred == null) {
pkg to className.split(".")
} else {
inferred to className.substringAfter("$inferred.").split(".")
}
}

private fun bestGuessPackageName(classNameString: String): String? {
var p = 0
while (p < classNameString.length && Character.isLowerCase(classNameString.codePointAt(p))) {
p = classNameString.indexOf('.', p) + 1
require(p != 0) { "couldn't make a guess for $classNameString" }
}
return if (p != 0) {
classNameString.substring(0, p - 1)
} else {
null
}
}
7 changes: 7 additions & 0 deletions unpublished/src/reflect/protokt/v1/reflect/PackageNames.kt
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,10 @@ internal fun requalifyProtoType(typeName: String) =
} else {
"$PROTOKT_V1." + typeName.removePrefix(".")
}

internal fun resolvePackage(pkg: String) =
if (pkg.startsWith(PROTOKT_V1)) {
pkg
} else {
"$PROTOKT_V1.$pkg"
}