chore(server): remove useless ServerConfigBuilder

[koushiro]

The ServerConfigBuilder is useless, so I remove it, but this will cause a breaking change, because ServerConfig exposes a builder method.

[koushiro]

Hey again, thanks for your PR but I had proper look and it's not possible to configure the ServerConfig except the default one, https://docs.rs/jsonrpsee-server/latest/jsonrpsee_server/struct.ServerConfig.html

Thus, I prefer to keep it as it is right now alternatively move all the setters to the ServerConfig but I think it's quite clean to have all settings on the ServerConfigBuilder.

Thus, the ServerConfigBuilder is not useless

Builder does all things that ServerConfigBuilder can do, and ServerConfigBuilder doesn't even have a build method to generate ServerConfig. I really don't know what ServerConfigBuilder is for.

[niklasad1]

Yes, you are right the ServerConfigBuilder::build was/is missing I noticed it myself now and thus I opened https://github.com/paritytech/jsonrpsee/pull/1484.

Thank you, nice catch

[koushiro]

For downstream developers, the jsonrpc server can only be constructed through the Builder type. Moreover, the existing Builder implementation doesn't have a method to accept a constructed ServerConfig (built by ServerConfigBuilder). So the changes in #1484 are useless for me, as the ServerConfigBuilder cannot be used by the Builder.

If you want to keep ServerConfigBuilder, you need Builder type to have a method to accept a constructed ServerConfig, like below:

        pub fn with_config(config: ServerConfig) -> Self {
        Self {
            server_cfg: config,
            rpc_middleware: RpcServiceBuilder::new(),
            http_middleware: tower::ServiceBuilder::new(),
        }
    }

and better to remove the config setter methods from Builder type to let downstream developers use ServerConfigBuilder instead of Builder's config setter methods, otherwise ServerConfigBuilder will still not be used by downstream developers.

[niklasad1]

So, the ServerBuilderConfig and Builder are intended for different use-cases.

If one wants to just build a server via the jsonrsee_server::Builder then the ServerConfig/ServerConfigBuilder it's not useful for sure as you say but we expose a low-level api where this jsonrpsee_server::ServerConfigBuilder is useful and example here

Thus, most users will not use the ServerConfigBuilder/ServerConfig but still not useless.

[koushiro]

My point is that if we decide to keep ServerConfigBuilder, why not just delete all the config setter methods of Builder and use the following way, which can avoid a lot of repeated setter implementations (and if it can also avoid adding new setter methods to ServerConfigBuilder later, but forget to add corresponding methods to Builder)

async fn demo() -> anyhow::Result<()> {
    let builder = Builder::with_config(
        ServerConfig::builder()
            .max_request_body_size(TEN_MB_SIZE_BYTES)
                         // ...
            .build()
    )
    .set_rpc_middleware(...)
    .set_http_middleware(...)
    .build(...)
    .await?;
    // ...
}

[niklasad1]

That would be neat and I'm not against removing such repeating setters but I have no good answer where to draw the line for such thing i.e, what to have in the ServerConfig vs `ServerBuilder, or in some other type.

If you could write up an issue with a suggestion that would be great