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
feat(spring-sdk): provide access to current state in views #1092
feat(spring-sdk): provide access to current state in views #1092
Conversation
8c6cdeb
to
c7e0cd9
Compare
c7e0cd9
to
497baff
Compare
f853ac0
to
a76f6e0
Compare
Updated now to allow for passing the |
591ff07
to
d7a4a02
Compare
sdk/spring-sdk/src/it/java/com/example/wiring/SpringSdkWiringIntegrationTest.java
Outdated
Show resolved
Hide resolved
834a958
to
dada7fa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I did a quick review. Good to have another ✅ before merging
samples/spring-wiring-tests/src/main/java/wiring/view/UserWithVersion.java
Outdated
Show resolved
Hide resolved
@@ -95,9 +95,9 @@ case class VirtualServiceMethod(component: Class[_], methodName: String, inputTy | |||
* Build from methods annotated with @Subscription. Those methods are not annotated with Spring REST annotations, but | |||
* they become a REST method at the end. | |||
*/ | |||
case class RestServiceMethod(javaMethod: Method) extends AnyServiceMethod { | |||
case class RestServiceMethod(javaMethod: Method, inputTypeParamIndex: Int = 0) extends AnyServiceMethod { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahah!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what was that? :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I added one h
to much
It was intended to be: Aha!
Like in: Aha! Now we have a solution for skipping the first arg and extracting the next one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aha! Now I get it! :D
sdk/spring-sdk/src/it/java/com/example/wiring/SpringSdkWiringIntegrationTest.java
Outdated
Show resolved
Hide resolved
…ntegrationTest.java Co-authored-by: Renato Cavalcanti <renato@cavalcanti.be>
5c720fd
to
fc19aa9
Compare
sample-spring-wiring-tests: | ||
machine: | ||
image: ubuntu-2004:202201-02 | ||
steps: | ||
- checkout-and-merge-to-main | ||
- setup_sbt | ||
- restore_deps_cache | ||
# note: `copy-from-workspace` requires `publish-local` to be run first as declared in workflow section below | ||
- copy-from-workspace | ||
- set-sdk-version | ||
- run: | ||
name: Spring SDK wiring tests | ||
command: | | ||
cd samples/spring-wiring-tests | ||
echo "Running mvn with SDK version: '$SDK_VERSION'" | ||
# no testing for now, only 'compile' | ||
mvn -Dkalix-sdk.version=$SDK_VERSION verify -Pit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -17,12 +17,15 @@ | |||
package kalix.springsdk.impl.view | |||
|
|||
import com.google.protobuf.any.{ Any => ScalaPbAny } | |||
import kalix.javasdk.Metadata | |||
import kalix.javasdk.{ JsonSupport, Metadata } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
two lines, one line? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure what you mean :S would you elaborate, please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, you mean two separate lines?
I mean, I guess this should be automatic.. otherwise it's a pain.. I think this is fine if no checks are triggered. I checked that we have other instances in the code of using this format.
References #1091
This MR: