feat: Initial Java runtime implementation #2318

stuartwdouglas · 2024-08-12T01:22:00Z

This is still very much a work in progress, however it contains a lot of basic functionality.

So far this includes support for:

Verb invocations
HTTP ingress
Cron
Topics and Subscriptions
Basic testing of Verbs

The existing Kotlin example has been migrated over to the new approach. At the moment the module is called Java even though it supports both, in future this will provide a base layer of functionality with some small language dependent features in separate Java/Kotlin modules.

alecthomas · 2024-08-12T01:47:50Z

java-runtime/.gitignore

@@ -0,0 +1,6 @@
+# Ignore Gradle project-specific cache directory
+.gradle


Nooooooooooooooooooooooooooo

This is the auto generated quarkus .gitignore, but it is coming at some point.

alecthomas

LGTM!

I'd let @worstell have a pass over the Java code.

alecthomas · 2024-08-12T03:20:13Z

java-runtime/devel.go

+)
+
+// Files is the FTL Java runtime scaffolding files.
+func Files() *zip.Reader { return internal.ZipRelativeToCaller("scaffolding") }


I don't think this directory exists does it? Can we get rid of Files() altogether?

I will remove it, I can re-add when it I add scaffolding.

alecthomas · 2024-08-12T03:21:21Z

examples/kotlin/echo/src/main/ftl-module-schema/builtin.pb

Just noting for future reference that this is a temporary situation until we figure out a better approach (eg. package servers).

alecthomas · 2024-08-12T03:23:10Z

Overall LGTM!

I couldn't quite tell how topics (and other resources) in other modules are codegenned? Could be something for later, or I missed it.

Once it's a bit more stable it might be an idea to add integration tests.

alecthomas · 2024-08-12T03:24:07Z

I'd also be interested to see how we're going to write more complex inter-verb/module tests with the injection approach. I think we will at some point need an equivalent to the ModuleContext in Go.

stuartwdouglas · 2024-08-12T03:28:48Z

Overall LGTM!

I couldn't quite tell how topics (and other resources) in other modules are codegenned? Could be something for later, or I missed it.

Topics are not quite there yet (well subscriptions really, the Topic). I have a plan for how to handle them, I should get to it soon.

Once it's a bit more stable it might be an idea to add integration tests.

Absolutely, I especially want to test interop with a go based module to make sure that everything is serializing / generating correctly. There is still heaps of work to do on tests.

gak · 2024-08-12T03:42:37Z

examples/kotlin/echo/src/main/kotlin/ftl/echo/Echo.kt

-fun echo(context: Context, req: EchoRequest): EchoResponse {
-  val response = context.call(::time, Empty())
+@Verb
+fun echo(req: EchoRequest, time: TimeClient): EchoResponse {


Looks like ctx is removed and replaced with DI?

Yes, you directly reference the resources you need. This allows us to explicitly list this in the schema, as there is no way to get these resources without referencing them in the signature.

gak · 2024-08-12T03:44:47Z

...time/ftl-runtime/deployment/src/main/java/xyz/block/ftl/deployment/VerbClientsProcessor.java

+                            }
+                            var publish = cc.getMethodCreator("call", void.class);
+                            var helper = publish.invokeStaticMethod(MethodDescriptor.ofMethod(VerbClientHelper.class, "instance", VerbClientHelper.class));
+                            publish.invokeVirtualMethod(MethodDescriptor.ofMethod(VerbClientHelper.class, "call", Object.class, String.class, String.class, Object.class, Class.class, boolean.class, boolean.class), helper, publish.load(name), publish.load(module), publish.loadNull(), publish.loadClass(Void.class), publish.load(false), publish.load(false));


This is a long line! Would it be useful to split it up a bit?

I need to setup auto formatting.

gak · 2024-08-12T03:50:12Z

...time/ftl-runtime/deployment/src/main/java/xyz/block/ftl/deployment/VerbClientsProcessor.java

+                    if (i.kind() == Type.Kind.PARAMETERIZED_TYPE) {
+                        var returnType = i.asParameterizedType().arguments().get(1);
+                        var paramType = i.asParameterizedType().arguments().get(0);
+                        try (ClassCreator cc = new ClassCreator(classOutput, iface.name().toString() + "_fit_verbclient", null, Object.class.getName(), iface.name().toString())) {
+                            if (launchModeBuildItem.isTest()) {
+                                cc.addAnnotation(TEST_ANNOTATION);
+                                cc.addAnnotation(Singleton.class);
+                            }
+                            var publish = cc.getMethodCreator("call", returnType.name().toString(), paramType.name().toString());
+                            var helper = publish.invokeStaticMethod(MethodDescriptor.ofMethod(VerbClientHelper.class, "instance", VerbClientHelper.class));
+                            var results = publish.invokeVirtualMethod(MethodDescriptor.ofMethod(VerbClientHelper.class, "call", Object.class, String.class, String.class, Object.class, Class.class, boolean.class, boolean.class), helper, publish.load(name), publish.load(module), publish.getMethodParam(0), publish.loadClass(returnType.name().toString()), publish.load(false), publish.load(false));
+                            publish.returnValue(results);
+                            publish = cc.getMethodCreator("call", Object.class, Object.class);
+                            helper = publish.invokeStaticMethod(MethodDescriptor.ofMethod(VerbClientHelper.class, "instance", VerbClientHelper.class));
+                            results = publish.invokeVirtualMethod(MethodDescriptor.ofMethod(VerbClientHelper.class, "call", Object.class, String.class, String.class, Object.class, Class.class, boolean.class, boolean.class), helper, publish.load(name), publish.load(module), publish.getMethodParam(0), publish.loadClass(returnType.name().toString()), publish.load(false), publish.load(false));
+                            publish.returnValue(results);
+                            clients.put(iface.name(), new VerbClientBuildItem.DiscoveredClients(name, module,cc.getClassName()));
+                        }
+                        found = true;
+                        break;
+                    } else {
+                        throw new RuntimeException("@VerbClientDefinition can only be applied to interfaces that directly extend a verb client type with concrete type parameters and " + iface.name() + " does not have concrete type parameters");
+                    }


Could all these if blocks be refactored out into a factory of sorts? There's a lot of very similar calls and variables. Maybe some DRY if it's appropriate?

Even though it all looks similar each line is different enough that it can't really be turned generic. Basically this is due to the fact that there is 4 different client signatures, and each once needs a slightly different implementation.

gak · 2024-08-12T03:53:31Z

...l-runtime/test-framework/src/main/java/xyz/block/ftl/java/test/internal/FTLTestResource.java

+    public Map<String, String> start() {
+        server = new FTLTestServer();
+        server.start();
+        return Map.of("ftl.endpoint", "http://localhost:" + server.getPort());


This is stupid but maybe use 127.0.0.1 instead because of the mac firewall? #2298

Ah if this is a client never mind.

worstell · 2024-08-12T19:29:29Z

java-runtime/ftl-runtime/deployment/src/main/java/xyz/block/ftl/deployment/FtlProcessor.java

+
+        output = outputTargetBuildItem.getOutputDirectory().resolve("main");
+        try (var out = Files.newOutputStream(output)) {
+            //            out.write("""


nit: should this be deleted?

worstell · 2024-08-12T19:55:06Z

common/moduleconfig/moduleconfig.go

@@ -37,6 +40,8 @@ type ModuleConfig struct {
 	Deploy []string `toml:"deploy"`
 	// DeployDir is the directory to deploy from, relative to the module directory.
 	DeployDir string `toml:"deploy-dir"`
+	// GeneratedSchemaDir is the directory to generate protobuf schema files into. These can be picked up by language specific build tools
+	GeneratedSchemaDir string `toml:"generated-schema-dir"`


what is the function of GeneratedSchemaDir? is it needed rather than using the existing Schema field in this config?

Basically I dump the protos for the external modules I need to generate code for into this dir, and then run the code generation in Maven. It means you can check them into source control and have a build that can run without needing an FTL instance with those modules installed present.

could we use the global build directory at <projectRoot>/.ftl for this? in Go we put all the external module stubs in .ftl/go

I could try that later, but this approach follows the standard maven code generation convention. It also allows for different modules in a multi module project to only generate clients for a subset of the modules on the server (this is not implemented yet). I would like to keep this approach for now to see how it works, and possibly re-visit it later.

worstell · 2024-08-12T20:35:27Z

java-runtime/ftl-runtime/runtime/src/main/java/xyz/block/ftl/runtime/FTLHttpHandler.java

+
+@SuppressWarnings("unused")
+@Singleton
+public class FTLHttpHandler implements VerbInvoker {


it looks like http ingress verbs will be registered to use this handler on invocation, is that right?

why does java/kotlin need its own ingress logic? shouldn't we be able to invoke these verbs as normal on the runtime side, since the FTL controller will be responsible for serving the HTTP endpoint and routing the inbound ingress requests appropriately?

This is purely about the Java API that is used (i.e. allowing you to use JAX-RS instead of a custom HTTP annotations). From the invocations side everything works as normal.

This is still very much a work in progress, however it contains a lot of basic functionality. So far this includes support for: - Verb invocations - HTTP ingress - Cron - Topics and Subscriptions - Basic testing of Verbs The existing Kotlin example has been migrated over to the new approach. At the moment the module is called Java even though it supports both, in future this will provide a base layer of functionality with some small language dependent features in separate Java/Kotlin modules.

github-actions bot changed the title ~~Initial Java runtime implementation~~ feat: Initial Java runtime implementation Aug 12, 2024

ftl-robot mentioned this pull request Aug 12, 2024

Dashboard #728

Open

stuartwdouglas force-pushed the stuartwdouglas/experimental-quarkus-runtime branch 3 times, most recently from eebabf2 to dff1bd2 Compare August 12, 2024 02:24

stuartwdouglas marked this pull request as ready for review August 12, 2024 02:29

stuartwdouglas requested a review from alecthomas as a code owner August 12, 2024 02:29

stuartwdouglas requested review from a team and gak and removed request for a team August 12, 2024 02:29

alecthomas reviewed Aug 12, 2024

View reviewed changes

alecthomas approved these changes Aug 12, 2024

View reviewed changes

alecthomas requested a review from worstell August 12, 2024 03:22

stuartwdouglas force-pushed the stuartwdouglas/experimental-quarkus-runtime branch from dff1bd2 to f5e3538 Compare August 12, 2024 03:29

gak reviewed Aug 12, 2024

View reviewed changes

gak approved these changes Aug 12, 2024

View reviewed changes

stuartwdouglas force-pushed the stuartwdouglas/experimental-quarkus-runtime branch 2 times, most recently from 4ead6b4 to 13c8ca7 Compare August 12, 2024 04:56

worstell reviewed Aug 12, 2024

View reviewed changes

stuartwdouglas force-pushed the stuartwdouglas/experimental-quarkus-runtime branch from d25dde4 to 4b84c0f Compare August 12, 2024 20:56

worstell self-requested a review August 12, 2024 21:10

worstell approved these changes Aug 12, 2024

View reviewed changes

stuartwdouglas added this pull request to the merge queue Aug 12, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 12, 2024

stuartwdouglas added 3 commits August 13, 2024 08:52

chore: initial Java runtime support

a862275

chore: add formatter

7a7c33a

feat: support for subscription meta annotations

0ef6c23

stuartwdouglas force-pushed the stuartwdouglas/experimental-quarkus-runtime branch 2 times, most recently from 120f16d to ebdebe8 Compare August 12, 2024 23:00

stuartwdouglas enabled auto-merge August 12, 2024 23:01

stuartwdouglas added this pull request to the merge queue Aug 12, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 12, 2024

feat: retry and catch

5ce8f26

stuartwdouglas force-pushed the stuartwdouglas/experimental-quarkus-runtime branch from ebdebe8 to 5ce8f26 Compare August 13, 2024 01:29

stuartwdouglas added this pull request to the merge queue Aug 13, 2024

Merged via the queue into main with commit 7b6a421 Aug 13, 2024
17 checks passed

stuartwdouglas deleted the stuartwdouglas/experimental-quarkus-runtime branch August 13, 2024 01:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Initial Java runtime implementation #2318

feat: Initial Java runtime implementation #2318

stuartwdouglas commented Aug 12, 2024

alecthomas Aug 12, 2024

stuartwdouglas Aug 12, 2024

alecthomas left a comment

alecthomas Aug 12, 2024

stuartwdouglas Aug 12, 2024

alecthomas Aug 12, 2024

alecthomas commented Aug 12, 2024

alecthomas commented Aug 12, 2024

stuartwdouglas commented Aug 12, 2024

gak Aug 12, 2024

stuartwdouglas Aug 12, 2024

gak Aug 12, 2024

stuartwdouglas Aug 12, 2024

gak Aug 12, 2024

stuartwdouglas Aug 12, 2024

gak Aug 12, 2024

gak Aug 12, 2024

stuartwdouglas Aug 12, 2024

worstell Aug 12, 2024

worstell Aug 12, 2024

stuartwdouglas Aug 12, 2024

worstell Aug 12, 2024

stuartwdouglas Aug 12, 2024

worstell Aug 12, 2024

stuartwdouglas Aug 12, 2024

worstell Aug 12, 2024

		@@ -0,0 +1,6 @@
		# Ignore Gradle project-specific cache directory
		.gradle

feat: Initial Java runtime implementation #2318

feat: Initial Java runtime implementation #2318

Conversation

stuartwdouglas commented Aug 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alecthomas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alecthomas commented Aug 12, 2024

alecthomas commented Aug 12, 2024

stuartwdouglas commented Aug 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment