Module separation

[lilgallon]

What is the purpose of having separate ocpi-2-2-1 and ocpi-transport maven dependencies?

ocpi-2-2-1 should include ocpi-transport in its jar. That way, users of the lib will just need to import ocpi-2-2-1 in their project.

Later, we could also provide transport implementations, and it would make sense in that case to create separate jars, because they will be optional (example: ocpi-transport-http4k-impl)

[atschabu]

I guess it made more sense while there was a ocpi 2.1 implementation. It might be relevant again when there is a 3.x implementation. But the relation is not as clear cut, I have far too many "hacks" to get the transport layer work nicely with ocpi module

[lilgallon]

Yes it was made because the transport layer is common for each OCPI version. The thing is that it should not be a separate maven dependency because it's not optional, but mandatory (main ocpi module depends on the transport module)

I am not super satisfied with the way the transport part of ocpi module is designed either. It may change in the future, but it's not the priority at the moment

[atschabu]

Assuming we'll be adding more OCPI versions in the future, it would make sense to have generic parts separated out. But I think the line is drawn too close to HTTP instead of OCPI. So we can bring certain protocol level interfaces and functionality (InterfaceRole, authHeaders) into the transport layer. Should a future version break some of these basics, then we could version the transport too .. eg transport2 transport4, based on the major version of the breaking standard.

We can make these adjustments in tiny steps, one interface at a time, making sure we don't break too much along the way

a TransportClientBuilder could then look like this, removing at least one non-overridable extension method.

interface ModuleUrlProvider<M> {
    // could be implemented by PartnerRepo, or better, a service depending on partnerrepo
    suspend fun urlFor(partnerId: String, moduleId: M, role: InterfaceRole): String
}

abstract class AbstractTransportClientBuilder<M>(urlProvider: ModuleUrlProvider<M>) : TransportClientBuilder<M> {
    override suspend fun buildFor(partnerId: String, moduleId: M, role: InterfaceRole): TransportClient {
        return build(urlProvider.urlFor(partnerId, moduleId, role)
    }
}

interface TransportClientBuilder<M> {
    suspend fun buildFor(partnerId: String, moduleId: M, role: InterfaceRole): TransportClient
    fun build(baseUrl: String): TransportClient
}

most of the functionality inside HttpUtils should be moveable to transport, especially client, header and context related functions

[lilgallon]

Now that we have enough feedback using the lib, this would ease development, especially, as you say, for http utils methods that are currently not overridable 👍