GWT-M3O-TEST / m3o-kotlin

Apache License 2.0
0 stars 0 forks source link

change the signature of methods to accept class object #1

Open lambdaR opened 2 years ago

lambdaR commented 2 years ago

@cyb3rko hi ... can we do this in kotlin? I have changed the signature of call method to accept HelloworldCallRequest instead of String


package com.m3o.m3okotlin.services

import com.m3o.m3okotlin.M3O.getUrl
import com.m3o.m3okotlin.M3O.ktorHttpClient
import com.m3o.m3okotlin.WebSocket

import io.ktor.client.request.*
import kotlinx.serialization.decodeFromString
import kotlinx.serialization.encodeToString
import kotlinx.serialization.json.Json
import kotlinx.serialization.Serializable

private const val SERVICE = "helloworld"

object HelloworldService {
    suspend fun call(req: HelloworldCallRequest): HelloworldCallResponse {
        return ktorHttpClient.post(getUrl(SERVICE, "Call")) {
          body = req
        }
    }
    fun stream(req: HelloworldStreamRequest, action: (Exception?, HelloworldStreamResponse?) -> Unit) {
        val url = getUrl(SERVICE, "Stream", true)
        WebSocket(url, Json.encodeToString(req)) { e, response ->
            action(e, if (response != null) Json.decodeFromString(response) else null)
        }.connect()
    }
}
@Serializable
internal data class HelloworldCallRequest(val name: String)
@Serializable
data class HelloworldCallResponse(val message: String)
@Serializable
internal data class HelloworldStreamRequest(val messages: Long, val name: String)
@Serializable
data class HelloworldStreamResponse(val message: String)

and then use it like this

package examples.helloworld.call

import com.m3o.m3okotlin.M3O
import com.m3o.m3okotlin.services.HelloWorldService

suspend fun main() {
    M3O.initialize("M3O_API_TOKEN")

    req = HelloWorldServiceRequest(name = "Jone")

    try {
        val response = HelloWorldService.call(req)
        println(response)
    } catch (e: Exception) {
        println(e)
    }
}
cyb3rko commented 2 years ago

That's an idea, it should work. You are only missing the imports and the val in the main method when initializing the req.

cyb3rko commented 2 years ago

And how do you implement e.g. the stream request to follow the code scheme we use here? Because I have some improvement, where can I at some point contribute it?

lambdaR commented 2 years ago

Because I have some improvement, where can I at some point contribute it?

You can contribute here (this org is basically a lab) until you PR kotlin project to m3o/m3o and m3o/m3o-kotlin.

And how do you implement e.g. the stream request to follow the code scheme we use here?

package examples.helloworld.stream

import com.cyb3rko.m3okotlin.M3O
import com.cyb3rko.m3okotlin.services.HelloWorldService

fun main() {
    M3O.initialize("NjJhODlhMmMtOGQ3Ni00YzljLWJhMzAtNGFkYzBhZTc5Yzcz")

    val req = HelloworldStreamRequest(messages = 2, name = "John")

    try {
        val socket = HelloWorldService.stream(req) { socketError, response ->
            if (socketError == null) {
                println(response)
            } else {
                println(socketError)
            }
        }
    } catch (e: Exception) {
        println(e)
    }
}
lambdaR commented 2 years ago

@cyb3rko I believe Asim wants all clients to look like below when you use it.

package examples.helloworld.call

import com.m3o.m3okotlin.M3O
import com.m3o.m3okotlin.services.HelloWorldService

suspend fun main() {
    val client = M3O.new("M3O_API_TOKEN")

    try {
        val response = client.HelloWorldService.call(
          req = HelloWorldServiceRequest(name = "Jone")
        )
        println(response)
    } catch (e: Exception) {
        println(e)
    }
}

see the equivalent go example https://github.com/m3o/m3o-go/blob/main/examples/helloworld/call/callHelloworld/main.go

cyb3rko commented 2 years ago

I think the solution I use in my handcrafted client is more convenient, but if Asim says he wants it like that, I can build new examples for that.

lambdaR commented 2 years ago

@cyb3rko Hi ... back to this .... i have applied the changes that we have discussed above ... can you make some random tests of these newly generated clients, lets see if they work properly.

cyb3rko commented 2 years ago

Hi, sorry for the delay. I was on holidays after finishing school.

I will take a look at it today.

lambdaR commented 2 years ago

Congrats... and don't worry

cyb3rko commented 2 years ago

Okay. Some other things we have to change:

  1. Every data class has to be public now (so the Request data classes are not allowed anymore to be "internal", just remove the keyword).

  2. The first line of every file currently is "package com.m3o.m3okotlin.services". But it has to end with the according name of the service. E.g. "package com.m3o.m3okotlin.services.helloworld".

  3. Furthermore we just need to fix a broken import line, I can open a pull request.

lambdaR commented 2 years ago

Cool... I will fix the generator... Then we can work on generating examples for each endpoint based on the examples.json

lambdaR commented 2 years ago

One thing that we can do to improve the visibility (it is quite verbose right now) is to create an index file where we list all repetitive imports in one file and then use that file instead, so for example at the top code there are 7 repeated import statements that we could replace with only one import.

cyb3rko commented 2 years ago

If I understand right you want to remove the following lines of the example above:

import com.m3o.m3okotlin.M3O.getUrl
import com.m3o.m3okotlin.M3O.ktorHttpClient
import com.m3o.m3okotlin.WebSocket

import io.ktor.client.request.*
import kotlinx.serialization.decodeFromString
import kotlinx.serialization.encodeToString
import kotlinx.serialization.json.Json
import kotlinx.serialization.Serializable

Unfortunately that's not a thing you can do in Kotlin. And honestly I don't see a reason why we should do changes about the imports. Additionally they are just necessary for the code to work xD We could just lower the number of imports with wildcarding.

You can even see this in code of Google Apps like here:

https://github.com/material-components/material-components-android/blob/master/catalog/java/io/material/catalog/main/MainActivity.java#L21-L38

Even though you could use more wildcard imports here for imports but they don't even use them here. What we can improve though is to use better formatting:

lambdaR commented 2 years ago

I see... One more thing that we need to know for sure is the process of publishing a library to the kotlin libraries/packages manager... What are the steps we need to do? Is there any community github action that do all/parts of the process?

cyb3rko commented 2 years ago

For sure. I will look it up and tell you when I've found something.

cyb3rko commented 2 years ago

But first let's make sure one single line of code is executable and does not end in 5 errors xD And mabye I should then try to test more then only HelloWorld.

lambdaR commented 2 years ago

We will not activate the publishing job in the github workflow until everything is correct. For some languages the process of publishing a library is quite easy and for some quite annoying (like Dart for example), so I think we need to prepare/investigate that matter as soon as possible.

lambdaR commented 2 years ago

I applied fixes to kotlin generator ... please check of there are any errors ... as for testing though, i think evchargers, weather and mq are suffice ... no need to test all services

cyb3rko commented 2 years ago

Okay so...

But removing them would also mean that the function with no response don't have a return type. So you have to somehow be able to remove the response types and request bodies for some of the functions if that's even possible.

lambdaR commented 2 years ago

you lost me here ... can you give an example

cyb3rko commented 2 years ago

Yh of course, the explanation wasn't the best lol :D

cyb3rko commented 2 years ago

https://github.com/GWT-M3O-TEST/m3o-kotlin/blob/30701aa38c1eb42879a203788e3961151d37ab55/src/main/kotlin/com/m3o/m3okotlin/services/analytics/analytics.kt#L43-L54

These are some of the Analytics data classes. In line 44 there's an empty request class and in line 54 there's an empty response class. Those kind of empty data classes are the problem.

cyb3rko commented 2 years ago

And let's compare my handcrafted method and the auto-generated one:

suspend fun list(req: AnalyticsListRequest): AnalyticsListResponse {
    return ktorHttpClient.post(getUrl(SERVICE, "List")) {
        body = req
    }
}

You ask for the empty class an an parameter where I drop it as it's obviously not needed because it's empty.

suspend fun list(): AnalyticsListResponse {
    return M3O.ktorHttpClient.post(M3O.getUrl(SERVICE, "List"))
}
lambdaR commented 2 years ago

It is doable... Will apply a fix soon

cyb3rko commented 2 years ago

Awesome, thanks for your work

lambdaR commented 2 years ago

can you check

cyb3rko commented 2 years ago

Alright, the Analytics endpoints work now. We can remove the empty request classes now as you correctly removed the request bodies.

Maybe the next step would be removing empty responses. Example:

suspend fun track(req: AnalyticsTrackRequest): AnalyticsTrackResponse {
    return ktorHttpClient.post(getUrl(SERVICE, "Track")) {
        body = req
    }
}

Here AnalyticsTrackResponse is returned, but the class is empty:

@Serializable
class AnalyticsTrackResponse()

It would then look like this:

suspend fun track(req: AnalyticsTrackRequest) {
    return ktorHttpClient.post(getUrl(SERVICE, "Track")) {
        body = req
    }
}

I just looked through the generator to see how it works and to be able to work on this as well. Most things I've read makes sense for me, maybe at some point I can work on that on my own. Then I don't have to let you do the work :)

lambdaR commented 2 years ago

I just looked through the generator to see how it works and to be able to work on this as well. Most things I've read makes sense for me, maybe at some point I can work on that on my own. Then I don't have to let you do the work :)

cool ... actually you should have a full understanding of how the generator works so you can further enhance your m3o-kotlin clients ... maybe m3o-kotlin 2.0 :)

I have applied new fixes ... see if there are any errors

cyb3rko commented 2 years ago

Regarding the request and response classes itself it looks good now. But it seems we still have the problem that the data types are not correct. Example: DBMap does not exist

Can you show me again where to fill out the language specific data types?

lambdaR commented 2 years ago

https://github.com/GWT-M3O-TEST/m3o/blob/ae43f821b2bc209572298a6440ca8bef50702655/cmd/m3o-client-gen/kotlin_generator.go#L65

some how the generator prefix Map with Db

i will check ... you too

lambdaR commented 2 years ago

I have pushed a fix

see if there are any errors