Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

MethodDescriptor: add input and output types #1503

Merged
merged 3 commits into from
Apr 10, 2023
Merged

MethodDescriptor: add input and output types #1503

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3fdbd29
MethodDescriptor: add input and output descriptors
jordancoll Mar 27, 2023
e997a32
return descriptors directly
jordancoll Apr 5, 2023
98521f8
add details to thrown exceptions
jordancoll Apr 9, 2023
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
7 changes: 2 additions & 5 deletions e2e-grpc/src/test/scala/MethodDescriptorSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,13 @@ import com.thesamet.proto.e2e.type_level.XYMessage
import org.scalatest.flatspec.AnyFlatSpec
import org.scalatest.matchers.must.Matchers
import org.scalatest.{LoneElement, OptionValues}
import scalapb.descriptors.ScalaType

class MethodDescriptorSpec extends AnyFlatSpec with Matchers with LoneElement with OptionValues {

"scala descriptor" must "expose correct input and output message descriptors" in {
val unaryMethod = Service1Grpc.Service1.scalaDescriptor.methods.find(_.name == "CustomUnary").get
val inputDescriptor = XYMessage.scalaDescriptor
val outputDescriptor = Res5.scalaDescriptor

unaryMethod.inputType mustBe ScalaType.Message(inputDescriptor)
unaryMethod.outputType mustBe ScalaType.Message(outputDescriptor)
unaryMethod.inputType must be theSameInstanceAs XYMessage.scalaDescriptor
unaryMethod.outputType must be theSameInstanceAs Res5.scalaDescriptor
}
}
32 changes: 14 additions & 18 deletions scalapb-runtime/src/main/scala/scalapb/descriptors/Descriptor.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,34 +149,30 @@ class MethodDescriptor private[descriptors] (

def getOptions = asProto.getOptions

lazy val inputType: ScalaType = {
lazy val inputType: Descriptor = {
val inputType = asProto.inputType.getOrElse(
throw new DescriptorValidationException(this, "missing method input type")
Copy link
Contributor

Choose a reason for hiding this comment

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

s"Missing method input type for method $fullName"

)

ScalaType.Message(
FileDescriptor
.find(containingService.file, this, inputType)
.getOrElse(
throw new DescriptorValidationException(this, "couldn't build input type descriptor")
)
.asInstanceOf[Descriptor]
)
FileDescriptor
.find(containingService.file, this, inputType)
.getOrElse(
Copy link
Contributor

Choose a reason for hiding this comment

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

lets use pattern matching on case Some(d: Descriptor) to make the asInstanceOf unnecessary and handle the case that what's being found isn't a descriptor.

throw new DescriptorValidationException(this, "couldn't build input type descriptor")
Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't ... for $fullName

)
.asInstanceOf[Descriptor]
}

lazy val outputType: ScalaType = {
lazy val outputType: Descriptor = {
val outputType = asProto.outputType.getOrElse(
throw new DescriptorValidationException(this, "missing method output type")
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing

)

ScalaType.Message(
FileDescriptor
.find(containingService.file, this, outputType)
.getOrElse(
throw new DescriptorValidationException(this, "couldn't build output type descriptor")
)
.asInstanceOf[Descriptor]
)
FileDescriptor
.find(containingService.file, this, outputType)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as earlier comments (capital letter, avoid asInstanceOf through pattern matching)

.getOrElse(
throw new DescriptorValidationException(this, "couldn't build output type descriptor")
)
.asInstanceOf[Descriptor]
}
}

Expand Down