Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Write the error output from Protobuf parse exceptions #83

Merged
merged 8 commits into from
Aug 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ final case class AvroSrcGenerator(
s"${protocol.getNamespace.replace('.', '/')}/${protocol.getName}$ScalaFileExtension"

val schemaGenerator = if (protocol.getMessages.isEmpty) adtGenerator else mainGenerator

val schemaLines = schemaGenerator
.protocolToStrings(protocol)
.mkString
Expand Down Expand Up @@ -163,7 +164,7 @@ final case class AvroSrcGenerator(
outputPath -> outputCode
}

def validateRequest(request: Schema): ErrorsOr[String] = {
private def validateRequest(request: Schema): ErrorsOr[String] = {
val requestArgs = request.getFields.asScala
requestArgs.toList match {
case Nil =>
Expand All @@ -177,7 +178,7 @@ final case class AvroSrcGenerator(
}
}

def validateResponse(response: Schema): ErrorsOr[String] = {
private def validateResponse(response: Schema): ErrorsOr[String] = {
response.getType match {
case Schema.Type.NULL =>
EmptyType.validNel
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ import higherkindness.skeuomorph.protobuf.{ProtobufF, Protocol}

object ProtoSrcGenerator {

final case class ProtobufSrcGenException(message: String) extends NoStackTrace
final case class ProtobufSrcGenException(message: String) extends NoStackTrace {
override def toString: String = s"ProtoBufSrcGenException: $message"
}

def build(
compressionTypeGen: CompressionTypeGen,
Expand Down
19 changes: 19 additions & 0 deletions core/src/test/resources/avro/Invalid.avdl
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
@namespace("foo.bar")
protocol MyGreeterService {

record HelloRequest {
string arg1;
union { null, string } arg2;
array<string> arg3;
}

record HelloResponse {
string arg1;
union { null, string } arg2;
array<string> arg3;
}

string sayHelloAvro(foo.bar.HelloRequest arg);

void sayNothingAvro();
}
6 changes: 6 additions & 0 deletions core/src/test/resources/proto/broken.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
syntax = "proto3";

message Broken {
string name = 1;
string nick = 2;
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,18 @@

package higherkindness.mu.rpc.srcgen

import cats.data.Validated
import cats.data.Validated.Valid

import scala.io._
import higherkindness.mu.rpc.srcgen.AvroScalaGeneratorArbitrary._
import higherkindness.mu.rpc.srcgen.Model.ScalaBigDecimalTaggedGen
import higherkindness.mu.rpc.srcgen.Model.SerializationType.Avro
import higherkindness.mu.rpc.srcgen.Model.{
BigDecimalAvroMarshallers,
NoCompressionGen,
ScalaBigDecimalTaggedGen,
UseIdiomaticEndpoints
}
import higherkindness.mu.rpc.srcgen.avro._
import org.scalacheck.Prop.forAll
import org.scalatest._
Expand All @@ -37,6 +44,29 @@ class AvroSrcGenTests extends AnyWordSpec with Matchers with OneInstancePerTest
forAll { scenario: Scenario => test(scenario) }
}
}

"return a non-empty list of errors instead of generating code from an invalid IDL file" in {
val response = {
AvroSrcGenerator(
List(BigDecimalAvroMarshallers),
ScalaBigDecimalTaggedGen,
NoCompressionGen,
UseIdiomaticEndpoints.trueV
).generateFrom(
Source.fromInputStream(getClass.getResourceAsStream("/avro/Invalid.avdl")).mkString,
Avro
)
}

response shouldBe Some(
(
"foo/bar/MyGreeterService.scala",
Validated.invalidNel(
"RPC method response parameter has non-record response type 'STRING'"
)
)
)
}
}

private def test(scenario: Scenario): Boolean = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import higherkindness.mu.rpc.srcgen.Model.{
UseIdiomaticEndpoints
}
import higherkindness.mu.rpc.srcgen.proto.ProtoSrcGenerator
import higherkindness.mu.rpc.srcgen.proto.ProtoSrcGenerator.ProtobufSrcGenException
import org.scalatest._
import org.scalatest.matchers.should.Matchers
import org.scalatest.wordspec.AnyWordSpec
Expand Down Expand Up @@ -75,6 +76,22 @@ class ProtoSrcGenTests extends AnyWordSpec with Matchers with OneInstancePerTest
result shouldBe Some(("com/proto/book.scala", expectedFileContent))
}

"throw an exception on an incorrect Protobuf schema" in {
assertThrows[ProtobufSrcGenException] {
ProtoSrcGenerator
.build(
NoCompressionGen,
UseIdiomaticEndpoints(false),
MonixObservable,
new java.io.File(".")
)
.generateFrom(
files = Set(protoFile("broken")),
serializationType = SerializationType.Protobuf
)
}
}

}

def bookExpectation(streamOf: String => String): String =
Expand Down