Skip to content

feat(scalasdk): use package imports for views#448

Merged
patriknw merged 1 commit intomainfrom
wip-view-fqn-patriknw
Sep 22, 2021
Merged

feat(scalasdk): use package imports for views#448
patriknw merged 1 commit intomainfrom
wip-view-fqn-patriknw

Conversation

@patriknw
Copy link
Copy Markdown
Contributor

Similar to #443

  • Refactoring of TestData for more flexibility around the parent PackageNaming,
    javaOuterClassName

* Refactoring of TestData for more flexibility around the parent PackageNaming,
  javaOuterClassName
package com.lightbend.akkasls.codegen

class PackagingSuite extends munit.FunSuite {
private val testData = TestData()
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

TestData became a class (instead of object) so that it can be initialized with defaults (especially the difference between javaOuterClassName for Java vs Scala)

| eventName: String,
| state: ${qualifiedType(view.state.fqn)},
| event: Any): View.UpdateEffect[${qualifiedType(view.state.fqn)}] = {
| state: ${typeName(view.state.fqn)},
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

using the new typeName which takes the Imports in implicit scope

|
| def created(
| state: ServiceOuterClass.ViewState, entityCreated: EntityOuterClass.EntityCreated): View.UpdateEffect[ServiceOuterClass.ViewState]
| state: ViewState, entityCreated: EntityCreated): View.UpdateEffect[ViewState]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this PR solves the imports for the things from domain, such as the EntityCreated here.

|
| override final def serviceDescriptor: Descriptors.ServiceDescriptor =
| ServiceOuterClass.getDescriptor().findServiceByName("MyService")
| MyServiceProto.javaDescriptor.findServiceByName("MyService")
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is still wrong, but let's tackle that in a separate PR.
With ScalaPB the descriptors can be found in *Proto classes. In the sample it should use CustomerViewProto where I think CustomerView is derived from the file name customer_view.proto.

|
| override final def serviceDescriptor: Descriptors.ServiceDescriptor =
| ${view.fqn.parent.javaOuterClassname}.getDescriptor().findServiceByName("${view.fqn.protoName}")
| ${view.fqn.parent.name}Proto.javaDescriptor.findServiceByName("${view.fqn.protoName}")
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is still wrong, but let's tackle that in a separate PR.
With ScalaPB the descriptors can be found in *Proto classes. In the sample it should use CustomerViewProto where I think CustomerView is derived from the file name customer_view.proto.

The above code results in CustomerByNameProto instead of CustomerViewProto, i.e. view.fqn.parent.name is the rpc service name.


package customer.api;

option java_outer_classname = "CustomerApi";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not keep them? They're ignored for Scala anyway, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok, I was uncertain if they were fully ignored or not. However, since these are to be shown in doc snippets it would be confusing to include them when they are not used.

lazy val javaPackage: String = javaPackageOption.getOrElse(pkg)
lazy val scalaPackage: String = javaPackage
def scalaPackage: String = javaPackage
def javaOuterClassname: String = javaOuterClassnameOption.getOrElse(name)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@patriknw
Copy link
Copy Markdown
Contributor Author

Merging this, and @raboof will look into the Descriptor issue separately.

@patriknw patriknw merged commit 96cbe80 into main Sep 22, 2021
@patriknw patriknw deleted the wip-view-fqn-patriknw branch September 22, 2021 09:04
@raboof
Copy link
Copy Markdown
Contributor

raboof commented Sep 22, 2021

Merging this, and @raboof will look into the Descriptor issue separately.

#452

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants