From 6920b0ef5c38c991fede8fe54361924124d99de2 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 13:03:58 -0400 Subject: [PATCH 01/46] set up protovalidate module --- gradle/libs.versions.toml | 7 + protokt-protovalidate/build.gradle.kts | 27 ++ .../v1/buf/validate/ProtoktValidator.kt | 93 ++++ settings.gradle.kts | 4 +- .../build.gradle.kts | 107 ++++ .../DynamicConcreteMessageDeserializer.kt | 44 ++ .../conformance/FileDescriptorUtil.kt | 51 ++ .../v1/buf/validate/conformance/Main.kt | 93 ++++ .../validate/AbstractProtoktValidatorTest.kt | 459 ++++++++++++++++++ .../validate/ProtoktDynamicMessageTest.kt | 23 + 10 files changed, 907 insertions(+), 1 deletion(-) create mode 100644 protokt-protovalidate/build.gradle.kts create mode 100644 protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt create mode 100644 testing/protovalidate-conformance/build.gradle.kts create mode 100644 testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/DynamicConcreteMessageDeserializer.kt create mode 100644 testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/FileDescriptorUtil.kt create mode 100644 testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt create mode 100644 testing/protovalidate-conformance/src/test/kotlin/validate/AbstractProtoktValidatorTest.kt create mode 100644 testing/protovalidate-conformance/src/test/kotlin/validate/ProtoktDynamicMessageTest.kt diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index c85bc433..0fe1da65 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -13,6 +13,7 @@ [versions] autoService = "1.1.1" +cel = "0.4.4" grpc-java = "1.58.0" grpc-kotlin = "1.4.1" kotlinLogging = "5.1.0" @@ -23,12 +24,15 @@ ktlint = "1.2.1" protobuf-java = "3.21.7" protobuf-js = "7.2.6" protobufGradlePlugin = "0.9.4" +protovalidate = "0.6.4" +protovalidateJava = "0.2.1" slf4j = "2.0.6" # build androidGradlePlugin = "7.4.2" animalSnifferGradlePlugin = "1.7.1" binaryCompatibilityValidator = "0.14.0" +bufGradlePlugin = "0.9.0" buildConfig = "3.1.0" gradleMavenPublishPlugin = "0.25.3" gummyBears = "0.8.0" @@ -54,6 +58,7 @@ truth = "1.4.2" protoGoogleCommonProtos = "2.21.0" [plugins] +bufGradlePlugin = { id = "build.buf", version.ref = "bufGradlePlugin" } buildConfig = { id = "com.github.gmazzo.buildconfig", version.ref = "buildConfig" } download = { id = "de.undercouch.download", version.ref = "download" } pluginPublish = { id = "com.gradle.plugin-publish", version.ref = "pluginPublish" } @@ -62,6 +67,7 @@ wire = { id = "com.squareup.wire", version.ref = "wire" } [libraries] autoService = { module = "com.google.auto.service:auto-service", version.ref = "autoService" } autoServiceAnnotations = { module = "com.google.auto.service:auto-service-annotations", version.ref = "autoService" } +cel = { module = "org.projectnessie.cel:cel-tools", version.ref = "cel" } grpc-kotlin-gen = { module = "io.grpc:protoc-gen-grpc-kotlin", version.ref = "grpc-kotlin" } grpc-netty = { module = "io.grpc:grpc-netty", version.ref = "grpc-java" } grpc-stub = { module = "io.grpc:grpc-stub", version.ref = "grpc-java" } @@ -73,6 +79,7 @@ ktlintRuleSetStandard = { module = "com.pinterest.ktlint:ktlint-ruleset-standard protobuf-gradlePlugin = { module = "com.google.protobuf:protobuf-gradle-plugin", version.ref = "protobufGradlePlugin" } protobuf-java = { module ="com.google.protobuf:protobuf-java", version.ref = "protobuf-java" } protoc = { module = "com.google.protobuf:protoc", version.ref = "protobuf-java" } +protovalidateJava = { module = "build.buf:protovalidate", version.ref = "protovalidateJava" } slf4jSimple = { module = "org.slf4j:slf4j-simple", version.ref = "slf4j" } # build diff --git a/protokt-protovalidate/build.gradle.kts b/protokt-protovalidate/build.gradle.kts new file mode 100644 index 00000000..08bfc2eb --- /dev/null +++ b/protokt-protovalidate/build.gradle.kts @@ -0,0 +1,27 @@ +/* + * 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://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. + */ + +plugins { + id("protokt.jvm-conventions") +} + +enablePublishing() + +dependencies { + implementation(project(":protokt-reflect")) + implementation(kotlin("reflect")) + implementation(libs.cel) + implementation(libs.protovalidateJava) +} diff --git a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt new file mode 100644 index 00000000..6fb0aaef --- /dev/null +++ b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt @@ -0,0 +1,93 @@ +/* + * 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://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.buf.validate + +import build.buf.protovalidate.Config +import build.buf.protovalidate.ValidationResult +import build.buf.protovalidate.internal.celext.ValidateLibrary +import build.buf.protovalidate.internal.evaluator.Evaluator +import build.buf.protovalidate.internal.evaluator.EvaluatorBuilder +import build.buf.protovalidate.internal.evaluator.MessageValue +import com.google.protobuf.DescriptorProtos +import com.google.protobuf.Descriptors +import com.google.protobuf.Descriptors.Descriptor +import org.projectnessie.cel.Env +import org.projectnessie.cel.Library +import protokt.v1.Beta +import protokt.v1.GeneratedMessage +import protokt.v1.Message +import protokt.v1.google.protobuf.FileDescriptor +import protokt.v1.google.protobuf.RuntimeContext +import protokt.v1.google.protobuf.toDynamicMessage +import java.util.Collections +import java.util.concurrent.ConcurrentHashMap +import kotlin.reflect.full.findAnnotation + +@Beta +class ProtoktValidator( + config: Config = Config.newBuilder().build() +) { + private val evaluatorBuilder = + EvaluatorBuilder( + Env.newEnv(Library.Lib(ValidateLibrary())), + config.isDisableLazy + ) + + private val failFast = config.isFailFast + + private val evaluatorsByFullTypeName = ConcurrentHashMap() + private val descriptors = Collections.synchronizedList(mutableListOf()) + + fun load(descriptor: FileDescriptor) { + descriptor + .toProtobufJavaDescriptor() + .messageTypes + .forEach(::load) + } + + fun load( + descriptor: Descriptor, + message: Message? = null, + ) { + descriptors.add(descriptor) + try { + evaluatorsByFullTypeName[descriptor.fullName] = evaluatorBuilder.load(descriptor) + } catch (ex: Exception) { + // idiosyncrasy of the conformance suite runner requires this particular exception is rethrown rather than a lookup failure later + if (message != null) { + if (message::class.findAnnotation()!!.fullTypeName == descriptor.fullName) { + throw ex + } + } + } + descriptor.nestedTypes.forEach { load(it, message) } + } + + private fun FileDescriptor.toProtobufJavaDescriptor(): Descriptors.FileDescriptor = + Descriptors.FileDescriptor.buildFrom( + DescriptorProtos.FileDescriptorProto.parseFrom(proto.serialize()), + dependencies.map { it.toProtobufJavaDescriptor() }.toTypedArray(), + true + ) + + fun validate(message: Message): ValidationResult = + evaluatorsByFullTypeName.getValue( + message::class.findAnnotation()!!.fullTypeName, + ).evaluate( + MessageValue(message.toDynamicMessage(RuntimeContext(descriptors))), + failFast + ) +} diff --git a/settings.gradle.kts b/settings.gradle.kts index 8a8bc1c6..7ec56b81 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -31,11 +31,12 @@ include( "protokt-codegen", "protokt-core", "protokt-core-lite", + "protokt-gradle-plugin", + "protokt-protovalidate", "protokt-reflect", "protokt-runtime", "protokt-runtime-grpc", "protokt-runtime-grpc-lite", - "protokt-gradle-plugin", "grpc-kotlin-shim", @@ -70,6 +71,7 @@ include( "testing:protokt-generation", "testing:protokt-generation-2", "testing:protobuf-java", + "testing:protovalidate-conformance", "testing:protobufjs", "testing:testing-util", diff --git a/testing/protovalidate-conformance/build.gradle.kts b/testing/protovalidate-conformance/build.gradle.kts new file mode 100644 index 00000000..6a253065 --- /dev/null +++ b/testing/protovalidate-conformance/build.gradle.kts @@ -0,0 +1,107 @@ +/* + * 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://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. + */ + +import build.buf.gradle.BUF_BINARY_CONFIGURATION_NAME +import build.buf.gradle.BUF_BUILD_DIR +import build.buf.gradle.FormatCheckTask +import build.buf.gradle.LintTask +import com.android.build.gradle.internal.tasks.factory.dependsOn +import com.google.protobuf.gradle.GenerateProtoTask +import com.google.protobuf.gradle.proto +import org.gradle.api.distribution.plugins.DistributionPlugin.TASK_INSTALL_NAME +import org.gradle.language.base.plugins.LifecycleBasePlugin.CHECK_TASK_NAME + +plugins { + id("protokt.jvm-conventions") + alias(libs.plugins.bufGradlePlugin) + application +} + +localProtokt(false) + +dependencies { + implementation(project(":protokt-protovalidate")) + implementation(project(":protokt-reflect")) + implementation(kotlin("reflect")) + implementation(libs.cel) + implementation(libs.classgraph) + implementation(libs.protovalidateJava) + + testImplementation(libs.truth) +} + +val protovalidateVersion = libs.versions.protovalidate.get() + +val downloadConformanceProtos = + tasks.register("downloadConformanceProtos") { + commandLine( + configurations.getByName(BUF_BINARY_CONFIGURATION_NAME).singleFile.absolutePath, + "export", + "buf.build/bufbuild/protovalidate-testing:v$protovalidateVersion", + "--output=build/$BUF_BUILD_DIR/export" + ) + } + +tasks.withType { + dependsOn(downloadConformanceProtos) +} + +sourceSets.main { + proto { + srcDir(project.layout.buildDirectory.file("$BUF_BUILD_DIR/export")) + } +} + +val conformanceExecutable = project.layout.buildDirectory.file("gobin/protovalidate-conformance").get().asFile + +val installConformance = + tasks.register("installProtovalidateConformance") { + environment("GOBIN", project.layout.buildDirectory.file("gobin").get().asFile.absolutePath) + outputs.file(conformanceExecutable) + commandLine( + "go", + "install", + "github.com/bufbuild/protovalidate/tools/protovalidate-conformance@v$protovalidateVersion" + ) + } + +application { + mainClass.set("protokt.v1.buf.validate.conformance.Main") +} + +val conformance = + tasks.register("conformance") { + dependsOn(TASK_INSTALL_NAME, installConformance) + commandLine( + conformanceExecutable.absolutePath, + "--strict_message", + "--strict_error", + project.layout.buildDirectory + .file("install/protovalidate-conformance/bin/protovalidate-conformance") + .get() + .asFile + .absolutePath + ) + } + +tasks.named(CHECK_TASK_NAME).dependsOn(conformance) + +tasks.withType { + enabled = false +} + +tasks.withType { + enabled = false +} diff --git a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/DynamicConcreteMessageDeserializer.kt b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/DynamicConcreteMessageDeserializer.kt new file mode 100644 index 00000000..3bcc4b87 --- /dev/null +++ b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/DynamicConcreteMessageDeserializer.kt @@ -0,0 +1,44 @@ +/* + * 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://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.buf.validate.conformance + +import io.github.classgraph.ClassGraph +import protokt.v1.Deserializer +import protokt.v1.GeneratedMessage +import protokt.v1.google.protobuf.Empty +import java.io.InputStream +import kotlin.reflect.full.findAnnotation + +object DynamicConcreteMessageDeserializer { + private val deserializersByFullTypeName: Map> by lazy { + ClassGraph() + .enableAllInfo() + .scan() + .getClassesWithAnnotation(GeneratedMessage::class.java) + .asSequence() + .map { it.loadClass().kotlin } + .associate { messageClass -> + messageClass.findAnnotation()!!.fullTypeName to + messageClass + .nestedClasses + .single { it.simpleName == Empty.Deserializer::class.simpleName } + .objectInstance as Deserializer<*> + } + } + + fun parse(fullTypeName: String, bytes: InputStream) = + deserializersByFullTypeName.getValue(fullTypeName).deserialize(bytes) +} diff --git a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/FileDescriptorUtil.kt b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/FileDescriptorUtil.kt new file mode 100644 index 00000000..9803d6e4 --- /dev/null +++ b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/FileDescriptorUtil.kt @@ -0,0 +1,51 @@ +/* + * 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://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.buf.validate.conformance + +import com.google.protobuf.DescriptorProtos.FileDescriptorProto +import com.google.protobuf.DescriptorProtos.FileDescriptorSet +import com.google.protobuf.Descriptors.Descriptor +import com.google.protobuf.Descriptors.FileDescriptor + +fun parse(fileDescriptorSet: FileDescriptorSet): Map = + parseFileDescriptors(fileDescriptorSet) + .values + .flatMap { fileDescriptor -> + fileDescriptor.messageTypes.map { messageType -> + messageType.fullName to messageType + } + } + .toMap() + +private fun parseFileDescriptors(fileDescriptorSet: FileDescriptorSet): Map { + val fileDescriptorProtoMap = mutableMapOf() + for (fileDescriptorProto in fileDescriptorSet.fileList) { + if (fileDescriptorProto.getName() in fileDescriptorProtoMap) { + error("duplicate files found.") + } + fileDescriptorProtoMap[fileDescriptorProto.getName()] = fileDescriptorProto + } + + return fileDescriptorSet.fileList.fold(mutableMapOf()) { map, fileDescriptorProto -> + map[fileDescriptorProto.getName()] = + FileDescriptor.buildFrom( + fileDescriptorProto, + fileDescriptorProto.dependencyList.mapNotNull(map::get).toTypedArray(), + false + ) + map + } +} diff --git a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt new file mode 100644 index 00000000..a78a0191 --- /dev/null +++ b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt @@ -0,0 +1,93 @@ +/* + * 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://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.buf.validate.conformance + +import buf.validate.conformance.harness.Harness.TestConformanceRequest +import buf.validate.conformance.harness.Harness.TestConformanceResponse +import buf.validate.conformance.harness.Harness.TestResult +import build.buf.protovalidate.exceptions.CompilationException +import build.buf.protovalidate.exceptions.ExecutionException +import build.buf.validate.ValidateProto +import build.buf.validate.Violations +import com.google.protobuf.Descriptors +import com.google.protobuf.ExtensionRegistry +import protokt.v1.Message +import protokt.v1.buf.validate.ProtoktValidator + +object Main { + @JvmStatic + fun main(args: Array) { + val extensionRegistry = ExtensionRegistry.newInstance() + extensionRegistry.add(ValidateProto.message) + extensionRegistry.add(ValidateProto.field) + extensionRegistry.add(ValidateProto.oneof) + val request = TestConformanceRequest.parseFrom(System.`in`, extensionRegistry) + val response = testConformance(request) + response.writeTo(System.out) + } + + private fun testConformance(request: TestConformanceRequest): TestConformanceResponse { + val descriptorMap = parse(request.fdset) + val validator = ProtoktValidator() + val responseBuilder = TestConformanceResponse.newBuilder() + responseBuilder.putAllResults( + request.casesMap.mapValues { (_, value) -> + testCase(validator, descriptorMap, value) + } + ) + return responseBuilder.build() + } + + private fun testCase( + validator: ProtoktValidator, + fileDescriptors: Map, + testCase: com.google.protobuf.Any + ): TestResult { + val urlParts = testCase.typeUrl.split('/', limit = 2) + val fullName = urlParts[urlParts.size - 1] + fileDescriptors[fullName] ?: return unexpectedErrorResult("Unable to find descriptor: %s", fullName) + val testCaseValue = testCase.value + val message = DynamicConcreteMessageDeserializer.parse(fullName, testCaseValue.newInput()) + return validate(validator, message, fileDescriptors.values) + } + + private fun validate( + validator: ProtoktValidator, + message: Message, + descriptors: Iterable + ): TestResult { + try { + descriptors.forEach { validator.load(it, message) } + val result = validator.validate(message) + if (result.isSuccess) { + return TestResult.newBuilder().setSuccess(true).build() + } + val error = Violations.newBuilder().addAllViolations(result.violations).build() + return TestResult.newBuilder().setValidationError(error).build() + } catch (e: CompilationException) { + return TestResult.newBuilder().setCompilationError(e.message).build() + } catch (e: ExecutionException) { + return TestResult.newBuilder().setRuntimeError(e.message).build() + } catch (e: Exception) { + return unexpectedErrorResult("unknown error: %s", e.toString()) + } + } + + private fun unexpectedErrorResult(format: String?, vararg args: Any?): TestResult { + val errorMessage = String.format(format!!, *args) + return TestResult.newBuilder().setUnexpectedError(errorMessage).build() + } +} diff --git a/testing/protovalidate-conformance/src/test/kotlin/validate/AbstractProtoktValidatorTest.kt b/testing/protovalidate-conformance/src/test/kotlin/validate/AbstractProtoktValidatorTest.kt new file mode 100644 index 00000000..ea7eceb4 --- /dev/null +++ b/testing/protovalidate-conformance/src/test/kotlin/validate/AbstractProtoktValidatorTest.kt @@ -0,0 +1,459 @@ +/* + * 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://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 validate + +import buf.validate.conformance.cases.Bytes +import buf.validate.conformance.cases.Numbers +import buf.validate.conformance.cases.Repeated +import buf.validate.conformance.cases.Strings +import build.buf.protovalidate.ValidationResult +import com.google.common.truth.Truth.assertThat +import com.google.protobuf.ByteString +import org.junit.jupiter.api.Test +import protokt.v1.AbstractDeserializer +import protokt.v1.AbstractMessage +import protokt.v1.GeneratedMessage +import protokt.v1.Message +import protokt.v1.Reader +import protokt.v1.UnknownFieldSet +import protokt.v1.Writer +import protokt.v1.buf.validate.ProtoktValidator +import protokt.v1.buf.validate.conformance.cases.MessageRequiredOneof +import protokt.v1.buf.validate.conformance.cases.Oneof +import protokt.v1.buf.validate.conformance.cases.TestMsg +import protokt.v1.buf.validate.conformance.cases.UInt64In +import protokt.v1.buf.validate.conformance.cases.bytes_file_descriptor +import protokt.v1.buf.validate.conformance.cases.messages_file_descriptor +import protokt.v1.buf.validate.conformance.cases.numbers_file_descriptor +import protokt.v1.buf.validate.conformance.cases.oneofs_file_descriptor +import protokt.v1.buf.validate.conformance.cases.repeated_file_descriptor +import protokt.v1.buf.validate.conformance.cases.strings_file_descriptor + +abstract class AbstractProtoktValidatorTest { + protected val validator = ProtoktValidator() + + abstract fun validate(message: Message): ValidationResult + + @Test + fun `test required oneof constraint`() { + validator.load(messages_file_descriptor.descriptor) + + val result = + validate( + MessageRequiredOneof { + one = + MessageRequiredOneof.One.Val( + TestMsg { + const = "foo" + }, + ) + }, + ) + + assertThat(result.violations).isEmpty() + assertThat(result.isSuccess).isTrue() + } + + @Test + fun `test oneof constraint`() { + validator.load(oneofs_file_descriptor.descriptor) + + val result = + validate( + Oneof { + o = Oneof.O.X("foobar") + }, + ) + + assertThat(result.violations).isEmpty() + assertThat(result.isSuccess).isTrue() + } + + @Test + fun `test uint64 in constraint`() { + validator.load(numbers_file_descriptor.descriptor) + + val result = + validate( + UInt64In { + `val` = 4u + }, + ) + + assertThat(result.isSuccess).isFalse() + } + + @Test + fun `test message with varint non-uint64 encoded purely as unknown fields (dynamic message without a dedicated type)`() { + validator.load(numbers_file_descriptor.descriptor) + + val result = + validate( + Int64.deserialize( + Numbers.Int64In + .newBuilder() + .setVal(4) + .build() + .toByteArray(), + ), + ) + + assertThat(result.isSuccess).isFalse() + + val result2 = + validate( + Int64.deserialize( + Numbers.Int64In + .newBuilder() + .setVal(3) + .build() + .toByteArray(), + ), + ) + + assertThat(result2.isSuccess).isTrue() + } + + @Test + fun `test message with varint uint64 encoded purely as unknown fields (dynamic message without a dedicated type)`() { + validator.load(numbers_file_descriptor.descriptor) + + val result = + validate( + UInt64.deserialize( + Numbers.UInt64In + .newBuilder() + .setVal(4) + .build() + .toByteArray(), + ), + ) + + assertThat(result.isSuccess).isFalse() + + val result2 = + validate( + UInt64.deserialize( + Numbers.UInt64In + .newBuilder() + .setVal(3) + .build() + .toByteArray(), + ), + ) + + assertThat(result2.isSuccess).isTrue() + } + + @Test + fun `test message with fixed32 encoded purely as unknown fields (dynamic message without a dedicated type)`() { + validator.load(numbers_file_descriptor.descriptor) + + val result = + validate( + Fixed32.deserialize( + Numbers.Fixed32In + .newBuilder() + .setVal(4) + .build() + .toByteArray(), + ), + ) + + assertThat(result.isSuccess).isFalse() + + val result2 = + validate( + Fixed32.deserialize( + Numbers.Fixed32In + .newBuilder() + .setVal(3) + .build() + .toByteArray(), + ), + ) + + assertThat(result2.isSuccess).isTrue() + } + + @Test + fun `test message with fixed64 encoded purely as unknown fields (dynamic message without a dedicated type)`() { + validator.load(numbers_file_descriptor.descriptor) + + val result = + validate( + Fixed64.deserialize( + Numbers.Fixed64In + .newBuilder() + .setVal(4) + .build() + .toByteArray(), + ), + ) + + assertThat(result.isSuccess).isFalse() + + val result2 = + validate( + Fixed64.deserialize( + Numbers.Fixed64In + .newBuilder() + .setVal(3) + .build() + .toByteArray(), + ), + ) + + assertThat(result2.isSuccess).isTrue() + } + + @Test + fun `test message with length delimited string encoded purely as unknown fields (dynamic message without a dedicated type)`() { + validator.load(strings_file_descriptor.descriptor) + + val result = + validate( + LengthDelimitedString.deserialize( + Strings.StringIn + .newBuilder() + .setVal("foo") + .build() + .toByteArray(), + ), + ) + + assertThat(result.isSuccess).isFalse() + + val result2 = + validate( + LengthDelimitedString.deserialize( + Strings.StringIn + .newBuilder() + .setVal("bar") + .build() + .toByteArray(), + ), + ) + + assertThat(result2.isSuccess).isTrue() + } + + @Test + fun `test message with length delimited bytes encoded purely as unknown fields (dynamic message without a dedicated type)`() { + validator.load(bytes_file_descriptor.descriptor) + + val result = + validate( + LengthDelimitedBytes.deserialize( + Bytes.BytesIn + .newBuilder() + .setVal(ByteString.copyFromUtf8("foo")) + .build() + .toByteArray(), + ), + ) + + assertThat(result.isSuccess).isFalse() + + val result2 = + validate( + LengthDelimitedBytes.deserialize( + Bytes.BytesIn + .newBuilder() + .setVal(ByteString.copyFromUtf8("bar")) + .build() + .toByteArray(), + ), + ) + + assertThat(result2.isSuccess).isTrue() + } + + @Test + fun `test message with repeated values encoded purely as unknown fields (dynamic message without a dedicated type)`() { + validator.load(repeated_file_descriptor.descriptor) + + val result = + validate( + RepeatedLengthDelimited.deserialize( + Repeated.RepeatedUnique + .newBuilder() + .addAllVal(listOf("foo", "foo")) + .build() + .toByteArray(), + ), + ) + + assertThat(result.isSuccess).isFalse() + + val result2 = + validate( + RepeatedLengthDelimited.deserialize( + Repeated.RepeatedUnique + .newBuilder() + .addAllVal(listOf("foo", "bar")) + .build() + .toByteArray(), + ), + ) + + assertThat(result2.isSuccess).isTrue() + } + + abstract class AbstractDynamicMessage : AbstractMessage() { + abstract val unknownFields: UnknownFieldSet + + override fun messageSize() = + unknownFields.size() + + override fun serialize(writer: Writer) { + writer.writeUnknown(unknownFields) + } + } + + @GeneratedMessage("buf.validate.conformance.cases.Int64In") + class Int64( + override val unknownFields: UnknownFieldSet, + ) : AbstractDynamicMessage() { + companion object : AbstractDeserializer() { + @JvmStatic + override fun deserialize(reader: Reader): Int64 { + val unknownFields = UnknownFieldSet.Builder() + + while (true) { + when (reader.readTag()) { + 0u -> return Int64(UnknownFieldSet.from(unknownFields)) + else -> unknownFields.add(reader.readUnknown()) + } + } + } + } + } + + @GeneratedMessage("buf.validate.conformance.cases.UInt64In") + class UInt64( + override val unknownFields: UnknownFieldSet, + ) : AbstractDynamicMessage() { + companion object : AbstractDeserializer() { + @JvmStatic + override fun deserialize(reader: Reader): UInt64 { + val unknownFields = UnknownFieldSet.Builder() + + while (true) { + when (reader.readTag()) { + 0u -> return UInt64(UnknownFieldSet.from(unknownFields)) + else -> unknownFields.add(reader.readUnknown()) + } + } + } + } + } + + @GeneratedMessage("buf.validate.conformance.cases.Fixed32In") + class Fixed32( + override val unknownFields: UnknownFieldSet, + ) : AbstractDynamicMessage() { + companion object : AbstractDeserializer() { + @JvmStatic + override fun deserialize(reader: Reader): Fixed32 { + val unknownFields = UnknownFieldSet.Builder() + + while (true) { + when (reader.readTag()) { + 0u -> return Fixed32(UnknownFieldSet.from(unknownFields)) + else -> unknownFields.add(reader.readUnknown()) + } + } + } + } + } + + @GeneratedMessage("buf.validate.conformance.cases.Fixed64In") + class Fixed64( + override val unknownFields: UnknownFieldSet, + ) : AbstractDynamicMessage() { + companion object : AbstractDeserializer() { + @JvmStatic + override fun deserialize(reader: Reader): Fixed64 { + val unknownFields = UnknownFieldSet.Builder() + + while (true) { + when (reader.readTag()) { + 0u -> return Fixed64(UnknownFieldSet.from(unknownFields)) + else -> unknownFields.add(reader.readUnknown()) + } + } + } + } + } + + @GeneratedMessage("buf.validate.conformance.cases.StringIn") + class LengthDelimitedString( + override val unknownFields: UnknownFieldSet, + ) : AbstractDynamicMessage() { + companion object : AbstractDeserializer() { + @JvmStatic + override fun deserialize(reader: Reader): LengthDelimitedString { + val unknownFields = UnknownFieldSet.Builder() + + while (true) { + when (reader.readTag()) { + 0u -> return LengthDelimitedString(UnknownFieldSet.from(unknownFields)) + else -> unknownFields.add(reader.readUnknown()) + } + } + } + } + } + + @GeneratedMessage("buf.validate.conformance.cases.BytesIn") + class LengthDelimitedBytes( + override val unknownFields: UnknownFieldSet, + ) : AbstractDynamicMessage() { + companion object : AbstractDeserializer() { + @JvmStatic + override fun deserialize(reader: Reader): LengthDelimitedBytes { + val unknownFields = UnknownFieldSet.Builder() + + while (true) { + when (reader.readTag()) { + 0u -> return LengthDelimitedBytes(UnknownFieldSet.from(unknownFields)) + else -> unknownFields.add(reader.readUnknown()) + } + } + } + } + } + + @GeneratedMessage("buf.validate.conformance.cases.RepeatedUnique") + class RepeatedLengthDelimited( + override val unknownFields: UnknownFieldSet, + ) : AbstractDynamicMessage() { + companion object : AbstractDeserializer() { + @JvmStatic + override fun deserialize(reader: Reader): RepeatedLengthDelimited { + val unknownFields = UnknownFieldSet.Builder() + + while (true) { + when (reader.readTag()) { + 0u -> return RepeatedLengthDelimited(UnknownFieldSet.from(unknownFields)) + else -> unknownFields.add(reader.readUnknown()) + } + } + } + } + } +} diff --git a/testing/protovalidate-conformance/src/test/kotlin/validate/ProtoktDynamicMessageTest.kt b/testing/protovalidate-conformance/src/test/kotlin/validate/ProtoktDynamicMessageTest.kt new file mode 100644 index 00000000..0cf9ed76 --- /dev/null +++ b/testing/protovalidate-conformance/src/test/kotlin/validate/ProtoktDynamicMessageTest.kt @@ -0,0 +1,23 @@ +/* + * 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://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 validate + +import protokt.v1.Message + +class ProtoktDynamicMessageTest : AbstractProtoktValidatorTest() { + override fun validate(message: Message) = + validator.validate(message) +} From 4ba095031a5410257c38d4970df69b2336e59c0e Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 13:42:49 -0400 Subject: [PATCH 02/46] remove unneeded code --- .../buf/validate/conformance/FileDescriptorUtil.kt | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/FileDescriptorUtil.kt b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/FileDescriptorUtil.kt index 9803d6e4..4ad6a844 100644 --- a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/FileDescriptorUtil.kt +++ b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/FileDescriptorUtil.kt @@ -15,7 +15,6 @@ package protokt.v1.buf.validate.conformance -import com.google.protobuf.DescriptorProtos.FileDescriptorProto import com.google.protobuf.DescriptorProtos.FileDescriptorSet import com.google.protobuf.Descriptors.Descriptor import com.google.protobuf.Descriptors.FileDescriptor @@ -30,16 +29,8 @@ fun parse(fileDescriptorSet: FileDescriptorSet): Map = } .toMap() -private fun parseFileDescriptors(fileDescriptorSet: FileDescriptorSet): Map { - val fileDescriptorProtoMap = mutableMapOf() - for (fileDescriptorProto in fileDescriptorSet.fileList) { - if (fileDescriptorProto.getName() in fileDescriptorProtoMap) { - error("duplicate files found.") - } - fileDescriptorProtoMap[fileDescriptorProto.getName()] = fileDescriptorProto - } - - return fileDescriptorSet.fileList.fold(mutableMapOf()) { map, fileDescriptorProto -> +private fun parseFileDescriptors(fileDescriptorSet: FileDescriptorSet): Map = + fileDescriptorSet.fileList.fold(mutableMapOf()) { map, fileDescriptorProto -> map[fileDescriptorProto.getName()] = FileDescriptor.buildFrom( fileDescriptorProto, @@ -48,4 +39,3 @@ private fun parseFileDescriptors(fileDescriptorSet: FileDescriptorSet): Map Date: Sun, 12 May 2024 13:56:28 -0400 Subject: [PATCH 03/46] just use go --- gradle/libs.versions.toml | 2 - .../build.gradle.kts | 44 +++++++++---------- 2 files changed, 20 insertions(+), 26 deletions(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 0fe1da65..c25bb769 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -32,7 +32,6 @@ slf4j = "2.0.6" androidGradlePlugin = "7.4.2" animalSnifferGradlePlugin = "1.7.1" binaryCompatibilityValidator = "0.14.0" -bufGradlePlugin = "0.9.0" buildConfig = "3.1.0" gradleMavenPublishPlugin = "0.25.3" gummyBears = "0.8.0" @@ -58,7 +57,6 @@ truth = "1.4.2" protoGoogleCommonProtos = "2.21.0" [plugins] -bufGradlePlugin = { id = "build.buf", version.ref = "bufGradlePlugin" } buildConfig = { id = "com.github.gmazzo.buildconfig", version.ref = "buildConfig" } download = { id = "de.undercouch.download", version.ref = "download" } pluginPublish = { id = "com.gradle.plugin-publish", version.ref = "pluginPublish" } diff --git a/testing/protovalidate-conformance/build.gradle.kts b/testing/protovalidate-conformance/build.gradle.kts index 6a253065..e867c297 100644 --- a/testing/protovalidate-conformance/build.gradle.kts +++ b/testing/protovalidate-conformance/build.gradle.kts @@ -13,10 +13,6 @@ * limitations under the License. */ -import build.buf.gradle.BUF_BINARY_CONFIGURATION_NAME -import build.buf.gradle.BUF_BUILD_DIR -import build.buf.gradle.FormatCheckTask -import build.buf.gradle.LintTask import com.android.build.gradle.internal.tasks.factory.dependsOn import com.google.protobuf.gradle.GenerateProtoTask import com.google.protobuf.gradle.proto @@ -25,7 +21,6 @@ import org.gradle.language.base.plugins.LifecycleBasePlugin.CHECK_TASK_NAME plugins { id("protokt.jvm-conventions") - alias(libs.plugins.bufGradlePlugin) application } @@ -42,15 +37,32 @@ dependencies { testImplementation(libs.truth) } +sourceSets.main { + proto { + srcDir(project.layout.buildDirectory.file("protovalidate/export")) + } +} + val protovalidateVersion = libs.versions.protovalidate.get() +val gobin = project.layout.buildDirectory.file("gobin").get().asFile.absolutePath +val bufExecutable = project.layout.buildDirectory.file("gobin/buf").get().asFile +val conformanceExecutable = project.layout.buildDirectory.file("gobin/protovalidate-conformance").get().asFile + +val installBuf = + tasks.register("installBuf") { + environment("GOBIN", gobin) + outputs.file(bufExecutable) + commandLine("go", "install", "github.com/bufbuild/buf/cmd/buf@latest") + } val downloadConformanceProtos = tasks.register("downloadConformanceProtos") { + dependsOn(installBuf) commandLine( - configurations.getByName(BUF_BINARY_CONFIGURATION_NAME).singleFile.absolutePath, + bufExecutable, "export", "buf.build/bufbuild/protovalidate-testing:v$protovalidateVersion", - "--output=build/$BUF_BUILD_DIR/export" + "--output=build/protovalidate/export" ) } @@ -58,17 +70,9 @@ tasks.withType { dependsOn(downloadConformanceProtos) } -sourceSets.main { - proto { - srcDir(project.layout.buildDirectory.file("$BUF_BUILD_DIR/export")) - } -} - -val conformanceExecutable = project.layout.buildDirectory.file("gobin/protovalidate-conformance").get().asFile - val installConformance = tasks.register("installProtovalidateConformance") { - environment("GOBIN", project.layout.buildDirectory.file("gobin").get().asFile.absolutePath) + environment("GOBIN", gobin) outputs.file(conformanceExecutable) commandLine( "go", @@ -97,11 +101,3 @@ val conformance = } tasks.named(CHECK_TASK_NAME).dependsOn(conformance) - -tasks.withType { - enabled = false -} - -tasks.withType { - enabled = false -} From df2e8dbde592a0edb4da944db261f81c541ce996 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 13:58:18 -0400 Subject: [PATCH 04/46] extract buf version --- gradle/libs.versions.toml | 1 + testing/protovalidate-conformance/build.gradle.kts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index c25bb769..ac9dff01 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -47,6 +47,7 @@ jmh = "1.37" wire = "4.9.7" # test +buf = "1.31.0" classgraph = "4.8.153" grpc-js = "1.8.14" jackson = "2.17.0" diff --git a/testing/protovalidate-conformance/build.gradle.kts b/testing/protovalidate-conformance/build.gradle.kts index e867c297..f0cd7eee 100644 --- a/testing/protovalidate-conformance/build.gradle.kts +++ b/testing/protovalidate-conformance/build.gradle.kts @@ -52,7 +52,7 @@ val installBuf = tasks.register("installBuf") { environment("GOBIN", gobin) outputs.file(bufExecutable) - commandLine("go", "install", "github.com/bufbuild/buf/cmd/buf@latest") + commandLine("go", "install", "github.com/bufbuild/buf/cmd/buf@v${libs.versions.buf.get()}") } val downloadConformanceProtos = From 0cd3157ccf84b8e79485d892bfdc1a7d748e8900 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 14:56:40 -0400 Subject: [PATCH 05/46] enable lazy conversion --- .../kotlin/JavaBasedProjectConventions.kt | 1 + gradle/libs.versions.toml | 2 +- protokt-protovalidate/build.gradle.kts | 1 + .../protokt/v1/buf/validate/ProtoktImpl.kt | 86 +++++++++++++++++++ .../v1/buf/validate/ProtoktValidator.kt | 9 +- 5 files changed, 96 insertions(+), 3 deletions(-) create mode 100644 protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktImpl.kt diff --git a/buildSrc/src/main/kotlin/JavaBasedProjectConventions.kt b/buildSrc/src/main/kotlin/JavaBasedProjectConventions.kt index 3a04edc3..7daad3c4 100644 --- a/buildSrc/src/main/kotlin/JavaBasedProjectConventions.kt +++ b/buildSrc/src/main/kotlin/JavaBasedProjectConventions.kt @@ -27,6 +27,7 @@ import org.jetbrains.kotlin.gradle.tasks.KotlinCompile fun Project.javaBasedProjectConventions() { repositories { mavenCentral() + mavenLocal() } dependencies { diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index ac9dff01..76252cf9 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -25,7 +25,7 @@ protobuf-java = "3.21.7" protobuf-js = "7.2.6" protobufGradlePlugin = "0.9.4" protovalidate = "0.6.4" -protovalidateJava = "0.2.1" +protovalidateJava = "0.0.0-SNAPSHOT" slf4j = "2.0.6" # build diff --git a/protokt-protovalidate/build.gradle.kts b/protokt-protovalidate/build.gradle.kts index 08bfc2eb..c96fd7ec 100644 --- a/protokt-protovalidate/build.gradle.kts +++ b/protokt-protovalidate/build.gradle.kts @@ -18,6 +18,7 @@ plugins { } enablePublishing() +trackKotlinApiCompatibility() dependencies { implementation(project(":protokt-reflect")) diff --git a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktImpl.kt b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktImpl.kt new file mode 100644 index 00000000..ed09ec70 --- /dev/null +++ b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktImpl.kt @@ -0,0 +1,86 @@ +package protokt.v1.buf.validate + +import build.buf.protovalidate.internal.evaluator.MessageLike +import build.buf.protovalidate.internal.evaluator.Value +import com.google.protobuf.Descriptors.FieldDescriptor +import protokt.v1.Bytes +import protokt.v1.Enum +import protokt.v1.Message +import protokt.v1.google.protobuf.RuntimeContext +import protokt.v1.google.protobuf.getField +import protokt.v1.google.protobuf.hasField + +internal class ProtoktMessageLike( + val message: Message, + val context: RuntimeContext, +) : MessageLike { + override fun hasField(field: FieldDescriptor) = + message.hasField(field) + + override fun getField(field: FieldDescriptor) = + ProtoktObjectValue( + field, + message.getField(field)!!, + context + ) +} + +internal class ProtoktMessageValue( + private val message: Message, + private val context: RuntimeContext, +) : Value { + override fun messageValue() = + ProtoktMessageLike(message, context) + + override fun repeatedValue() = + emptyList() + + override fun mapValue() = + emptyMap() + + override fun celValue() = + context.convertValue(message) + + override fun jvmValue(clazz: Class) = + null +} + +internal class ProtoktObjectValue( + private val fieldDescriptor: FieldDescriptor, + private val value: Any, + private val context: RuntimeContext, +) : Value { + override fun messageValue(): MessageLike = + ProtoktMessageLike(value as Message, context) + + override fun repeatedValue() = + (value as List<*>).map { ProtoktObjectValue(fieldDescriptor, it!!, context) } + + override fun mapValue(): Map { + val input = value as Map<*, *> + + val keyDesc = fieldDescriptor.messageType.findFieldByNumber(1) + val valDesc = fieldDescriptor.messageType.findFieldByNumber(2) + + return input.entries.associate { (key, value) -> + Pair( + ProtoktObjectValue(keyDesc, key!!, context), + ProtoktObjectValue(valDesc, value!!, context), + ) + } + } + + override fun celValue() = + when (value) { + is Enum -> value.value + is UInt -> org.projectnessie.cel.common.ULong.valueOf(value.toLong()) + is ULong -> org.projectnessie.cel.common.ULong.valueOf(value.toLong()) + is Message, is Bytes -> context.convertValue(value) + + // pray + else -> value + } + + override fun jvmValue(clazz: Class): T? = + context.convertValue(value)?.let(clazz::cast) +} diff --git a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt index 6fb0aaef..54555fa1 100644 --- a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt +++ b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt @@ -38,7 +38,8 @@ import kotlin.reflect.full.findAnnotation @Beta class ProtoktValidator( - config: Config = Config.newBuilder().build() + config: Config = Config.newBuilder().build(), + private val lazyConvert: Boolean = true ) { private val evaluatorBuilder = EvaluatorBuilder( @@ -87,7 +88,11 @@ class ProtoktValidator( evaluatorsByFullTypeName.getValue( message::class.findAnnotation()!!.fullTypeName, ).evaluate( - MessageValue(message.toDynamicMessage(RuntimeContext(descriptors))), + if (lazyConvert) { + ProtoktMessageValue(message, RuntimeContext(descriptors)) + } else { + MessageValue(message.toDynamicMessage(RuntimeContext(descriptors))) + }, failFast ) } From c5f00691588f50b124e11eab3eae6d06863452d9 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 14:57:34 -0400 Subject: [PATCH 06/46] Revert "enable lazy conversion" This reverts commit 0cd3157ccf84b8e79485d892bfdc1a7d748e8900. --- .../kotlin/JavaBasedProjectConventions.kt | 1 - gradle/libs.versions.toml | 2 +- protokt-protovalidate/build.gradle.kts | 1 - .../protokt/v1/buf/validate/ProtoktImpl.kt | 86 ------------------- .../v1/buf/validate/ProtoktValidator.kt | 9 +- 5 files changed, 3 insertions(+), 96 deletions(-) delete mode 100644 protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktImpl.kt diff --git a/buildSrc/src/main/kotlin/JavaBasedProjectConventions.kt b/buildSrc/src/main/kotlin/JavaBasedProjectConventions.kt index 7daad3c4..3a04edc3 100644 --- a/buildSrc/src/main/kotlin/JavaBasedProjectConventions.kt +++ b/buildSrc/src/main/kotlin/JavaBasedProjectConventions.kt @@ -27,7 +27,6 @@ import org.jetbrains.kotlin.gradle.tasks.KotlinCompile fun Project.javaBasedProjectConventions() { repositories { mavenCentral() - mavenLocal() } dependencies { diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 76252cf9..ac9dff01 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -25,7 +25,7 @@ protobuf-java = "3.21.7" protobuf-js = "7.2.6" protobufGradlePlugin = "0.9.4" protovalidate = "0.6.4" -protovalidateJava = "0.0.0-SNAPSHOT" +protovalidateJava = "0.2.1" slf4j = "2.0.6" # build diff --git a/protokt-protovalidate/build.gradle.kts b/protokt-protovalidate/build.gradle.kts index c96fd7ec..08bfc2eb 100644 --- a/protokt-protovalidate/build.gradle.kts +++ b/protokt-protovalidate/build.gradle.kts @@ -18,7 +18,6 @@ plugins { } enablePublishing() -trackKotlinApiCompatibility() dependencies { implementation(project(":protokt-reflect")) diff --git a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktImpl.kt b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktImpl.kt deleted file mode 100644 index ed09ec70..00000000 --- a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktImpl.kt +++ /dev/null @@ -1,86 +0,0 @@ -package protokt.v1.buf.validate - -import build.buf.protovalidate.internal.evaluator.MessageLike -import build.buf.protovalidate.internal.evaluator.Value -import com.google.protobuf.Descriptors.FieldDescriptor -import protokt.v1.Bytes -import protokt.v1.Enum -import protokt.v1.Message -import protokt.v1.google.protobuf.RuntimeContext -import protokt.v1.google.protobuf.getField -import protokt.v1.google.protobuf.hasField - -internal class ProtoktMessageLike( - val message: Message, - val context: RuntimeContext, -) : MessageLike { - override fun hasField(field: FieldDescriptor) = - message.hasField(field) - - override fun getField(field: FieldDescriptor) = - ProtoktObjectValue( - field, - message.getField(field)!!, - context - ) -} - -internal class ProtoktMessageValue( - private val message: Message, - private val context: RuntimeContext, -) : Value { - override fun messageValue() = - ProtoktMessageLike(message, context) - - override fun repeatedValue() = - emptyList() - - override fun mapValue() = - emptyMap() - - override fun celValue() = - context.convertValue(message) - - override fun jvmValue(clazz: Class) = - null -} - -internal class ProtoktObjectValue( - private val fieldDescriptor: FieldDescriptor, - private val value: Any, - private val context: RuntimeContext, -) : Value { - override fun messageValue(): MessageLike = - ProtoktMessageLike(value as Message, context) - - override fun repeatedValue() = - (value as List<*>).map { ProtoktObjectValue(fieldDescriptor, it!!, context) } - - override fun mapValue(): Map { - val input = value as Map<*, *> - - val keyDesc = fieldDescriptor.messageType.findFieldByNumber(1) - val valDesc = fieldDescriptor.messageType.findFieldByNumber(2) - - return input.entries.associate { (key, value) -> - Pair( - ProtoktObjectValue(keyDesc, key!!, context), - ProtoktObjectValue(valDesc, value!!, context), - ) - } - } - - override fun celValue() = - when (value) { - is Enum -> value.value - is UInt -> org.projectnessie.cel.common.ULong.valueOf(value.toLong()) - is ULong -> org.projectnessie.cel.common.ULong.valueOf(value.toLong()) - is Message, is Bytes -> context.convertValue(value) - - // pray - else -> value - } - - override fun jvmValue(clazz: Class): T? = - context.convertValue(value)?.let(clazz::cast) -} diff --git a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt index 54555fa1..6fb0aaef 100644 --- a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt +++ b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt @@ -38,8 +38,7 @@ import kotlin.reflect.full.findAnnotation @Beta class ProtoktValidator( - config: Config = Config.newBuilder().build(), - private val lazyConvert: Boolean = true + config: Config = Config.newBuilder().build() ) { private val evaluatorBuilder = EvaluatorBuilder( @@ -88,11 +87,7 @@ class ProtoktValidator( evaluatorsByFullTypeName.getValue( message::class.findAnnotation()!!.fullTypeName, ).evaluate( - if (lazyConvert) { - ProtoktMessageValue(message, RuntimeContext(descriptors)) - } else { - MessageValue(message.toDynamicMessage(RuntimeContext(descriptors))) - }, + MessageValue(message.toDynamicMessage(RuntimeContext(descriptors))), failFast ) } From a9acd637e29f9181d2866554a50a9d98115c323f Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 18:02:28 -0400 Subject: [PATCH 07/46] restrict accepted packages --- .../conformance/DynamicConcreteMessageDeserializer.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/DynamicConcreteMessageDeserializer.kt b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/DynamicConcreteMessageDeserializer.kt index 3bcc4b87..803c89b6 100644 --- a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/DynamicConcreteMessageDeserializer.kt +++ b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/DynamicConcreteMessageDeserializer.kt @@ -25,7 +25,11 @@ import kotlin.reflect.full.findAnnotation object DynamicConcreteMessageDeserializer { private val deserializersByFullTypeName: Map> by lazy { ClassGraph() - .enableAllInfo() + .enableAnnotationInfo() + .acceptPackages( + "protokt.v1.buf.validate.conformance.*", + "protokt.v1.google.protobuf" + ) .scan() .getClassesWithAnnotation(GeneratedMessage::class.java) .asSequence() From 698609b66eb92ff10c360563b69425902e697171 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 18:04:53 -0400 Subject: [PATCH 08/46] close resource --- .../validate/conformance/DynamicConcreteMessageDeserializer.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/DynamicConcreteMessageDeserializer.kt b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/DynamicConcreteMessageDeserializer.kt index 803c89b6..f530b9c2 100644 --- a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/DynamicConcreteMessageDeserializer.kt +++ b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/DynamicConcreteMessageDeserializer.kt @@ -31,8 +31,7 @@ object DynamicConcreteMessageDeserializer { "protokt.v1.google.protobuf" ) .scan() - .getClassesWithAnnotation(GeneratedMessage::class.java) - .asSequence() + .use { it.getClassesWithAnnotation(GeneratedMessage::class.java) } .map { it.loadClass().kotlin } .associate { messageClass -> messageClass.findAnnotation()!!.fullTypeName to From 97717ba53aff3f2437bb8d805e0660825b30b28e Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 18:20:56 -0400 Subject: [PATCH 09/46] oops --- .../conformance/DynamicConcreteMessageDeserializer.kt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/DynamicConcreteMessageDeserializer.kt b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/DynamicConcreteMessageDeserializer.kt index f530b9c2..ad58c118 100644 --- a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/DynamicConcreteMessageDeserializer.kt +++ b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/DynamicConcreteMessageDeserializer.kt @@ -31,8 +31,10 @@ object DynamicConcreteMessageDeserializer { "protokt.v1.google.protobuf" ) .scan() - .use { it.getClassesWithAnnotation(GeneratedMessage::class.java) } - .map { it.loadClass().kotlin } + .use { result -> + result.getClassesWithAnnotation(GeneratedMessage::class.java) + .map { it.loadClass().kotlin } + } .associate { messageClass -> messageClass.findAnnotation()!!.fullTypeName to messageClass From 94b96a32a162da1c0c71d791d67206266f37bc57 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 18:42:56 -0400 Subject: [PATCH 10/46] try debug --- .../protovalidate-conformance/build.gradle.kts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/testing/protovalidate-conformance/build.gradle.kts b/testing/protovalidate-conformance/build.gradle.kts index f0cd7eee..0882c7c9 100644 --- a/testing/protovalidate-conformance/build.gradle.kts +++ b/testing/protovalidate-conformance/build.gradle.kts @@ -13,11 +13,14 @@ * limitations under the License. */ +import com.android.build.gradle.internal.cxx.os.exe import com.android.build.gradle.internal.tasks.factory.dependsOn import com.google.protobuf.gradle.GenerateProtoTask import com.google.protobuf.gradle.proto import org.gradle.api.distribution.plugins.DistributionPlugin.TASK_INSTALL_NAME import org.gradle.language.base.plugins.LifecycleBasePlugin.CHECK_TASK_NAME +import java.io.ByteArrayOutputStream +import java.nio.charset.StandardCharsets plugins { id("protokt.jvm-conventions") @@ -98,6 +101,20 @@ val conformance = .asFile .absolutePath ) + setIgnoreExitValue(true) + val err = ByteArrayOutputStream() + errorOutput = err + + val out = ByteArrayOutputStream() + standardOutput = out + + doLast { + if (executionResult.get().exitValue != 0) { + logger.quiet("err: \n" + err.toString(StandardCharsets.UTF_8)) + logger.quiet("out: \n" + out.toString(StandardCharsets.UTF_8)) + error("broken") + } + } } tasks.named(CHECK_TASK_NAME).dependsOn(conformance) From 3cbe1b546969df8ba1097c5b7911f585a8eab973 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 18:47:57 -0400 Subject: [PATCH 11/46] really --- testing/protovalidate-conformance/build.gradle.kts | 1 - 1 file changed, 1 deletion(-) diff --git a/testing/protovalidate-conformance/build.gradle.kts b/testing/protovalidate-conformance/build.gradle.kts index 0882c7c9..13159515 100644 --- a/testing/protovalidate-conformance/build.gradle.kts +++ b/testing/protovalidate-conformance/build.gradle.kts @@ -13,7 +13,6 @@ * limitations under the License. */ -import com.android.build.gradle.internal.cxx.os.exe import com.android.build.gradle.internal.tasks.factory.dependsOn import com.google.protobuf.gradle.GenerateProtoTask import com.google.protobuf.gradle.proto From be0793202add962c81aec7af8e8e2d580622a719 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 19:09:26 -0400 Subject: [PATCH 12/46] log more --- testing/protovalidate-conformance/build.gradle.kts | 1 + 1 file changed, 1 insertion(+) diff --git a/testing/protovalidate-conformance/build.gradle.kts b/testing/protovalidate-conformance/build.gradle.kts index 13159515..98405a14 100644 --- a/testing/protovalidate-conformance/build.gradle.kts +++ b/testing/protovalidate-conformance/build.gradle.kts @@ -108,6 +108,7 @@ val conformance = standardOutput = out doLast { + logger.quiet("Result: " + executionResult.get().exitValue) if (executionResult.get().exitValue != 0) { logger.quiet("err: \n" + err.toString(StandardCharsets.UTF_8)) logger.quiet("out: \n" + out.toString(StandardCharsets.UTF_8)) From 99587fab8473e98f0553a16c1cb90016ac18132f Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 19:49:09 -0400 Subject: [PATCH 13/46] stop ignoring error --- testing/protovalidate-conformance/build.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/protovalidate-conformance/build.gradle.kts b/testing/protovalidate-conformance/build.gradle.kts index 98405a14..a60a8ca2 100644 --- a/testing/protovalidate-conformance/build.gradle.kts +++ b/testing/protovalidate-conformance/build.gradle.kts @@ -100,7 +100,7 @@ val conformance = .asFile .absolutePath ) - setIgnoreExitValue(true) + //setIgnoreExitValue(true) val err = ByteArrayOutputStream() errorOutput = err From 3304d1fe6486d0440a7e985f5d6ee8766983b0f8 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 19:51:06 -0400 Subject: [PATCH 14/46] oof --- testing/protovalidate-conformance/build.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/protovalidate-conformance/build.gradle.kts b/testing/protovalidate-conformance/build.gradle.kts index a60a8ca2..a04d6c93 100644 --- a/testing/protovalidate-conformance/build.gradle.kts +++ b/testing/protovalidate-conformance/build.gradle.kts @@ -100,7 +100,7 @@ val conformance = .asFile .absolutePath ) - //setIgnoreExitValue(true) + // setIgnoreExitValue(true) val err = ByteArrayOutputStream() errorOutput = err From bfa5fa23f287cf6347682ecb8fb5f61d3f5c43c8 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 20:28:39 -0400 Subject: [PATCH 15/46] try again --- testing/protovalidate-conformance/build.gradle.kts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testing/protovalidate-conformance/build.gradle.kts b/testing/protovalidate-conformance/build.gradle.kts index a04d6c93..fb79a33b 100644 --- a/testing/protovalidate-conformance/build.gradle.kts +++ b/testing/protovalidate-conformance/build.gradle.kts @@ -102,10 +102,10 @@ val conformance = ) // setIgnoreExitValue(true) val err = ByteArrayOutputStream() - errorOutput = err + // errorOutput = err val out = ByteArrayOutputStream() - standardOutput = out + // standardOutput = out doLast { logger.quiet("Result: " + executionResult.get().exitValue) From 1b6a44d4134253ac28c8e9e2ee3f0aa13b81081f Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 20:38:35 -0400 Subject: [PATCH 16/46] more refinement --- testing/protovalidate-conformance/build.gradle.kts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/testing/protovalidate-conformance/build.gradle.kts b/testing/protovalidate-conformance/build.gradle.kts index fb79a33b..8960723a 100644 --- a/testing/protovalidate-conformance/build.gradle.kts +++ b/testing/protovalidate-conformance/build.gradle.kts @@ -100,18 +100,10 @@ val conformance = .asFile .absolutePath ) - // setIgnoreExitValue(true) - val err = ByteArrayOutputStream() - // errorOutput = err - - val out = ByteArrayOutputStream() - // standardOutput = out doLast { logger.quiet("Result: " + executionResult.get().exitValue) if (executionResult.get().exitValue != 0) { - logger.quiet("err: \n" + err.toString(StandardCharsets.UTF_8)) - logger.quiet("out: \n" + out.toString(StandardCharsets.UTF_8)) error("broken") } } From fe3665d19d41068388eead57cc91420f27bc770e Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 20:39:39 -0400 Subject: [PATCH 17/46] imports --- testing/protovalidate-conformance/build.gradle.kts | 2 -- 1 file changed, 2 deletions(-) diff --git a/testing/protovalidate-conformance/build.gradle.kts b/testing/protovalidate-conformance/build.gradle.kts index 8960723a..6c6e823e 100644 --- a/testing/protovalidate-conformance/build.gradle.kts +++ b/testing/protovalidate-conformance/build.gradle.kts @@ -18,8 +18,6 @@ import com.google.protobuf.gradle.GenerateProtoTask import com.google.protobuf.gradle.proto import org.gradle.api.distribution.plugins.DistributionPlugin.TASK_INSTALL_NAME import org.gradle.language.base.plugins.LifecycleBasePlugin.CHECK_TASK_NAME -import java.io.ByteArrayOutputStream -import java.nio.charset.StandardCharsets plugins { id("protokt.jvm-conventions") From 8a01142750b2dcac680da45ad08063e420b82f9d Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 20:57:22 -0400 Subject: [PATCH 18/46] last removal --- testing/protovalidate-conformance/build.gradle.kts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/testing/protovalidate-conformance/build.gradle.kts b/testing/protovalidate-conformance/build.gradle.kts index 6c6e823e..f0cd7eee 100644 --- a/testing/protovalidate-conformance/build.gradle.kts +++ b/testing/protovalidate-conformance/build.gradle.kts @@ -98,13 +98,6 @@ val conformance = .asFile .absolutePath ) - - doLast { - logger.quiet("Result: " + executionResult.get().exitValue) - if (executionResult.get().exitValue != 0) { - error("broken") - } - } } tasks.named(CHECK_TASK_NAME).dependsOn(conformance) From 632cdd6bedbf066509ebc34355945ec8e75764fb Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 21:24:39 -0400 Subject: [PATCH 19/46] overloads --- protokt-protovalidate/api/protokt-protovalidate.api | 10 ++++++++++ protokt-protovalidate/build.gradle.kts | 1 + .../kotlin/protokt/v1/buf/validate/ProtoktValidator.kt | 2 +- 3 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 protokt-protovalidate/api/protokt-protovalidate.api diff --git a/protokt-protovalidate/api/protokt-protovalidate.api b/protokt-protovalidate/api/protokt-protovalidate.api new file mode 100644 index 00000000..f6d4806d --- /dev/null +++ b/protokt-protovalidate/api/protokt-protovalidate.api @@ -0,0 +1,10 @@ +public final class protokt/v1/buf/validate/ProtoktValidator { + public fun ()V + public fun (Lbuild/buf/protovalidate/Config;)V + public synthetic fun (Lbuild/buf/protovalidate/Config;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public final fun load (Lcom/google/protobuf/Descriptors$Descriptor;Lprotokt/v1/Message;)V + public final fun load (Lprotokt/v1/google/protobuf/FileDescriptor;)V + public static synthetic fun load$default (Lprotokt/v1/buf/validate/ProtoktValidator;Lcom/google/protobuf/Descriptors$Descriptor;Lprotokt/v1/Message;ILjava/lang/Object;)V + public final fun validate (Lprotokt/v1/Message;)Lbuild/buf/protovalidate/ValidationResult; +} + diff --git a/protokt-protovalidate/build.gradle.kts b/protokt-protovalidate/build.gradle.kts index 08bfc2eb..c96fd7ec 100644 --- a/protokt-protovalidate/build.gradle.kts +++ b/protokt-protovalidate/build.gradle.kts @@ -18,6 +18,7 @@ plugins { } enablePublishing() +trackKotlinApiCompatibility() dependencies { implementation(project(":protokt-reflect")) diff --git a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt index 6fb0aaef..e79c0aa3 100644 --- a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt +++ b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt @@ -37,7 +37,7 @@ import java.util.concurrent.ConcurrentHashMap import kotlin.reflect.full.findAnnotation @Beta -class ProtoktValidator( +class ProtoktValidator @JvmOverloads constructor( config: Config = Config.newBuilder().build() ) { private val evaluatorBuilder = From 37fc289a8d4e8a94176289e7bc164cdaa400e99d Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 21:43:00 -0400 Subject: [PATCH 20/46] remove unneeded iteration --- .../v1/buf/validate/ProtoktValidator.kt | 39 +++++++++---------- protokt-reflect/api/protokt-reflect.api | 1 + .../v1/google/protobuf/RuntimeContext.kt | 7 +++- 3 files changed, 25 insertions(+), 22 deletions(-) diff --git a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt index e79c0aa3..2acb32c8 100644 --- a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt +++ b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt @@ -32,7 +32,6 @@ import protokt.v1.Message import protokt.v1.google.protobuf.FileDescriptor import protokt.v1.google.protobuf.RuntimeContext import protokt.v1.google.protobuf.toDynamicMessage -import java.util.Collections import java.util.concurrent.ConcurrentHashMap import kotlin.reflect.full.findAnnotation @@ -49,7 +48,7 @@ class ProtoktValidator @JvmOverloads constructor( private val failFast = config.isFailFast private val evaluatorsByFullTypeName = ConcurrentHashMap() - private val descriptors = Collections.synchronizedList(mutableListOf()) + private val runtimeContext = RuntimeContext(emptyList()) fun load(descriptor: FileDescriptor) { descriptor @@ -58,36 +57,34 @@ class ProtoktValidator @JvmOverloads constructor( .forEach(::load) } + private fun FileDescriptor.toProtobufJavaDescriptor(): Descriptors.FileDescriptor = + Descriptors.FileDescriptor.buildFrom( + DescriptorProtos.FileDescriptorProto.parseFrom(proto.serialize()), + dependencies.map { it.toProtobufJavaDescriptor() }.toTypedArray(), + true + ) + fun load( descriptor: Descriptor, - message: Message? = null, + message: Message? = null ) { - descriptors.add(descriptor) + runtimeContext.add(descriptor) try { evaluatorsByFullTypeName[descriptor.fullName] = evaluatorBuilder.load(descriptor) } catch (ex: Exception) { // idiosyncrasy of the conformance suite runner requires this particular exception is rethrown rather than a lookup failure later - if (message != null) { - if (message::class.findAnnotation()!!.fullTypeName == descriptor.fullName) { - throw ex - } + if (message != null && message.fullTypeName == descriptor.fullName) { + throw ex } } descriptor.nestedTypes.forEach { load(it, message) } } - private fun FileDescriptor.toProtobufJavaDescriptor(): Descriptors.FileDescriptor = - Descriptors.FileDescriptor.buildFrom( - DescriptorProtos.FileDescriptorProto.parseFrom(proto.serialize()), - dependencies.map { it.toProtobufJavaDescriptor() }.toTypedArray(), - true - ) - fun validate(message: Message): ValidationResult = - evaluatorsByFullTypeName.getValue( - message::class.findAnnotation()!!.fullTypeName, - ).evaluate( - MessageValue(message.toDynamicMessage(RuntimeContext(descriptors))), - failFast - ) + evaluatorsByFullTypeName + .getValue(message.fullTypeName) + .evaluate(MessageValue(message.toDynamicMessage(runtimeContext)), failFast) + + private val Message.fullTypeName + get() = this::class.findAnnotation()!!.fullTypeName } diff --git a/protokt-reflect/api/protokt-reflect.api b/protokt-reflect/api/protokt-reflect.api index d951bdb0..754e6d4c 100644 --- a/protokt-reflect/api/protokt-reflect.api +++ b/protokt-reflect/api/protokt-reflect.api @@ -1240,6 +1240,7 @@ public final class protokt/v1/google/protobuf/Messages { public final class protokt/v1/google/protobuf/RuntimeContext { public fun (Ljava/lang/Iterable;)V + public final fun add (Lcom/google/protobuf/Descriptors$Descriptor;)V public final fun convertValue (Ljava/lang/Object;)Ljava/lang/Object; } diff --git a/protokt-reflect/src/jvmMain/kotlin/protokt/v1/google/protobuf/RuntimeContext.kt b/protokt-reflect/src/jvmMain/kotlin/protokt/v1/google/protobuf/RuntimeContext.kt index b1c04630..1d3fc452 100644 --- a/protokt-reflect/src/jvmMain/kotlin/protokt/v1/google/protobuf/RuntimeContext.kt +++ b/protokt-reflect/src/jvmMain/kotlin/protokt/v1/google/protobuf/RuntimeContext.kt @@ -37,6 +37,7 @@ import protokt.v1.reflect.WellKnownTypes import protokt.v1.reflect.inferClassName import protokt.v1.reflect.resolvePackage import protokt.v1.reflect.typeName +import java.util.concurrent.ConcurrentHashMap import kotlin.Any import kotlin.reflect.full.findAnnotation @@ -47,7 +48,11 @@ class RuntimeContext internal constructor( ) { constructor(descriptors: Iterable) : this(descriptors, ClassLookup(emptyList())) - internal val descriptorsByTypeName = descriptors.associateBy { it.fullName } + internal val descriptorsByTypeName = ConcurrentHashMap(descriptors.associateBy { it.fullName }) + + fun add(descriptor: Descriptors.Descriptor) { + descriptorsByTypeName[descriptor.fullName] = descriptor + } fun convertValue(value: Any) = when (value) { From 8b9c44a69f1261370e9e52ac2394b8dee48f6e44 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 22:00:33 -0400 Subject: [PATCH 21/46] begin simplifying --- .../protokt/v1/buf/validate/conformance/Main.kt | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt index a78a0191..30cb2b2c 100644 --- a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt +++ b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt @@ -41,7 +41,7 @@ object Main { private fun testConformance(request: TestConformanceRequest): TestConformanceResponse { val descriptorMap = parse(request.fdset) - val validator = ProtoktValidator() + val validator = ProtoktValidator().apply { descriptorMap.values.forEach(::load) } val responseBuilder = TestConformanceResponse.newBuilder() responseBuilder.putAllResults( request.casesMap.mapValues { (_, value) -> @@ -58,7 +58,7 @@ object Main { ): TestResult { val urlParts = testCase.typeUrl.split('/', limit = 2) val fullName = urlParts[urlParts.size - 1] - fileDescriptors[fullName] ?: return unexpectedErrorResult("Unable to find descriptor: %s", fullName) + fileDescriptors[fullName] ?: return unexpectedErrorResult("Unable to find descriptor $fullName") val testCaseValue = testCase.value val message = DynamicConcreteMessageDeserializer.parse(fullName, testCaseValue.newInput()) return validate(validator, message, fileDescriptors.values) @@ -82,12 +82,10 @@ object Main { } catch (e: ExecutionException) { return TestResult.newBuilder().setRuntimeError(e.message).build() } catch (e: Exception) { - return unexpectedErrorResult("unknown error: %s", e.toString()) + return unexpectedErrorResult("unknown error: $e") } } - private fun unexpectedErrorResult(format: String?, vararg args: Any?): TestResult { - val errorMessage = String.format(format!!, *args) - return TestResult.newBuilder().setUnexpectedError(errorMessage).build() - } + private fun unexpectedErrorResult(message: String) = + TestResult.newBuilder().setUnexpectedError(message).build() } From 6653e759b6de2f257fa5fa0aaec2ac4dc9814a44 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 22:28:54 -0400 Subject: [PATCH 22/46] simplify --- .../v1/buf/validate/ProtoktValidator.kt | 21 ++---- .../DynamicConcreteMessageDeserializer.kt | 6 +- .../v1/buf/validate/conformance/Main.kt | 66 +++++++++++-------- 3 files changed, 45 insertions(+), 48 deletions(-) diff --git a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt index 2acb32c8..5081bf10 100644 --- a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt +++ b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt @@ -64,27 +64,14 @@ class ProtoktValidator @JvmOverloads constructor( true ) - fun load( - descriptor: Descriptor, - message: Message? = null - ) { + fun load(descriptor: Descriptor) { runtimeContext.add(descriptor) - try { - evaluatorsByFullTypeName[descriptor.fullName] = evaluatorBuilder.load(descriptor) - } catch (ex: Exception) { - // idiosyncrasy of the conformance suite runner requires this particular exception is rethrown rather than a lookup failure later - if (message != null && message.fullTypeName == descriptor.fullName) { - throw ex - } - } - descriptor.nestedTypes.forEach { load(it, message) } + evaluatorsByFullTypeName[descriptor.fullName] = evaluatorBuilder.load(descriptor) + descriptor.nestedTypes.forEach(::load) } fun validate(message: Message): ValidationResult = evaluatorsByFullTypeName - .getValue(message.fullTypeName) + .getValue(message::class.findAnnotation()!!.fullTypeName) .evaluate(MessageValue(message.toDynamicMessage(runtimeContext)), failFast) - - private val Message.fullTypeName - get() = this::class.findAnnotation()!!.fullTypeName } diff --git a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/DynamicConcreteMessageDeserializer.kt b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/DynamicConcreteMessageDeserializer.kt index ad58c118..9ef2e779 100644 --- a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/DynamicConcreteMessageDeserializer.kt +++ b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/DynamicConcreteMessageDeserializer.kt @@ -15,11 +15,11 @@ package protokt.v1.buf.validate.conformance +import com.google.protobuf.ByteString import io.github.classgraph.ClassGraph import protokt.v1.Deserializer import protokt.v1.GeneratedMessage import protokt.v1.google.protobuf.Empty -import java.io.InputStream import kotlin.reflect.full.findAnnotation object DynamicConcreteMessageDeserializer { @@ -44,6 +44,6 @@ object DynamicConcreteMessageDeserializer { } } - fun parse(fullTypeName: String, bytes: InputStream) = - deserializersByFullTypeName.getValue(fullTypeName).deserialize(bytes) + fun parse(fullTypeName: String, bytes: ByteString) = + deserializersByFullTypeName.getValue(fullTypeName).deserialize(bytes.newInput()) } diff --git a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt index 30cb2b2c..5c3a78ad 100644 --- a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt +++ b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt @@ -35,20 +35,31 @@ object Main { extensionRegistry.add(ValidateProto.field) extensionRegistry.add(ValidateProto.oneof) val request = TestConformanceRequest.parseFrom(System.`in`, extensionRegistry) - val response = testConformance(request) - response.writeTo(System.out) + testConformance(request).writeTo(System.out) } private fun testConformance(request: TestConformanceRequest): TestConformanceResponse { val descriptorMap = parse(request.fdset) - val validator = ProtoktValidator().apply { descriptorMap.values.forEach(::load) } - val responseBuilder = TestConformanceResponse.newBuilder() - responseBuilder.putAllResults( - request.casesMap.mapValues { (_, value) -> - testCase(validator, descriptorMap, value) + val validator = ProtoktValidator() + loadValidDescriptors(validator, descriptorMap.values) + return TestConformanceResponse + .newBuilder() + .putAllResults( + request.casesMap.mapValues { (_, value) -> + testCase(validator, descriptorMap, value) + } + ) + .build() + } + + private fun loadValidDescriptors(validator: ProtoktValidator, descriptors: Iterable) { + descriptors.forEach { + try { + validator.load(it) + } catch (_: CompilationException) { + // leave failures for later; they trigger specific conformance results } - ) - return responseBuilder.build() + } } private fun testCase( @@ -58,34 +69,33 @@ object Main { ): TestResult { val urlParts = testCase.typeUrl.split('/', limit = 2) val fullName = urlParts[urlParts.size - 1] - fileDescriptors[fullName] ?: return unexpectedErrorResult("Unable to find descriptor $fullName") - val testCaseValue = testCase.value - val message = DynamicConcreteMessageDeserializer.parse(fullName, testCaseValue.newInput()) - return validate(validator, message, fileDescriptors.values) + val descriptor = fileDescriptors[fullName] ?: return unexpected("Unable to find descriptor $fullName") + + try { + validator.load(descriptor) + } catch (e: CompilationException) { + return TestResult.newBuilder().setCompilationError(e.message).build() + } + + return validate(validator, DynamicConcreteMessageDeserializer.parse(fullName, testCase.value)) } - private fun validate( - validator: ProtoktValidator, - message: Message, - descriptors: Iterable - ): TestResult { + private fun validate(validator: ProtoktValidator, message: Message) = try { - descriptors.forEach { validator.load(it, message) } val result = validator.validate(message) if (result.isSuccess) { - return TestResult.newBuilder().setSuccess(true).build() + TestResult.newBuilder().setSuccess(true).build() + } else { + TestResult.newBuilder() + .setValidationError(Violations.newBuilder().addAllViolations(result.violations).build()) + .build() } - val error = Violations.newBuilder().addAllViolations(result.violations).build() - return TestResult.newBuilder().setValidationError(error).build() - } catch (e: CompilationException) { - return TestResult.newBuilder().setCompilationError(e.message).build() } catch (e: ExecutionException) { - return TestResult.newBuilder().setRuntimeError(e.message).build() + TestResult.newBuilder().setRuntimeError(e.message).build() } catch (e: Exception) { - return unexpectedErrorResult("unknown error: $e") + unexpected("unknown error: $e") } - } - private fun unexpectedErrorResult(message: String) = + private fun unexpected(message: String) = TestResult.newBuilder().setUnexpectedError(message).build() } From 2b78bbaa2d34a1d2f112f395b09566d13a2bf6a0 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 22:29:07 -0400 Subject: [PATCH 23/46] apidump --- protokt-protovalidate/api/protokt-protovalidate.api | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/protokt-protovalidate/api/protokt-protovalidate.api b/protokt-protovalidate/api/protokt-protovalidate.api index f6d4806d..4db567de 100644 --- a/protokt-protovalidate/api/protokt-protovalidate.api +++ b/protokt-protovalidate/api/protokt-protovalidate.api @@ -2,9 +2,8 @@ public final class protokt/v1/buf/validate/ProtoktValidator { public fun ()V public fun (Lbuild/buf/protovalidate/Config;)V public synthetic fun (Lbuild/buf/protovalidate/Config;ILkotlin/jvm/internal/DefaultConstructorMarker;)V - public final fun load (Lcom/google/protobuf/Descriptors$Descriptor;Lprotokt/v1/Message;)V + public final fun load (Lcom/google/protobuf/Descriptors$Descriptor;)V public final fun load (Lprotokt/v1/google/protobuf/FileDescriptor;)V - public static synthetic fun load$default (Lprotokt/v1/buf/validate/ProtoktValidator;Lcom/google/protobuf/Descriptors$Descriptor;Lprotokt/v1/Message;ILjava/lang/Object;)V public final fun validate (Lprotokt/v1/Message;)Lbuild/buf/protovalidate/ValidationResult; } From 048c96589874bd20293af741bf294548ebfa31fb Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 22:43:14 -0400 Subject: [PATCH 24/46] fix --- .../v1/buf/validate/ProtoktValidator.kt | 17 --- .../validate/AbstractProtoktValidatorTest.kt | 103 +++++++++++------- 2 files changed, 61 insertions(+), 59 deletions(-) diff --git a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt index 5081bf10..73db6d07 100644 --- a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt +++ b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt @@ -21,15 +21,12 @@ import build.buf.protovalidate.internal.celext.ValidateLibrary import build.buf.protovalidate.internal.evaluator.Evaluator import build.buf.protovalidate.internal.evaluator.EvaluatorBuilder import build.buf.protovalidate.internal.evaluator.MessageValue -import com.google.protobuf.DescriptorProtos -import com.google.protobuf.Descriptors import com.google.protobuf.Descriptors.Descriptor import org.projectnessie.cel.Env import org.projectnessie.cel.Library import protokt.v1.Beta import protokt.v1.GeneratedMessage import protokt.v1.Message -import protokt.v1.google.protobuf.FileDescriptor import protokt.v1.google.protobuf.RuntimeContext import protokt.v1.google.protobuf.toDynamicMessage import java.util.concurrent.ConcurrentHashMap @@ -50,20 +47,6 @@ class ProtoktValidator @JvmOverloads constructor( private val evaluatorsByFullTypeName = ConcurrentHashMap() private val runtimeContext = RuntimeContext(emptyList()) - fun load(descriptor: FileDescriptor) { - descriptor - .toProtobufJavaDescriptor() - .messageTypes - .forEach(::load) - } - - private fun FileDescriptor.toProtobufJavaDescriptor(): Descriptors.FileDescriptor = - Descriptors.FileDescriptor.buildFrom( - DescriptorProtos.FileDescriptorProto.parseFrom(proto.serialize()), - dependencies.map { it.toProtobufJavaDescriptor() }.toTypedArray(), - true - ) - fun load(descriptor: Descriptor) { runtimeContext.add(descriptor) evaluatorsByFullTypeName[descriptor.fullName] = evaluatorBuilder.load(descriptor) diff --git a/testing/protovalidate-conformance/src/test/kotlin/validate/AbstractProtoktValidatorTest.kt b/testing/protovalidate-conformance/src/test/kotlin/validate/AbstractProtoktValidatorTest.kt index ea7eceb4..09cac413 100644 --- a/testing/protovalidate-conformance/src/test/kotlin/validate/AbstractProtoktValidatorTest.kt +++ b/testing/protovalidate-conformance/src/test/kotlin/validate/AbstractProtoktValidatorTest.kt @@ -22,6 +22,8 @@ import buf.validate.conformance.cases.Strings import build.buf.protovalidate.ValidationResult import com.google.common.truth.Truth.assertThat import com.google.protobuf.ByteString +import com.google.protobuf.DescriptorProtos +import com.google.protobuf.Descriptors import org.junit.jupiter.api.Test import protokt.v1.AbstractDeserializer import protokt.v1.AbstractMessage @@ -41,15 +43,32 @@ import protokt.v1.buf.validate.conformance.cases.numbers_file_descriptor import protokt.v1.buf.validate.conformance.cases.oneofs_file_descriptor import protokt.v1.buf.validate.conformance.cases.repeated_file_descriptor import protokt.v1.buf.validate.conformance.cases.strings_file_descriptor +import protokt.v1.google.protobuf.FileDescriptor abstract class AbstractProtoktValidatorTest { protected val validator = ProtoktValidator() abstract fun validate(message: Message): ValidationResult + private fun load(descriptor: FileDescriptor) { + descriptor + .toProtobufJavaDescriptor() + .messageTypes + .forEach { + runCatching { validator.load(it) } + } + } + + private fun FileDescriptor.toProtobufJavaDescriptor(): Descriptors.FileDescriptor = + Descriptors.FileDescriptor.buildFrom( + DescriptorProtos.FileDescriptorProto.parseFrom(proto.serialize()), + dependencies.map { it.toProtobufJavaDescriptor() }.toTypedArray(), + true + ) + @Test fun `test required oneof constraint`() { - validator.load(messages_file_descriptor.descriptor) + load(messages_file_descriptor.descriptor) val result = validate( @@ -58,9 +77,9 @@ abstract class AbstractProtoktValidatorTest { MessageRequiredOneof.One.Val( TestMsg { const = "foo" - }, + } ) - }, + } ) assertThat(result.violations).isEmpty() @@ -69,13 +88,13 @@ abstract class AbstractProtoktValidatorTest { @Test fun `test oneof constraint`() { - validator.load(oneofs_file_descriptor.descriptor) + load(oneofs_file_descriptor.descriptor) val result = validate( Oneof { o = Oneof.O.X("foobar") - }, + } ) assertThat(result.violations).isEmpty() @@ -84,13 +103,13 @@ abstract class AbstractProtoktValidatorTest { @Test fun `test uint64 in constraint`() { - validator.load(numbers_file_descriptor.descriptor) + load(numbers_file_descriptor.descriptor) val result = validate( UInt64In { `val` = 4u - }, + } ) assertThat(result.isSuccess).isFalse() @@ -98,7 +117,7 @@ abstract class AbstractProtoktValidatorTest { @Test fun `test message with varint non-uint64 encoded purely as unknown fields (dynamic message without a dedicated type)`() { - validator.load(numbers_file_descriptor.descriptor) + load(numbers_file_descriptor.descriptor) val result = validate( @@ -107,8 +126,8 @@ abstract class AbstractProtoktValidatorTest { .newBuilder() .setVal(4) .build() - .toByteArray(), - ), + .toByteArray() + ) ) assertThat(result.isSuccess).isFalse() @@ -120,8 +139,8 @@ abstract class AbstractProtoktValidatorTest { .newBuilder() .setVal(3) .build() - .toByteArray(), - ), + .toByteArray() + ) ) assertThat(result2.isSuccess).isTrue() @@ -129,7 +148,7 @@ abstract class AbstractProtoktValidatorTest { @Test fun `test message with varint uint64 encoded purely as unknown fields (dynamic message without a dedicated type)`() { - validator.load(numbers_file_descriptor.descriptor) + load(numbers_file_descriptor.descriptor) val result = validate( @@ -138,8 +157,8 @@ abstract class AbstractProtoktValidatorTest { .newBuilder() .setVal(4) .build() - .toByteArray(), - ), + .toByteArray() + ) ) assertThat(result.isSuccess).isFalse() @@ -151,8 +170,8 @@ abstract class AbstractProtoktValidatorTest { .newBuilder() .setVal(3) .build() - .toByteArray(), - ), + .toByteArray() + ) ) assertThat(result2.isSuccess).isTrue() @@ -160,7 +179,7 @@ abstract class AbstractProtoktValidatorTest { @Test fun `test message with fixed32 encoded purely as unknown fields (dynamic message without a dedicated type)`() { - validator.load(numbers_file_descriptor.descriptor) + load(numbers_file_descriptor.descriptor) val result = validate( @@ -169,8 +188,8 @@ abstract class AbstractProtoktValidatorTest { .newBuilder() .setVal(4) .build() - .toByteArray(), - ), + .toByteArray() + ) ) assertThat(result.isSuccess).isFalse() @@ -182,8 +201,8 @@ abstract class AbstractProtoktValidatorTest { .newBuilder() .setVal(3) .build() - .toByteArray(), - ), + .toByteArray() + ) ) assertThat(result2.isSuccess).isTrue() @@ -191,7 +210,7 @@ abstract class AbstractProtoktValidatorTest { @Test fun `test message with fixed64 encoded purely as unknown fields (dynamic message without a dedicated type)`() { - validator.load(numbers_file_descriptor.descriptor) + load(numbers_file_descriptor.descriptor) val result = validate( @@ -200,8 +219,8 @@ abstract class AbstractProtoktValidatorTest { .newBuilder() .setVal(4) .build() - .toByteArray(), - ), + .toByteArray() + ) ) assertThat(result.isSuccess).isFalse() @@ -213,8 +232,8 @@ abstract class AbstractProtoktValidatorTest { .newBuilder() .setVal(3) .build() - .toByteArray(), - ), + .toByteArray() + ) ) assertThat(result2.isSuccess).isTrue() @@ -222,7 +241,7 @@ abstract class AbstractProtoktValidatorTest { @Test fun `test message with length delimited string encoded purely as unknown fields (dynamic message without a dedicated type)`() { - validator.load(strings_file_descriptor.descriptor) + load(strings_file_descriptor.descriptor) val result = validate( @@ -231,8 +250,8 @@ abstract class AbstractProtoktValidatorTest { .newBuilder() .setVal("foo") .build() - .toByteArray(), - ), + .toByteArray() + ) ) assertThat(result.isSuccess).isFalse() @@ -244,8 +263,8 @@ abstract class AbstractProtoktValidatorTest { .newBuilder() .setVal("bar") .build() - .toByteArray(), - ), + .toByteArray() + ) ) assertThat(result2.isSuccess).isTrue() @@ -253,7 +272,7 @@ abstract class AbstractProtoktValidatorTest { @Test fun `test message with length delimited bytes encoded purely as unknown fields (dynamic message without a dedicated type)`() { - validator.load(bytes_file_descriptor.descriptor) + load(bytes_file_descriptor.descriptor) val result = validate( @@ -262,8 +281,8 @@ abstract class AbstractProtoktValidatorTest { .newBuilder() .setVal(ByteString.copyFromUtf8("foo")) .build() - .toByteArray(), - ), + .toByteArray() + ) ) assertThat(result.isSuccess).isFalse() @@ -275,8 +294,8 @@ abstract class AbstractProtoktValidatorTest { .newBuilder() .setVal(ByteString.copyFromUtf8("bar")) .build() - .toByteArray(), - ), + .toByteArray() + ) ) assertThat(result2.isSuccess).isTrue() @@ -284,7 +303,7 @@ abstract class AbstractProtoktValidatorTest { @Test fun `test message with repeated values encoded purely as unknown fields (dynamic message without a dedicated type)`() { - validator.load(repeated_file_descriptor.descriptor) + load(repeated_file_descriptor.descriptor) val result = validate( @@ -293,8 +312,8 @@ abstract class AbstractProtoktValidatorTest { .newBuilder() .addAllVal(listOf("foo", "foo")) .build() - .toByteArray(), - ), + .toByteArray() + ) ) assertThat(result.isSuccess).isFalse() @@ -306,8 +325,8 @@ abstract class AbstractProtoktValidatorTest { .newBuilder() .addAllVal(listOf("foo", "bar")) .build() - .toByteArray(), - ), + .toByteArray() + ) ) assertThat(result2.isSuccess).isTrue() From 42d1ef64bfd9fbbbcbb1d41ade91d93c83aa580d Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sun, 12 May 2024 22:48:02 -0400 Subject: [PATCH 25/46] apidump --- protokt-protovalidate/api/protokt-protovalidate.api | 1 - 1 file changed, 1 deletion(-) diff --git a/protokt-protovalidate/api/protokt-protovalidate.api b/protokt-protovalidate/api/protokt-protovalidate.api index 4db567de..507e2600 100644 --- a/protokt-protovalidate/api/protokt-protovalidate.api +++ b/protokt-protovalidate/api/protokt-protovalidate.api @@ -3,7 +3,6 @@ public final class protokt/v1/buf/validate/ProtoktValidator { public fun (Lbuild/buf/protovalidate/Config;)V public synthetic fun (Lbuild/buf/protovalidate/Config;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public final fun load (Lcom/google/protobuf/Descriptors$Descriptor;)V - public final fun load (Lprotokt/v1/google/protobuf/FileDescriptor;)V public final fun validate (Lprotokt/v1/Message;)Lbuild/buf/protovalidate/ValidationResult; } From bc3e7fbd3335abe93adaf4675e762025dc9a6d7e Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Mon, 13 May 2024 07:54:53 -0400 Subject: [PATCH 26/46] package change --- .../validate/AbstractProtoktValidatorTest.kt | 45 +++++++------------ .../validate/ProtoktDynamicMessageTest.kt | 2 +- 2 files changed, 16 insertions(+), 31 deletions(-) rename testing/protovalidate-conformance/src/test/kotlin/{ => protokt/v1/buf}/validate/AbstractProtoktValidatorTest.kt (92%) rename testing/protovalidate-conformance/src/test/kotlin/{ => protokt/v1/buf}/validate/ProtoktDynamicMessageTest.kt (96%) diff --git a/testing/protovalidate-conformance/src/test/kotlin/validate/AbstractProtoktValidatorTest.kt b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/AbstractProtoktValidatorTest.kt similarity index 92% rename from testing/protovalidate-conformance/src/test/kotlin/validate/AbstractProtoktValidatorTest.kt rename to testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/AbstractProtoktValidatorTest.kt index 09cac413..cb66d9a1 100644 --- a/testing/protovalidate-conformance/src/test/kotlin/validate/AbstractProtoktValidatorTest.kt +++ b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/AbstractProtoktValidatorTest.kt @@ -13,7 +13,7 @@ * limitations under the License. */ -package validate +package protokt.v1.buf.validate import buf.validate.conformance.cases.Bytes import buf.validate.conformance.cases.Numbers @@ -32,7 +32,6 @@ import protokt.v1.Message import protokt.v1.Reader import protokt.v1.UnknownFieldSet import protokt.v1.Writer -import protokt.v1.buf.validate.ProtoktValidator import protokt.v1.buf.validate.conformance.cases.MessageRequiredOneof import protokt.v1.buf.validate.conformance.cases.Oneof import protokt.v1.buf.validate.conformance.cases.TestMsg @@ -122,8 +121,7 @@ abstract class AbstractProtoktValidatorTest { val result = validate( Int64.deserialize( - Numbers.Int64In - .newBuilder() + Numbers.Int64In.newBuilder() .setVal(4) .build() .toByteArray() @@ -135,8 +133,7 @@ abstract class AbstractProtoktValidatorTest { val result2 = validate( Int64.deserialize( - Numbers.Int64In - .newBuilder() + Numbers.Int64In.newBuilder() .setVal(3) .build() .toByteArray() @@ -153,8 +150,7 @@ abstract class AbstractProtoktValidatorTest { val result = validate( UInt64.deserialize( - Numbers.UInt64In - .newBuilder() + Numbers.UInt64In.newBuilder() .setVal(4) .build() .toByteArray() @@ -166,8 +162,7 @@ abstract class AbstractProtoktValidatorTest { val result2 = validate( UInt64.deserialize( - Numbers.UInt64In - .newBuilder() + Numbers.UInt64In.newBuilder() .setVal(3) .build() .toByteArray() @@ -184,8 +179,7 @@ abstract class AbstractProtoktValidatorTest { val result = validate( Fixed32.deserialize( - Numbers.Fixed32In - .newBuilder() + Numbers.Fixed32In.newBuilder() .setVal(4) .build() .toByteArray() @@ -197,8 +191,7 @@ abstract class AbstractProtoktValidatorTest { val result2 = validate( Fixed32.deserialize( - Numbers.Fixed32In - .newBuilder() + Numbers.Fixed32In.newBuilder() .setVal(3) .build() .toByteArray() @@ -215,8 +208,7 @@ abstract class AbstractProtoktValidatorTest { val result = validate( Fixed64.deserialize( - Numbers.Fixed64In - .newBuilder() + Numbers.Fixed64In.newBuilder() .setVal(4) .build() .toByteArray() @@ -228,8 +220,7 @@ abstract class AbstractProtoktValidatorTest { val result2 = validate( Fixed64.deserialize( - Numbers.Fixed64In - .newBuilder() + Numbers.Fixed64In.newBuilder() .setVal(3) .build() .toByteArray() @@ -246,8 +237,7 @@ abstract class AbstractProtoktValidatorTest { val result = validate( LengthDelimitedString.deserialize( - Strings.StringIn - .newBuilder() + Strings.StringIn.newBuilder() .setVal("foo") .build() .toByteArray() @@ -259,8 +249,7 @@ abstract class AbstractProtoktValidatorTest { val result2 = validate( LengthDelimitedString.deserialize( - Strings.StringIn - .newBuilder() + Strings.StringIn.newBuilder() .setVal("bar") .build() .toByteArray() @@ -277,8 +266,7 @@ abstract class AbstractProtoktValidatorTest { val result = validate( LengthDelimitedBytes.deserialize( - Bytes.BytesIn - .newBuilder() + Bytes.BytesIn.newBuilder() .setVal(ByteString.copyFromUtf8("foo")) .build() .toByteArray() @@ -290,8 +278,7 @@ abstract class AbstractProtoktValidatorTest { val result2 = validate( LengthDelimitedBytes.deserialize( - Bytes.BytesIn - .newBuilder() + Bytes.BytesIn.newBuilder() .setVal(ByteString.copyFromUtf8("bar")) .build() .toByteArray() @@ -308,8 +295,7 @@ abstract class AbstractProtoktValidatorTest { val result = validate( RepeatedLengthDelimited.deserialize( - Repeated.RepeatedUnique - .newBuilder() + Repeated.RepeatedUnique.newBuilder() .addAllVal(listOf("foo", "foo")) .build() .toByteArray() @@ -321,8 +307,7 @@ abstract class AbstractProtoktValidatorTest { val result2 = validate( RepeatedLengthDelimited.deserialize( - Repeated.RepeatedUnique - .newBuilder() + Repeated.RepeatedUnique.newBuilder() .addAllVal(listOf("foo", "bar")) .build() .toByteArray() diff --git a/testing/protovalidate-conformance/src/test/kotlin/validate/ProtoktDynamicMessageTest.kt b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ProtoktDynamicMessageTest.kt similarity index 96% rename from testing/protovalidate-conformance/src/test/kotlin/validate/ProtoktDynamicMessageTest.kt rename to testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ProtoktDynamicMessageTest.kt index 0cf9ed76..efc3ee1e 100644 --- a/testing/protovalidate-conformance/src/test/kotlin/validate/ProtoktDynamicMessageTest.kt +++ b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ProtoktDynamicMessageTest.kt @@ -13,7 +13,7 @@ * limitations under the License. */ -package validate +package protokt.v1.buf.validate import protokt.v1.Message From 7604111ef76383b5d93a9c85e1449267075cd84b Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sat, 18 May 2024 15:16:58 -0400 Subject: [PATCH 27/46] try using junit to run the runner --- .../build.gradle.kts | 25 ++++-------- .../v1/buf/validate/ConformanceTest.kt | 38 +++++++++++++++++++ .../kotlin/protokt/v1/testing/ProcessUtils.kt | 6 ++- 3 files changed, 49 insertions(+), 20 deletions(-) create mode 100644 testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt diff --git a/testing/protovalidate-conformance/build.gradle.kts b/testing/protovalidate-conformance/build.gradle.kts index f0cd7eee..8e733e6d 100644 --- a/testing/protovalidate-conformance/build.gradle.kts +++ b/testing/protovalidate-conformance/build.gradle.kts @@ -13,11 +13,8 @@ * limitations under the License. */ -import com.android.build.gradle.internal.tasks.factory.dependsOn import com.google.protobuf.gradle.GenerateProtoTask import com.google.protobuf.gradle.proto -import org.gradle.api.distribution.plugins.DistributionPlugin.TASK_INSTALL_NAME -import org.gradle.language.base.plugins.LifecycleBasePlugin.CHECK_TASK_NAME plugins { id("protokt.jvm-conventions") @@ -34,6 +31,7 @@ dependencies { implementation(libs.classgraph) implementation(libs.protovalidateJava) + testImplementation(project(":testing:testing-util")) testImplementation(libs.truth) } @@ -85,19 +83,10 @@ application { mainClass.set("protokt.v1.buf.validate.conformance.Main") } -val conformance = - tasks.register("conformance") { - dependsOn(TASK_INSTALL_NAME, installConformance) - commandLine( - conformanceExecutable.absolutePath, - "--strict_message", - "--strict_error", - project.layout.buildDirectory - .file("install/protovalidate-conformance/bin/protovalidate-conformance") - .get() - .asFile - .absolutePath - ) +tasks { + test { + systemProperty("conformance-runner", conformanceExecutable.absolutePath) + outputs.upToDateWhen { false } + dependsOn(installConformance) } - -tasks.named(CHECK_TASK_NAME).dependsOn(conformance) +} diff --git a/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt new file mode 100644 index 00000000..9a0ecad5 --- /dev/null +++ b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt @@ -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://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.buf.validate + +import org.junit.jupiter.api.Test +import protokt.v1.testing.ProcessOutput +import protokt.v1.testing.projectRoot +import protokt.v1.testing.runCommand +import java.nio.file.Path +import java.time.Duration + +class ConformanceTest { + @Test + fun `run conformance test`() { + command() + .runCommand(projectRoot.toPath(), timeout = Duration.ofMinutes(2)) + .orFail("Protovalidate conformance tests failed", ProcessOutput.Src.ERR) + } +} + +private val driver = + Path.of(projectRoot.absolutePath, "build", "install", "protovalidate-conformance", "bin", "protovalidate-conformance") + +private fun command() = + "${System.getProperty("conformance-runner")} --strict_message --strict_error $driver" diff --git a/testing/testing-util/src/main/kotlin/protokt/v1/testing/ProcessUtils.kt b/testing/testing-util/src/main/kotlin/protokt/v1/testing/ProcessUtils.kt index a73e3266..0bcff177 100644 --- a/testing/testing-util/src/main/kotlin/protokt/v1/testing/ProcessUtils.kt +++ b/testing/testing-util/src/main/kotlin/protokt/v1/testing/ProcessUtils.kt @@ -20,6 +20,7 @@ import protokt.v1.testing.ProcessOutput.Src.ERR import protokt.v1.testing.ProcessOutput.Src.OUT import java.io.File import java.nio.file.Path +import java.time.Duration import java.util.concurrent.TimeUnit val projectRoot = @@ -27,7 +28,8 @@ val projectRoot = fun String.runCommand( workingDir: Path, - env: Map = emptyMap() + env: Map = emptyMap(), + timeout: Duration = Duration.ofSeconds(10) ): ProcessOutput { println("Executing $this in $workingDir with $env") @@ -39,7 +41,7 @@ fun String.runCommand( .apply { environment().putAll(env) } .start() - if (!proc.waitFor(10, TimeUnit.SECONDS)) { + if (!proc.waitFor(timeout.toSeconds(), TimeUnit.SECONDS)) { proc.destroyForcibly() fail("Process '$this' took too long") } From 1faf174bbe88eb97144173c09a91b90135496a0c Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sat, 18 May 2024 16:11:49 -0400 Subject: [PATCH 28/46] try setting java opts --- .../protovalidate-conformance/build.gradle.kts | 3 ++- .../protokt/v1/buf/validate/ConformanceTest.kt | 18 ++++++++++++++---- .../resources/protokt/v1/buf/validate/driver | 2 ++ 3 files changed, 18 insertions(+), 5 deletions(-) create mode 100755 testing/protovalidate-conformance/src/test/resources/protokt/v1/buf/validate/driver diff --git a/testing/protovalidate-conformance/build.gradle.kts b/testing/protovalidate-conformance/build.gradle.kts index 8e733e6d..3f3e3a4b 100644 --- a/testing/protovalidate-conformance/build.gradle.kts +++ b/testing/protovalidate-conformance/build.gradle.kts @@ -15,6 +15,7 @@ import com.google.protobuf.gradle.GenerateProtoTask import com.google.protobuf.gradle.proto +import org.gradle.api.distribution.plugins.DistributionPlugin.TASK_INSTALL_NAME plugins { id("protokt.jvm-conventions") @@ -87,6 +88,6 @@ tasks { test { systemProperty("conformance-runner", conformanceExecutable.absolutePath) outputs.upToDateWhen { false } - dependsOn(installConformance) + dependsOn(installConformance, TASK_INSTALL_NAME) } } diff --git a/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt index 9a0ecad5..4541b2f8 100644 --- a/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt +++ b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt @@ -21,18 +21,28 @@ import protokt.v1.testing.projectRoot import protokt.v1.testing.runCommand import java.nio.file.Path import java.time.Duration +import kotlin.io.path.absolutePathString class ConformanceTest { @Test fun `run conformance test`() { command() - .runCommand(projectRoot.toPath(), timeout = Duration.ofMinutes(2)) + .runCommand(projectRoot.toPath(), timeout = Duration.ofMinutes(8)) .orFail("Protovalidate conformance tests failed", ProcessOutput.Src.ERR) } } -private val driver = - Path.of(projectRoot.absolutePath, "build", "install", "protovalidate-conformance", "bin", "protovalidate-conformance") - private fun command() = "${System.getProperty("conformance-runner")} --strict_message --strict_error $driver" + +private val driver = + Path.of( + "src", + "test", + "resources", + "protokt", + "v1", + "buf", + "validate", + "driver" + ).absolutePathString() diff --git a/testing/protovalidate-conformance/src/test/resources/protokt/v1/buf/validate/driver b/testing/protovalidate-conformance/src/test/resources/protokt/v1/buf/validate/driver new file mode 100755 index 00000000..b87b3218 --- /dev/null +++ b/testing/protovalidate-conformance/src/test/resources/protokt/v1/buf/validate/driver @@ -0,0 +1,2 @@ +#!/usr/bin/env bash +exec /Users/andrew.parmet/toast/git-repos/protokt/testing/protovalidate-conformance/build/install/protovalidate-conformance/bin/protovalidate-conformance -Xmx2048m From 7d72f62bbf11c57e070a8b93bdeb51422a71b1d7 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sat, 18 May 2024 16:18:46 -0400 Subject: [PATCH 29/46] publish test results --- .github/workflows/ci.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4779234f..f1e1b20b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,6 +23,12 @@ jobs: run: npm install protobufjs@7.2.6 long@5.2.3 - name: Build and test run: ./gradlew clean check publishToIntegrationRepository --stacktrace --no-daemon + - uses: EnricoMi/publish-unit-test-result-action@v2 + if: always() && matrix.os == 'ubuntu-latest' + with: + files: "**/test-results/**/*.xml" + comment_mode: off + check_name: "Test Results" - uses: actions/upload-artifact@v4 with: name: integration-repository From abb17e56dff93b9b0b52e8ee63b5f0619736e311 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sat, 18 May 2024 16:39:21 -0400 Subject: [PATCH 30/46] always --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f1e1b20b..64a98ae1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,7 +24,7 @@ jobs: - name: Build and test run: ./gradlew clean check publishToIntegrationRepository --stacktrace --no-daemon - uses: EnricoMi/publish-unit-test-result-action@v2 - if: always() && matrix.os == 'ubuntu-latest' + if: always() with: files: "**/test-results/**/*.xml" comment_mode: off From 0394c455fc2807d8350868e074d6128d7ee44313 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sat, 18 May 2024 17:35:32 -0400 Subject: [PATCH 31/46] debugging changes --- testing/protovalidate-conformance/build.gradle.kts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/testing/protovalidate-conformance/build.gradle.kts b/testing/protovalidate-conformance/build.gradle.kts index 3f3e3a4b..71097d50 100644 --- a/testing/protovalidate-conformance/build.gradle.kts +++ b/testing/protovalidate-conformance/build.gradle.kts @@ -89,5 +89,8 @@ tasks { systemProperty("conformance-runner", conformanceExecutable.absolutePath) outputs.upToDateWhen { false } dependsOn(installConformance, TASK_INSTALL_NAME) + testLogging.showStandardStreams = true + maxHeapSize = "512m" + jvmArgs = listOf("-XX:MaxPermSize=512m") } } From 5da6b1c480d06c67bbf511a7d875daff778101f3 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sat, 18 May 2024 17:53:30 -0400 Subject: [PATCH 32/46] log failure --- testing/protovalidate-conformance/build.gradle.kts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/testing/protovalidate-conformance/build.gradle.kts b/testing/protovalidate-conformance/build.gradle.kts index 71097d50..7c42572a 100644 --- a/testing/protovalidate-conformance/build.gradle.kts +++ b/testing/protovalidate-conformance/build.gradle.kts @@ -16,6 +16,7 @@ import com.google.protobuf.gradle.GenerateProtoTask import com.google.protobuf.gradle.proto import org.gradle.api.distribution.plugins.DistributionPlugin.TASK_INSTALL_NAME +import org.gradle.api.tasks.testing.logging.TestLogEvent plugins { id("protokt.jvm-conventions") @@ -90,6 +91,7 @@ tasks { outputs.upToDateWhen { false } dependsOn(installConformance, TASK_INSTALL_NAME) testLogging.showStandardStreams = true + testLogging.events(TestLogEvent.FAILED) maxHeapSize = "512m" jvmArgs = listOf("-XX:MaxPermSize=512m") } From 9d17b3019bd7dff90081c70439a21777b07c0503 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sat, 18 May 2024 21:26:39 -0400 Subject: [PATCH 33/46] restrict driver --- testing/protovalidate-conformance/build.gradle.kts | 2 -- .../src/test/resources/protokt/v1/buf/validate/driver | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/testing/protovalidate-conformance/build.gradle.kts b/testing/protovalidate-conformance/build.gradle.kts index 7c42572a..29441bb6 100644 --- a/testing/protovalidate-conformance/build.gradle.kts +++ b/testing/protovalidate-conformance/build.gradle.kts @@ -92,7 +92,5 @@ tasks { dependsOn(installConformance, TASK_INSTALL_NAME) testLogging.showStandardStreams = true testLogging.events(TestLogEvent.FAILED) - maxHeapSize = "512m" - jvmArgs = listOf("-XX:MaxPermSize=512m") } } diff --git a/testing/protovalidate-conformance/src/test/resources/protokt/v1/buf/validate/driver b/testing/protovalidate-conformance/src/test/resources/protokt/v1/buf/validate/driver index b87b3218..7c307c5b 100755 --- a/testing/protovalidate-conformance/src/test/resources/protokt/v1/buf/validate/driver +++ b/testing/protovalidate-conformance/src/test/resources/protokt/v1/buf/validate/driver @@ -1,2 +1,2 @@ #!/usr/bin/env bash -exec /Users/andrew.parmet/toast/git-repos/protokt/testing/protovalidate-conformance/build/install/protovalidate-conformance/bin/protovalidate-conformance -Xmx2048m +exec /Users/andrew.parmet/toast/git-repos/protokt/testing/protovalidate-conformance/build/install/protovalidate-conformance/bin/protovalidate-conformance -Xmx512m From c51c0543b3af3b794156e834fe7a2e0b97a314ca Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sat, 18 May 2024 21:49:12 -0400 Subject: [PATCH 34/46] print stack trace --- .../kotlin/protokt/v1/buf/validate/ConformanceTest.kt | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt index 4541b2f8..f8959495 100644 --- a/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt +++ b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt @@ -26,9 +26,14 @@ import kotlin.io.path.absolutePathString class ConformanceTest { @Test fun `run conformance test`() { - command() - .runCommand(projectRoot.toPath(), timeout = Duration.ofMinutes(8)) - .orFail("Protovalidate conformance tests failed", ProcessOutput.Src.ERR) + try { + command() + .runCommand(projectRoot.toPath(), timeout = Duration.ofMinutes(8)) + .orFail("Protovalidate conformance tests failed", ProcessOutput.Src.ERR) + } catch (t: Throwable) { + t.printStackTrace() + throw t + } } } From b2226736495477651ed65f219fdbd837042f4b20 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Sat, 18 May 2024 22:06:50 -0400 Subject: [PATCH 35/46] improve conformance enforcement --- .../protokt/v1/conformance/ConformanceTest.kt | 15 ++++++++++++--- .../protokt/v1/buf/validate/ConformanceTest.kt | 9 +++++---- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/testing/conformance/runner/src/test/kotlin/protokt/v1/conformance/ConformanceTest.kt b/testing/conformance/runner/src/test/kotlin/protokt/v1/conformance/ConformanceTest.kt index 9770e9f2..cf593320 100644 --- a/testing/conformance/runner/src/test/kotlin/protokt/v1/conformance/ConformanceTest.kt +++ b/testing/conformance/runner/src/test/kotlin/protokt/v1/conformance/ConformanceTest.kt @@ -15,6 +15,7 @@ package protokt.v1.conformance +import com.google.common.truth.Truth.assertThat import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.EnumSource @@ -36,6 +37,7 @@ class ConformanceTest { jvmConformanceDriver }, + /* JS_IR("js-ir") { override fun driver() = jsConformanceDriver(project) @@ -47,6 +49,8 @@ class ConformanceTest { } } }, // https://github.com/pinterest/ktlint/issues/1933 + + */ ; abstract fun driver(): Path @@ -63,9 +67,14 @@ class ConformanceTest { @EnumSource fun `run conformance tests`(runner: ConformanceRunner) { try { - command(runner) - .runCommand(projectRoot.toPath()) - .orFail("Conformance tests failed", ProcessOutput.Src.ERR) + val output = command(runner).runCommand(projectRoot.toPath()) + println(output.stderr) + + assertThat(output.stderr).contains("CONFORMANCE SUITE PASSED") + val matches = " (\\d+) unexpected failures".toRegex().findAll(output.stderr).toList() + // the current implementation runs two conformance suites + assertThat(matches).hasSize(2) + matches.forEach { assertThat(it.groupValues[1].toInt()).isEqualTo(0) } } catch (t: Throwable) { if (failingTests.exists()) { println("Failing tests:\n" + failingTests.readText()) diff --git a/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt index f8959495..77dad035 100644 --- a/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt +++ b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt @@ -15,8 +15,8 @@ package protokt.v1.buf.validate +import com.google.common.truth.Truth.assertThat import org.junit.jupiter.api.Test -import protokt.v1.testing.ProcessOutput import protokt.v1.testing.projectRoot import protokt.v1.testing.runCommand import java.nio.file.Path @@ -27,9 +27,10 @@ class ConformanceTest { @Test fun `run conformance test`() { try { - command() - .runCommand(projectRoot.toPath(), timeout = Duration.ofMinutes(8)) - .orFail("Protovalidate conformance tests failed", ProcessOutput.Src.ERR) + val output = command().runCommand(projectRoot.toPath(), timeout = Duration.ofMinutes(8)) + println(output.stderr) + assertThat(output.stderr).startsWith("PASS") + assertThat(output.stderr).contains("failed: 0") } catch (t: Throwable) { t.printStackTrace() throw t From a125e7ee44cf614546745bd8bb93a8928d0c7075 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Mon, 27 May 2024 21:31:13 -0400 Subject: [PATCH 36/46] severely restrict jvm memory --- .../protokt/v1/conformance/ConformanceTest.kt | 1 - .../build.gradle.kts | 9 ++++++++ .../v1/buf/validate/conformance/Main.kt | 22 +++++++++++++++++++ .../v1/buf/validate/ConformanceTest.kt | 6 ++++- 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/testing/conformance/runner/src/test/kotlin/protokt/v1/conformance/ConformanceTest.kt b/testing/conformance/runner/src/test/kotlin/protokt/v1/conformance/ConformanceTest.kt index cf593320..b7b6d09b 100644 --- a/testing/conformance/runner/src/test/kotlin/protokt/v1/conformance/ConformanceTest.kt +++ b/testing/conformance/runner/src/test/kotlin/protokt/v1/conformance/ConformanceTest.kt @@ -19,7 +19,6 @@ import com.google.common.truth.Truth.assertThat import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.EnumSource -import protokt.v1.testing.ProcessOutput import protokt.v1.testing.projectRoot import protokt.v1.testing.runCommand import java.io.File diff --git a/testing/protovalidate-conformance/build.gradle.kts b/testing/protovalidate-conformance/build.gradle.kts index 29441bb6..5a250697 100644 --- a/testing/protovalidate-conformance/build.gradle.kts +++ b/testing/protovalidate-conformance/build.gradle.kts @@ -21,10 +21,19 @@ import org.gradle.api.tasks.testing.logging.TestLogEvent plugins { id("protokt.jvm-conventions") application + // id("org.graalvm.buildtools.native") version "0.10.2" } localProtokt(false) +/* +graalvmNative { + binaries.all { + resources.autodetect() + } +} + */ + dependencies { implementation(project(":protokt-protovalidate")) implementation(project(":protokt-reflect")) diff --git a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt index 5c3a78ad..e36a16c9 100644 --- a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt +++ b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt @@ -26,26 +26,48 @@ import com.google.protobuf.Descriptors import com.google.protobuf.ExtensionRegistry import protokt.v1.Message import protokt.v1.buf.validate.ProtoktValidator +import java.io.File +import java.time.Instant + +fun stats(): Any { + return Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory() +} + +fun withStats(act: File.() -> Unit) { + val stats = File("/Users/andrew.parmet/Desktop/protovalidate-stats.txt") + + if (false) { + if (!stats.exists()) { + stats.createNewFile() + } + stats.act() + } +} object Main { @JvmStatic fun main(args: Array) { + withStats { appendText("${Instant.now()} Starting\n") } + withStats { appendText("${Instant.now()} Env: ${System.getenv()}\n") } val extensionRegistry = ExtensionRegistry.newInstance() extensionRegistry.add(ValidateProto.message) extensionRegistry.add(ValidateProto.field) extensionRegistry.add(ValidateProto.oneof) val request = TestConformanceRequest.parseFrom(System.`in`, extensionRegistry) testConformance(request).writeTo(System.out) + withStats { appendText("${Instant.now()} Ending\n") } } private fun testConformance(request: TestConformanceRequest): TestConformanceResponse { val descriptorMap = parse(request.fdset) val validator = ProtoktValidator() loadValidDescriptors(validator, descriptorMap.values) + var idx = 0 return TestConformanceResponse .newBuilder() .putAllResults( request.casesMap.mapValues { (_, value) -> + withStats { appendText("${Instant.now()} Finished case ${++idx}; used memory: ${stats()}\n") } testCase(validator, descriptorMap, value) } ) diff --git a/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt index 77dad035..55401a78 100644 --- a/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt +++ b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt @@ -27,7 +27,11 @@ class ConformanceTest { @Test fun `run conformance test`() { try { - val output = command().runCommand(projectRoot.toPath(), timeout = Duration.ofMinutes(8)) + val output = command().runCommand( + projectRoot.toPath(), + env = mapOf("JAVA_OPTS" to "-Xmx32M"), + timeout = Duration.ofMinutes(8) + ) println(output.stderr) assertThat(output.stderr).startsWith("PASS") assertThat(output.stderr).contains("failed: 0") From 9d69198bae89816ea85ac5ab109d1b7760875686 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Mon, 27 May 2024 21:33:09 -0400 Subject: [PATCH 37/46] run directly --- .../kotlin/protokt/v1/buf/validate/ConformanceTest.kt | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt index 55401a78..86044b7d 100644 --- a/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt +++ b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt @@ -46,13 +46,4 @@ private fun command() = "${System.getProperty("conformance-runner")} --strict_message --strict_error $driver" private val driver = - Path.of( - "src", - "test", - "resources", - "protokt", - "v1", - "buf", - "validate", - "driver" - ).absolutePathString() + Path.of(projectRoot.absolutePath, "build", "install", "protovalidate-conformance", "bin", "protovalidate-conformance") From e61a0df884ec907391f54cde33f910955eaedec1 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Mon, 27 May 2024 21:52:00 -0400 Subject: [PATCH 38/46] lint --- .../src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt index 86044b7d..1c04dbd6 100644 --- a/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt +++ b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt @@ -21,7 +21,6 @@ import protokt.v1.testing.projectRoot import protokt.v1.testing.runCommand import java.nio.file.Path import java.time.Duration -import kotlin.io.path.absolutePathString class ConformanceTest { @Test From 4cd8b94c85c27f13d9236a6ffa9e937f05362603 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Wed, 29 May 2024 20:21:10 -0400 Subject: [PATCH 39/46] try restricting go runtime --- .../test/kotlin/protokt/v1/conformance/ConformanceTest.kt | 4 +--- .../test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt | 6 +++++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/testing/conformance/runner/src/test/kotlin/protokt/v1/conformance/ConformanceTest.kt b/testing/conformance/runner/src/test/kotlin/protokt/v1/conformance/ConformanceTest.kt index b7b6d09b..06db2dcd 100644 --- a/testing/conformance/runner/src/test/kotlin/protokt/v1/conformance/ConformanceTest.kt +++ b/testing/conformance/runner/src/test/kotlin/protokt/v1/conformance/ConformanceTest.kt @@ -36,7 +36,6 @@ class ConformanceTest { jvmConformanceDriver }, - /* JS_IR("js-ir") { override fun driver() = jsConformanceDriver(project) @@ -48,8 +47,6 @@ class ConformanceTest { } } }, // https://github.com/pinterest/ktlint/issues/1933 - - */ ; abstract fun driver(): Path @@ -74,6 +71,7 @@ class ConformanceTest { // the current implementation runs two conformance suites assertThat(matches).hasSize(2) matches.forEach { assertThat(it.groupValues[1].toInt()).isEqualTo(0) } + assertThat(output.exitCode).isEqualTo(0) } catch (t: Throwable) { if (failingTests.exists()) { println("Failing tests:\n" + failingTests.readText()) diff --git a/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt index 1c04dbd6..1c95bf35 100644 --- a/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt +++ b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt @@ -28,12 +28,16 @@ class ConformanceTest { try { val output = command().runCommand( projectRoot.toPath(), - env = mapOf("JAVA_OPTS" to "-Xmx32M"), + env = mapOf( + "JAVA_OPTS" to "-Xmx32M", + "GOMEMLIMIT" to "32000000" + ), timeout = Duration.ofMinutes(8) ) println(output.stderr) assertThat(output.stderr).startsWith("PASS") assertThat(output.stderr).contains("failed: 0") + assertThat(output.exitCode).isEqualTo(0) } catch (t: Throwable) { t.printStackTrace() throw t From 60302b432d798d25f4def7a4499edd91fbc55046 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Wed, 29 May 2024 20:26:22 -0400 Subject: [PATCH 40/46] bump mem --- .../src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt index 1c95bf35..2e1615b3 100644 --- a/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt +++ b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ConformanceTest.kt @@ -29,7 +29,7 @@ class ConformanceTest { val output = command().runCommand( projectRoot.toPath(), env = mapOf( - "JAVA_OPTS" to "-Xmx32M", + "JAVA_OPTS" to "-Xmx64M", "GOMEMLIMIT" to "32000000" ), timeout = Duration.ofMinutes(8) From 57e4640229e52125996741fa981d0bc6270389c9 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Wed, 29 May 2024 20:49:18 -0400 Subject: [PATCH 41/46] cleanup --- .github/workflows/ci.yml | 6 ------ .../build.gradle.kts | 12 ----------- .../v1/buf/validate/conformance/Main.kt | 20 ------------------- .../resources/protokt/v1/buf/validate/driver | 2 -- 4 files changed, 40 deletions(-) delete mode 100755 testing/protovalidate-conformance/src/test/resources/protokt/v1/buf/validate/driver diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 64a98ae1..4779234f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,12 +23,6 @@ jobs: run: npm install protobufjs@7.2.6 long@5.2.3 - name: Build and test run: ./gradlew clean check publishToIntegrationRepository --stacktrace --no-daemon - - uses: EnricoMi/publish-unit-test-result-action@v2 - if: always() - with: - files: "**/test-results/**/*.xml" - comment_mode: off - check_name: "Test Results" - uses: actions/upload-artifact@v4 with: name: integration-repository diff --git a/testing/protovalidate-conformance/build.gradle.kts b/testing/protovalidate-conformance/build.gradle.kts index 5a250697..3f3e3a4b 100644 --- a/testing/protovalidate-conformance/build.gradle.kts +++ b/testing/protovalidate-conformance/build.gradle.kts @@ -16,24 +16,14 @@ import com.google.protobuf.gradle.GenerateProtoTask import com.google.protobuf.gradle.proto import org.gradle.api.distribution.plugins.DistributionPlugin.TASK_INSTALL_NAME -import org.gradle.api.tasks.testing.logging.TestLogEvent plugins { id("protokt.jvm-conventions") application - // id("org.graalvm.buildtools.native") version "0.10.2" } localProtokt(false) -/* -graalvmNative { - binaries.all { - resources.autodetect() - } -} - */ - dependencies { implementation(project(":protokt-protovalidate")) implementation(project(":protokt-reflect")) @@ -99,7 +89,5 @@ tasks { systemProperty("conformance-runner", conformanceExecutable.absolutePath) outputs.upToDateWhen { false } dependsOn(installConformance, TASK_INSTALL_NAME) - testLogging.showStandardStreams = true - testLogging.events(TestLogEvent.FAILED) } } diff --git a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt index e36a16c9..c081f89f 100644 --- a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt +++ b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt @@ -29,45 +29,25 @@ import protokt.v1.buf.validate.ProtoktValidator import java.io.File import java.time.Instant -fun stats(): Any { - return Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory() -} - -fun withStats(act: File.() -> Unit) { - val stats = File("/Users/andrew.parmet/Desktop/protovalidate-stats.txt") - - if (false) { - if (!stats.exists()) { - stats.createNewFile() - } - stats.act() - } -} - object Main { @JvmStatic fun main(args: Array) { - withStats { appendText("${Instant.now()} Starting\n") } - withStats { appendText("${Instant.now()} Env: ${System.getenv()}\n") } val extensionRegistry = ExtensionRegistry.newInstance() extensionRegistry.add(ValidateProto.message) extensionRegistry.add(ValidateProto.field) extensionRegistry.add(ValidateProto.oneof) val request = TestConformanceRequest.parseFrom(System.`in`, extensionRegistry) testConformance(request).writeTo(System.out) - withStats { appendText("${Instant.now()} Ending\n") } } private fun testConformance(request: TestConformanceRequest): TestConformanceResponse { val descriptorMap = parse(request.fdset) val validator = ProtoktValidator() loadValidDescriptors(validator, descriptorMap.values) - var idx = 0 return TestConformanceResponse .newBuilder() .putAllResults( request.casesMap.mapValues { (_, value) -> - withStats { appendText("${Instant.now()} Finished case ${++idx}; used memory: ${stats()}\n") } testCase(validator, descriptorMap, value) } ) diff --git a/testing/protovalidate-conformance/src/test/resources/protokt/v1/buf/validate/driver b/testing/protovalidate-conformance/src/test/resources/protokt/v1/buf/validate/driver deleted file mode 100755 index 7c307c5b..00000000 --- a/testing/protovalidate-conformance/src/test/resources/protokt/v1/buf/validate/driver +++ /dev/null @@ -1,2 +0,0 @@ -#!/usr/bin/env bash -exec /Users/andrew.parmet/toast/git-repos/protokt/testing/protovalidate-conformance/build/install/protovalidate-conformance/bin/protovalidate-conformance -Xmx512m From 824775fa7603bb035cff77f58b32d09195b0894b Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Wed, 29 May 2024 20:50:56 -0400 Subject: [PATCH 42/46] lint --- .../src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt index c081f89f..5c3a78ad 100644 --- a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt +++ b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt @@ -26,8 +26,6 @@ import com.google.protobuf.Descriptors import com.google.protobuf.ExtensionRegistry import protokt.v1.Message import protokt.v1.buf.validate.ProtoktValidator -import java.io.File -import java.time.Instant object Main { @JvmStatic From a7b3f56dfd24d1bd53bb79ffbc4d3d4847a4bbba Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Wed, 29 May 2024 21:02:03 -0400 Subject: [PATCH 43/46] rename --- protokt-protovalidate/api/protokt-protovalidate.api | 2 +- .../buf/validate/{ProtoktValidator.kt => Validator.kt} | 2 +- .../kotlin/protokt/v1/buf/validate/conformance/Main.kt | 10 +++++----- ...rotoktValidatorTest.kt => AbstractValidatorTest.kt} | 4 ++-- .../v1/buf/validate/ProtoktDynamicMessageTest.kt | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) rename protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/{ProtoktValidator.kt => Validator.kt} (97%) rename testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/{AbstractProtoktValidatorTest.kt => AbstractValidatorTest.kt} (99%) diff --git a/protokt-protovalidate/api/protokt-protovalidate.api b/protokt-protovalidate/api/protokt-protovalidate.api index 507e2600..8a33e8a4 100644 --- a/protokt-protovalidate/api/protokt-protovalidate.api +++ b/protokt-protovalidate/api/protokt-protovalidate.api @@ -1,4 +1,4 @@ -public final class protokt/v1/buf/validate/ProtoktValidator { +public final class protokt/v1/buf/validate/Validator { public fun ()V public fun (Lbuild/buf/protovalidate/Config;)V public synthetic fun (Lbuild/buf/protovalidate/Config;ILkotlin/jvm/internal/DefaultConstructorMarker;)V diff --git a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/Validator.kt similarity index 97% rename from protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt rename to protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/Validator.kt index 73db6d07..477cc0f9 100644 --- a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/ProtoktValidator.kt +++ b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/Validator.kt @@ -33,7 +33,7 @@ import java.util.concurrent.ConcurrentHashMap import kotlin.reflect.full.findAnnotation @Beta -class ProtoktValidator @JvmOverloads constructor( +class Validator @JvmOverloads constructor( config: Config = Config.newBuilder().build() ) { private val evaluatorBuilder = diff --git a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt index 5c3a78ad..ee522495 100644 --- a/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt +++ b/testing/protovalidate-conformance/src/main/kotlin/protokt/v1/buf/validate/conformance/Main.kt @@ -25,7 +25,7 @@ import build.buf.validate.Violations import com.google.protobuf.Descriptors import com.google.protobuf.ExtensionRegistry import protokt.v1.Message -import protokt.v1.buf.validate.ProtoktValidator +import protokt.v1.buf.validate.Validator object Main { @JvmStatic @@ -40,7 +40,7 @@ object Main { private fun testConformance(request: TestConformanceRequest): TestConformanceResponse { val descriptorMap = parse(request.fdset) - val validator = ProtoktValidator() + val validator = Validator() loadValidDescriptors(validator, descriptorMap.values) return TestConformanceResponse .newBuilder() @@ -52,7 +52,7 @@ object Main { .build() } - private fun loadValidDescriptors(validator: ProtoktValidator, descriptors: Iterable) { + private fun loadValidDescriptors(validator: Validator, descriptors: Iterable) { descriptors.forEach { try { validator.load(it) @@ -63,7 +63,7 @@ object Main { } private fun testCase( - validator: ProtoktValidator, + validator: Validator, fileDescriptors: Map, testCase: com.google.protobuf.Any ): TestResult { @@ -80,7 +80,7 @@ object Main { return validate(validator, DynamicConcreteMessageDeserializer.parse(fullName, testCase.value)) } - private fun validate(validator: ProtoktValidator, message: Message) = + private fun validate(validator: Validator, message: Message) = try { val result = validator.validate(message) if (result.isSuccess) { diff --git a/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/AbstractProtoktValidatorTest.kt b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/AbstractValidatorTest.kt similarity index 99% rename from testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/AbstractProtoktValidatorTest.kt rename to testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/AbstractValidatorTest.kt index cb66d9a1..d1287759 100644 --- a/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/AbstractProtoktValidatorTest.kt +++ b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/AbstractValidatorTest.kt @@ -44,8 +44,8 @@ import protokt.v1.buf.validate.conformance.cases.repeated_file_descriptor import protokt.v1.buf.validate.conformance.cases.strings_file_descriptor import protokt.v1.google.protobuf.FileDescriptor -abstract class AbstractProtoktValidatorTest { - protected val validator = ProtoktValidator() +abstract class AbstractValidatorTest { + protected val validator = Validator() abstract fun validate(message: Message): ValidationResult diff --git a/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ProtoktDynamicMessageTest.kt b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ProtoktDynamicMessageTest.kt index efc3ee1e..f0e20c0f 100644 --- a/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ProtoktDynamicMessageTest.kt +++ b/testing/protovalidate-conformance/src/test/kotlin/protokt/v1/buf/validate/ProtoktDynamicMessageTest.kt @@ -17,7 +17,7 @@ package protokt.v1.buf.validate import protokt.v1.Message -class ProtoktDynamicMessageTest : AbstractProtoktValidatorTest() { +class ProtoktDynamicMessageTest : AbstractValidatorTest() { override fun validate(message: Message) = validator.validate(message) } From ce4e1cd5cd31a78cf96d1cf2799dde82cef62060 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Wed, 29 May 2024 21:08:42 -0400 Subject: [PATCH 44/46] meh, don't be mutable --- .../kotlin/protokt/v1/buf/validate/Validator.kt | 15 ++++++++++++--- protokt-reflect/api/protokt-reflect.api | 1 - .../protokt/v1/google/protobuf/RuntimeContext.kt | 7 +------ 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/Validator.kt b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/Validator.kt index 477cc0f9..b656f85f 100644 --- a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/Validator.kt +++ b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/Validator.kt @@ -29,6 +29,7 @@ import protokt.v1.GeneratedMessage import protokt.v1.Message import protokt.v1.google.protobuf.RuntimeContext import protokt.v1.google.protobuf.toDynamicMessage +import java.util.Collections import java.util.concurrent.ConcurrentHashMap import kotlin.reflect.full.findAnnotation @@ -45,12 +46,20 @@ class Validator @JvmOverloads constructor( private val failFast = config.isFailFast private val evaluatorsByFullTypeName = ConcurrentHashMap() - private val runtimeContext = RuntimeContext(emptyList()) + private val descriptors = Collections.newSetFromMap(ConcurrentHashMap()) + + @Volatile + private var runtimeContext = RuntimeContext(emptyList()) fun load(descriptor: Descriptor) { - runtimeContext.add(descriptor) + doLoad(descriptor) + runtimeContext = RuntimeContext(descriptors) + } + + private fun doLoad(descriptor: Descriptor) { + descriptors.add(descriptor) evaluatorsByFullTypeName[descriptor.fullName] = evaluatorBuilder.load(descriptor) - descriptor.nestedTypes.forEach(::load) + descriptor.nestedTypes.forEach(::doLoad) } fun validate(message: Message): ValidationResult = diff --git a/protokt-reflect/api/protokt-reflect.api b/protokt-reflect/api/protokt-reflect.api index 754e6d4c..d951bdb0 100644 --- a/protokt-reflect/api/protokt-reflect.api +++ b/protokt-reflect/api/protokt-reflect.api @@ -1240,7 +1240,6 @@ public final class protokt/v1/google/protobuf/Messages { public final class protokt/v1/google/protobuf/RuntimeContext { public fun (Ljava/lang/Iterable;)V - public final fun add (Lcom/google/protobuf/Descriptors$Descriptor;)V public final fun convertValue (Ljava/lang/Object;)Ljava/lang/Object; } diff --git a/protokt-reflect/src/jvmMain/kotlin/protokt/v1/google/protobuf/RuntimeContext.kt b/protokt-reflect/src/jvmMain/kotlin/protokt/v1/google/protobuf/RuntimeContext.kt index 1d3fc452..b1c04630 100644 --- a/protokt-reflect/src/jvmMain/kotlin/protokt/v1/google/protobuf/RuntimeContext.kt +++ b/protokt-reflect/src/jvmMain/kotlin/protokt/v1/google/protobuf/RuntimeContext.kt @@ -37,7 +37,6 @@ import protokt.v1.reflect.WellKnownTypes import protokt.v1.reflect.inferClassName import protokt.v1.reflect.resolvePackage import protokt.v1.reflect.typeName -import java.util.concurrent.ConcurrentHashMap import kotlin.Any import kotlin.reflect.full.findAnnotation @@ -48,11 +47,7 @@ class RuntimeContext internal constructor( ) { constructor(descriptors: Iterable) : this(descriptors, ClassLookup(emptyList())) - internal val descriptorsByTypeName = ConcurrentHashMap(descriptors.associateBy { it.fullName }) - - fun add(descriptor: Descriptors.Descriptor) { - descriptorsByTypeName[descriptor.fullName] = descriptor - } + internal val descriptorsByTypeName = descriptors.associateBy { it.fullName } fun convertValue(value: Any) = when (value) { From 1421ef4106e9b55a4ac01172aec0aae71ce3d3c8 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Wed, 29 May 2024 21:21:10 -0400 Subject: [PATCH 45/46] make users synchronize --- .../src/main/kotlin/protokt/v1/buf/validate/Validator.kt | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/Validator.kt b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/Validator.kt index b656f85f..6758d44e 100644 --- a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/Validator.kt +++ b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/Validator.kt @@ -29,8 +29,6 @@ import protokt.v1.GeneratedMessage import protokt.v1.Message import protokt.v1.google.protobuf.RuntimeContext import protokt.v1.google.protobuf.toDynamicMessage -import java.util.Collections -import java.util.concurrent.ConcurrentHashMap import kotlin.reflect.full.findAnnotation @Beta @@ -45,10 +43,9 @@ class Validator @JvmOverloads constructor( private val failFast = config.isFailFast - private val evaluatorsByFullTypeName = ConcurrentHashMap() - private val descriptors = Collections.newSetFromMap(ConcurrentHashMap()) + private val evaluatorsByFullTypeName = mutableMapOf() + private val descriptors = mutableSetOf() - @Volatile private var runtimeContext = RuntimeContext(emptyList()) fun load(descriptor: Descriptor) { From 37f96388ee2c954c050c9bbd6af2f1b68655daed Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Tue, 11 Jun 2024 21:04:10 -0400 Subject: [PATCH 46/46] meh, make it thread safe --- .../src/main/kotlin/protokt/v1/buf/validate/Validator.kt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/Validator.kt b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/Validator.kt index 6758d44e..b656f85f 100644 --- a/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/Validator.kt +++ b/protokt-protovalidate/src/main/kotlin/protokt/v1/buf/validate/Validator.kt @@ -29,6 +29,8 @@ import protokt.v1.GeneratedMessage import protokt.v1.Message import protokt.v1.google.protobuf.RuntimeContext import protokt.v1.google.protobuf.toDynamicMessage +import java.util.Collections +import java.util.concurrent.ConcurrentHashMap import kotlin.reflect.full.findAnnotation @Beta @@ -43,9 +45,10 @@ class Validator @JvmOverloads constructor( private val failFast = config.isFailFast - private val evaluatorsByFullTypeName = mutableMapOf() - private val descriptors = mutableSetOf() + private val evaluatorsByFullTypeName = ConcurrentHashMap() + private val descriptors = Collections.newSetFromMap(ConcurrentHashMap()) + @Volatile private var runtimeContext = RuntimeContext(emptyList()) fun load(descriptor: Descriptor) {