Support splitting a single item into multiple files

[missingdays]

Motivation

To allow generating code for a single entity in multiple files. See a related issue in volo - https://github.com/cloudwego/volo/issues/454

Solution

Introduce a new split mode that can be enabled using the builder

[CLAassistant]

All committers have signed the CLA.

[missingdays]

I've only added a single test so far, considering this is a proposal and not a final solution. When we agree on how the final solution should look like, I will implement the remaining tests.

[Millione]

LGTM. Maybe a test for the workspace mode can be added?

[missingdays]

@Millione Hi. I tried adding a test for a workspace, but couldn't find any existing tests for it. And my test fails with a panic at pilota-build/src/codegen/workspace.rs:91 because Cargo.toml doesn't exist. I'm not sure how this file should be created when generating the workspace for the first time. Here's how I compile with the workspace mode

      crate::Builder::thrift()
          .ignore_unused(false)
          .compile_with_config(
              vec![IdlService::from_path(source.to_owned())],
              crate::Output::Workspace(target.into()),
          )

Could you please point me to an existing test for a workspace mode, or help me figure out why Cargo.toml is expected to already exist in group_defs?

[Millione]

Could you please point me to an existing test for a workspace mode

Unfortunately, there is no existing test for this, but I'll do my best to help you figure it out.

Why Cargo.toml is expected to already exist in group_defs?

The difference between normal mode and workspace mode is that in workspace mode the code generation doesn't take place in the build phase, but in the run phase. It generates multiple crates and adds all these crates as workspace members to the Cargo.toml file.

I'm not sure how this file should be created when generating the workspace for the first time

In my opinion, we should put the workspace mode test in a separate tests folder, so that it is a package with a binary target and fits into the real usage.

[missingdays]

I added tests for the workspace mode, as well as splitting the files in the workspace mode. I added all the tests from the thrift tests, I'm not sure if so many are needed or if it's too much

[Millione]

I added tests for the workspace mode, as well as splitting the files in the workspace mode.

Fantastic jobs!

I added all the tests from the thrift tests, I'm not sure if so many are needed or if it's too much

Maybe that's a bit too much? I think one thrift test is enough, but the tested thrift files should be well-constructed for the workspace, e.g. A.thrift and B.thrift both use C.thrift struct, and see if the struct is generated in common crates and used correctly by A and B.

[PureWhiteWu]

A.thrift and B.thrift both use C.thrift struct

Maybe more complex? for example, more layers of nesting mods/namespaces?

[missingdays]

Could you maybe write down the .thrift file you have in mind? My knowledge of thrift is quite limited

[PureWhiteWu]

Sorry for the late reply, I've constructed five idl files.

article.thrift:

include "image.thrift"
include "author.thrift"
include "common.thrift"

namespace rs article

enum Status {
    NORMAL = 0,
    DELETED = 1,
}

struct Article {
    1: required i64 id,
    2: required string title,
    3: required string content,
    4: required author.Author author,
    5: required Status status,
    6: required list<image.Image> images,
    7: required common.CommonData common_data,
}

struct GetArticleRequest {
    1: required i64 id,
}

struct GetArticleResponse {
    1: required Article article,
}

service ArticleService {
    GetArticleResponse GetArticle(1: GetArticleRequest req),
}

author.thrift:

include "image.thrift"
include "common.thrift"

namespace rs author

struct Author {
    1: required i64 id,
    2: required string username,
    3: required string email,
    4: required image.Image avatar,
    5: required common.CommonData common_data,
}

struct GetAuthorRequest {
    1: required i64 id,
}

struct GetAuthorResponse {
    1: required Author author,
}

service AuthorService {
    GetAuthorResponse GetAuthor(1: GetAuthorRequest req),
}

cdn.thrift:

include "common.thrift"

namespace rs article.image.cdn

struct CDN {
    1: required i64 id,
    2: required string url,
    3: required common.CommonData common_data,
}

common.thrift:

namespace rs common

struct CommonData {
    1: required i64 id,
    2: required string name,
    3: required string description,
}

image.thrift:

include "common.thrift"
include "cdn.thrift"

namespace rs article.image

struct Image {
    1: required i64 id,
    2: required string url,
    3: required cdn.CDN cdn,
    4: required common.CommonData common_data,
}

struct GetImageRequest {
    1: required i64 id,
}

struct GetImageResponse {
    1: required Image image,
}

service ImageService {
    GetImageResponse GetImage(1: GetImageRequest req),
}

And there are three services that can be used in workspace mode, in article.thrift, author.thrift, image.thrift.

[missingdays]

@PureWhiteWu Thanks, I've replaced the workspace tests with the one you provided, it seems to work fine.

[PureWhiteWu]

Thanks again! LGTM now, seems there's still a little format issue, could you please use cargo fmt to fix it?

[PureWhiteWu]

LGTM, @Millione PTAL

[Millione]

Thanks very much for this great job!

[missingdays]

Great, thanks for the reviews! Now I'd like to use this functionality in volo to fix https://github.com/cloudwego/volo/issues/454 Could you please publish the new (unstable?) version of pilota so that I can use it in volo?

[PureWhiteWu]

@Millione Maybe we can publish a new version and make split the default behaviour?

[Millione]

@Millione Maybe we can publish a new version and make split the default behaviour?

Get it.

[Millione]

Could you please publish the new (unstable?) version of pilota so that I can use it in volo?

@missingdays Maybe you can use the git dependencies temporarily? Once it is ready in volo, we can release the version in pilota.

[missingdays]

@Millione @PureWhiteWu I have submite the pull request to volo with this functionality https://github.com/cloudwego/volo/pull/510