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.

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

Add support for remote on fields #323

Closed
banool opened this issue Jul 19, 2022 · 12 comments
Closed

Add support for remote on fields #323

banool opened this issue Jul 19, 2022 · 12 comments
Labels
enhancement New feature or request Stale

Comments

@banool
Copy link
Contributor

banool commented Jul 19, 2022

Hey, I just took a look at modifying poem-openapi-derive/src/object.rs but I couldn't quite figure out what to do.

What I'd love to be able to do is something like this:

#[derive(Object)]
struct MyStruct {
    a: u32,
    #[oai(remote = "u32")]
    b: u64,
}

This is just an example, in reality the type I use in remote would be a local version of a remote type, so I can impl the Poem stuff on it.

I want this because today if I wanted to do this, I would have to add remote to the whole struct instead and define a whole new wrapper struct, even though I only want this for one field.

I suppose in some ways this is similar to #322. My ultimate goal is I have a newtype in a remote crate that I don't want to have to wrap, since I use it in many places.

@banool banool added the enhancement New feature or request label Jul 19, 2022
@sunli829
Copy link
Collaborator

Do you want the code below to work?

InternalMyObj has a and b fields, but MyObj has only a field.

    mod remote {
        #[derive(Debug, Eq, PartialEq)]
        pub struct InternalMyObj {
            pub a: i32,
            pub b: String,
        }
    }

    #[derive(Debug, Object, Eq, PartialEq)]
    #[oai(remote = "remote::InternalMyObj")]
    struct MyObj {
        a: i32,
    }

@banool
Copy link
Contributor Author

banool commented Jul 19, 2022

Hmm not quite, what I really want is something like this: #322. This code explains what I want better:

    mod remote {
        #[derive(Debug, Eq, PartialEq)]
        pub struct Address(pub String);
    }

    #[derive(Debug, Eq, PartialEq, NewType)]
    struct AddressHelper(pub String);

    impl From<remote::Address> for AddressHelper {
        fn from(address: remote::Address) -> Self {
            Self(address.0)
        }
    }

    impl Into<remote::Address> for AddressHelper {
        fn into(self) -> remote::Address {
            remote::Address(self.0)
        }
    }


    #[derive(Debug, Object, Eq, PartialEq)]
    struct MyObj {
        #[oai(serializer_type = AddressHelper)]
        address: Address,
        value: u32,
    }

For this serializer_type should be a struct that implements Into<T>, From<T> like how in the above example AddressHelper does. It should also impl the traits that allow it to be used in a Poem object, like ParseFromJSON, ParseFromParameter, ToJSON, etc. AKA all the traits that you get when you add a Poem derive like NewType.

The purpose of this is I can now use Address in my poem type even though I am not able to derive a Poem trait on it.

This is pretty much a clearer description of #322, so let me know if you want me to close this issue and we migrate to that one.

@sunli829
Copy link
Collaborator

sunli829 commented Jul 19, 2022

There is a problem with ownership here, when calling Address::ToJson, need to convert Address to AddressHelper, which will transfer ownership, unless Address implements Clone.

@banool
Copy link
Contributor Author

banool commented Jul 19, 2022

I think that's an acceptable bound to have if you want to use this feature. I'll look at the traits some more to make sure I understand what you're talking about though.

@sunli829
Copy link
Collaborator

Maybe you can use AddressHelper directly?

    #[derive(Debug, Object, Eq, PartialEq)]
    struct MyObj {
        address: AddressHelper,
        value: u32,
    }

@banool
Copy link
Contributor Author

banool commented Jul 19, 2022

The problem is I have like 100 places where I use the original Address, so that's a lot of refactoring, plus I interact with other crates that I don't control that expect that type. So for now I'm using a wrapper and a lot of .into() (which has the same problem where I have to replace those 100 places with different usage, but at least I don't have to change crates I don't own).

@sunli829
Copy link
Collaborator

sunli829 commented Jul 19, 2022

Although there are many places to add .into(), at least the compiler will tell you where to add it, is this a one-time job? 😂

@banool
Copy link
Contributor Author

banool commented Jul 19, 2022

Yeah one time job lol, probably not worth your effort to add right now hahah. A nice feature for the future perhaps.

@banool
Copy link
Contributor Author

banool commented Jul 19, 2022

Okay more than a one time job, wrapping everything is becoming a bit painful. It's okay for now though.

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added Stale and removed Stale labels Aug 19, 2022
@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Sep 19, 2022
@github-actions
Copy link

This issue was closed because it has been stalled for 5 days with no activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Stale
Projects
None yet
Development

No branches or pull requests

2 participants