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

feat: use builtin.Empty instead of Unit typealias in Kotlin #904

Merged
merged 1 commit into from
Feb 8, 2024
+172 −88
Merged

feat: use builtin.Empty instead of Unit typealias in Kotlin #904

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 2 additions & 3 deletions examples/kotlin/ftl-module-api/src/main/kotlin/ftl/api/Api.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import com.google.gson.GsonBuilder
import com.google.gson.JsonDeserializer
import com.google.gson.JsonPrimitive
import com.google.gson.JsonSerializer
import ftl.builtin.Empty
import ftl.builtin.HttpRequest
import ftl.builtin.HttpResponse
import xyz.block.ftl.*
Expand All @@ -18,8 +19,6 @@ data class Todo(
val completed: Boolean = false,
)

typealias GetStatusRequest = Unit

data class GetStatusResponse(
val status: String,
)
Expand Down Expand Up @@ -59,7 +58,7 @@ class Api {

@Verb
@HttpIngress(Method.GET, "/api/status")
fun status(context: Context, req: HttpRequest<GetStatusRequest>): HttpResponse<GetStatusResponse> {
fun status(context: Context, req: HttpRequest<Empty>): HttpResponse<GetStatusResponse> {
return HttpResponse<GetStatusResponse>(status = 200, headers = mapOf(), body = GetStatusResponse("OK"))
}

Expand Down
4 changes: 2 additions & 2 deletions integration/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,12 @@ func TestHttpIngress(t *testing.T) {
httpCall(rd, http.MethodPut, "/users/123", jsonData(t, obj{"postId": "346"}), func(t testing.TB, resp *httpResponse) {
assert.Equal(t, 200, resp.status)
assert.Equal(t, []string{"Header from FTL"}, resp.headers["Put"])
assert.Equal(t, nil, resp.body)
assert.Equal(t, map[string]any{}, resp.body)
}),
httpCall(rd, http.MethodDelete, "/users/123", jsonData(t, obj{}), func(t testing.TB, resp *httpResponse) {
assert.Equal(t, 200, resp.status)
assert.Equal(t, []string{"Header from FTL"}, resp.headers["Delete"])
assert.Equal(t, nil, resp.body)
assert.Equal(t, map[string]any{}, resp.body)
}),
Comment on lines +99 to 105
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we should leave one of these Unit so we can ensure that nil works as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the /empty path still tests Unit!

httpCall(rd, http.MethodGet, "/empty", nil, func(t testing.TB, resp *httpResponse) {
	assert.Equal(t, 200, resp.status)
	assert.Equal(t, nil, resp.headers["Content-Type"])
	assert.Equal(t, nil, resp.bodyBytes)
}),


httpCall(rd, http.MethodGet, "/html", jsonData(t, obj{}), func(t testing.TB, resp *httpResponse) {
Expand Down
12 changes: 6 additions & 6 deletions integration/testdata/go/httpingress/echo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@ type PutResponse struct{}

//ftl:verb
//ftl:ingress http PUT /users/{userId}
func Put(ctx context.Context, req builtin.HttpRequest[PutRequest]) (builtin.HttpResponse[ftl.Unit], error) {
return builtin.HttpResponse[ftl.Unit]{
func Put(ctx context.Context, req builtin.HttpRequest[PutRequest]) (builtin.HttpResponse[builtin.Empty], error) {
return builtin.HttpResponse[builtin.Empty]{
Headers: map[string][]string{"Put": {"Header from FTL"}},
Body: ftl.Unit{},
Body: builtin.Empty{},
}, nil
}

Expand All @@ -81,11 +81,11 @@ type DeleteResponse struct{}

//ftl:verb
//ftl:ingress http DELETE /users/{userId}
func Delete(ctx context.Context, req builtin.HttpRequest[DeleteRequest]) (builtin.HttpResponse[ftl.Unit], error) {
return builtin.HttpResponse[ftl.Unit]{
func Delete(ctx context.Context, req builtin.HttpRequest[DeleteRequest]) (builtin.HttpResponse[builtin.Empty], error) {
return builtin.HttpResponse[builtin.Empty]{
Status: 200,
Headers: map[string][]string{"Delete": {"Header from FTL"}},
Body: ftl.Unit{},
Body: builtin.Empty{},
}, nil
}

Expand Down
6 changes: 3 additions & 3 deletions integration/testdata/kotlin/database/Echo.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
package ftl.echo

import ftl.builtin.Empty
import xyz.block.ftl.Context
import xyz.block.ftl.Verb
import xyz.block.ftl.Database

data class InsertRequest(val data: String)
typealias InsertResponse = Unit

val db = Database("testdb")

class Echo {

@Verb
fun insert(context: Context, req: InsertRequest): InsertResponse {
fun insert(context: Context, req: InsertRequest): Empty {
persistRequest(req)
return InsertResponse
return Empty()
}

fun persistRequest(req: InsertRequest) {
Expand Down
24 changes: 10 additions & 14 deletions integration/testdata/kotlin/httpingress/Echo.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package ftl.echo

import ftl.builtin.Empty
import ftl.builtin.HttpRequest
import ftl.builtin.HttpResponse
import kotlin.String
Expand All @@ -25,8 +26,8 @@ data class GetResponse(
)

data class PostRequest(
@Alias("user_id") val userID: Int,
val postID: Int,
@Alias("user_id") val userId: Int,
val postId: Int,
)

data class PostResponse(
Expand All @@ -38,14 +39,9 @@ data class PutRequest(
@Alias("postId") val postID: String,
)

typealias PutResponse = Unit

data class DeleteRequest(
@Alias("userId") val userID: String,
)
typealias DeleteResponse = Unit

typealias HtmlRequest = Unit

class Echo {
@Verb
Expand Down Expand Up @@ -74,27 +70,27 @@ class Echo {

@Verb
@HttpIngress(Method.PUT, "/users/{userId}")
fun put(context: Context, req: HttpRequest<PutRequest>): HttpResponse<PutResponse> {
return HttpResponse<PutResponse>(
fun put(context: Context, req: HttpRequest<PutRequest>): HttpResponse<Empty> {
return HttpResponse<Empty>(
status = 200,
headers = mapOf("Put" to arrayListOf("Header from FTL")),
body = PutResponse
body = Empty()
)
}

@Verb
@HttpIngress(Method.DELETE, "/users/{userId}")
fun delete(context: Context, req: HttpRequest<DeleteRequest>): HttpResponse<DeleteResponse> {
return HttpResponse<DeleteResponse>(
fun delete(context: Context, req: HttpRequest<DeleteRequest>): HttpResponse<Empty> {
return HttpResponse<Empty>(
status = 200,
headers = mapOf("Delete" to arrayListOf("Header from FTL")),
body = DeleteResponse
body = Empty()
)
}

@Verb
@HttpIngress(Method.GET, "/html")
fun html(context: Context, req: HttpRequest<HtmlRequest>): HttpResponse<String> {
fun html(context: Context, req: HttpRequest<Empty>): HttpResponse<String> {
return HttpResponse<String>(
status = 200,
headers = mapOf("Content-Type" to arrayListOf("text/html; charset=utf-8")),
Expand Down
6 changes: 3 additions & 3 deletions kotlin-runtime/ftl-generator/src/main/kotlin/xyz/block/ftl/generator/ModuleGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ class ModuleGenerator() {

val types = module.decls.mapNotNull { it.data_ }
types.forEach {
if (it.fields.isEmpty()) {
file.addTypeAlias(
TypeAliasSpec.builder(it.name, Unit::class)
if (namespace == "ftl.builtin" && it.name == "Empty") {
file.addType(
TypeSpec.classBuilder(it.name)
.addKdoc(it.comments.joinToString("\n"))
.build()
)
Expand Down
133 changes: 107 additions & 26 deletions kotlin-runtime/ftl-generator/src/test/kotlin/xyz/block/ftl/generator/ModuleGeneratorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ public class TestModule()
)
)
),
Decl(data_ = Data(comments = listOf("Request comments"), name = "TestRequest")),
Decl(
data_ = Data(
comments = listOf("Response comments"), name = "TestResponse",
Expand Down Expand Up @@ -110,7 +109,6 @@ import kotlin.ByteArray
import kotlin.Float
import kotlin.Long
import kotlin.String
import kotlin.Unit
import kotlin.collections.ArrayList
import kotlin.collections.Map
import xyz.block.ftl.Alias
Expand All @@ -120,11 +118,6 @@ public data class ParamTestData<T>(
public val t: T,
)

/**
* Request comments
*/
public typealias TestRequest = Unit

/**
* Response comments
*/
Expand Down Expand Up @@ -158,22 +151,20 @@ public class TestModule()
@Test
fun `should generate all Verbs`() {
val decls = listOf(
Decl(data_ = Data(comments = listOf("Request comments"), name = "TestRequest")),
Decl(data_ = Data(comments = listOf("Response comments"), name = "TestResponse")),
Decl(
verb = Verb(
name = "TestVerb",
comments = listOf("TestVerb comments"),
request = Type(dataRef = DataRef(name = "TestRequest")),
response = Type(dataRef = DataRef(name = "TestResponse"))
request = Type(dataRef = DataRef(name = "Empty", module = "builtin")),
response = Type(dataRef = DataRef(name = "Empty", module = "builtin"))
)
),
Decl(
verb = Verb(
name = "TestIngressVerb",
comments = listOf("TestIngressVerb comments"),
request = Type(dataRef = DataRef(name = "TestRequest")),
response = Type(dataRef = DataRef(name = "TestResponse")),
request = Type(dataRef = DataRef(name = "Empty", module = "builtin")),
response = Type(dataRef = DataRef(name = "Empty", module = "builtin")),
metadata = listOf(
Metadata(
ingress = MetadataIngress(
Expand All @@ -192,30 +183,20 @@ public class TestModule()
// Module comments
package ftl.test

import kotlin.Unit
import ftl.builtin.Empty
import xyz.block.ftl.Context
import xyz.block.ftl.HttpIngress
import xyz.block.ftl.Ignore
import xyz.block.ftl.Method.GET
import xyz.block.ftl.Verb

/**
* Request comments
*/
public typealias TestRequest = Unit

/**
* Response comments
*/
public typealias TestResponse = Unit

@Ignore
public class TestModule() {
/**
* TestVerb comments
*/
@Verb
public fun TestVerb(context: Context, req: TestRequest): TestResponse = throw
public fun TestVerb(context: Context, req: Empty): Empty = throw
NotImplementedError("Verb stubs should not be called directly, instead use context.call(TestModule::TestVerb, ...)")

/**
Expand All @@ -226,9 +207,109 @@ public class TestModule() {
GET,
"/test",
)
public fun TestIngressVerb(context: Context, req: TestRequest): TestResponse = throw
public fun TestIngressVerb(context: Context, req: Empty): Empty = throw
NotImplementedError("Verb stubs should not be called directly, instead use context.call(TestModule::TestIngressVerb, ...)")
}
"""
assertEquals(expected, file.toString())
}

@Test
fun `builtins are handled correctly`() {
val decls = listOf(
Decl(
data_ = Data(
comments = listOf("HTTP request structure used for HTTP ingress verbs."),
name = "HttpRequest",
fields = listOf(
Field(name = "method", type = Type(string = String())),
Field(name = "path", type = Type(string = String())),
Field(
name = "pathParameters",
type = Type(map = Map(key = Type(string = String()), value_ = Type(string = String())))
),
Field(
name = "query",
type = Type(
map = Map(
key = Type(string = String()),
value_ = Type(array = Array(element = Type(string = String())))
)
)
),
Field(
name = "headers",
type = Type(
map = Map(
key = Type(string = String()),
value_ = Type(array = Array(element = Type(string = String())))
)
)
),
Field(name = "body", type = Type(dataRef = DataRef(name = "Body")))
),
typeParameters = listOf(TypeParameter(name = "Body"))
)
),
Decl(
data_ = Data(
comments = listOf("HTTP response structure used for HTTP ingress verbs."),
name = "HttpResponse",
fields = listOf(
Field(name = "status", type = Type(int = Int())),
Field(
name = "headers",
type = Type(
map = Map(
key = Type(string = String()),
value_ = Type(array = Array(element = Type(string = String())))
)
)
),
Field(name = "body", type = Type(dataRef = DataRef(name = "Body")))
),
typeParameters = listOf(TypeParameter(name = "Body"))
)
),
Decl(data_ = Data(name = "Empty")),
)
val module = Module(name = "builtin", comments = listOf("Built-in types for FTL"), decls = decls)
val file = generator.generateModule(module)
val expected = """// Code generated by FTL-Generator, do not edit.
// Built-in types for FTL
package ftl.builtin

import kotlin.Long
import kotlin.String
import kotlin.collections.ArrayList
import kotlin.collections.Map
import xyz.block.ftl.Ignore

/**
* HTTP request structure used for HTTP ingress verbs.
*/
public data class HttpRequest<Body>(
public val method: String,
public val path: String,
public val pathParameters: Map<String, String>,
public val query: Map<String, ArrayList<String>>,
public val headers: Map<String, ArrayList<String>>,
public val body: Body,
)

/**
* HTTP response structure used for HTTP ingress verbs.
*/
public data class HttpResponse<Body>(
public val status: Long,
public val headers: Map<String, ArrayList<String>>,
public val body: Body,
)

public class Empty

@Ignore
public class BuiltinModule()
"""
assertEquals(expected, file.toString())
}
Expand Down
Loading