Introduce #[dynomite(flatten)] attribute

[Veetaha]

What did you implement:

This should be useful when we have fat enums support in dynomite (#131), for enum variants to be able to be used as partition/sort keys.

This works like #[serde(flatten)], the fields of the nested struct are flattened to the attributes of the surrounding struct where the flattened field is declared. The reverse mapping is done when parsing the struct from raw Attributes.

How did you verify your change:

Added a unit test, compile_fail test, and documentation (doc test)

What (if anything) would need to be called out in the CHANGELOG for the next release:

• Introduce #[dynomite(flatten)] field attribute
  UPD
• BREAKING CHANGE FromAttributes now requires the impl of fn from_mut_attrs(attrs: &mut Attributes) instead of from_attrs(attrs: Attributes), the latter now has the default impl based on from_mut_attrs impl

[softprops]

@Veetaha I'm happy to merge this as is. Let me know if there are more changes coming

[Veetaha]

Added traits to make flattening work more naturally, and support collecting additional attributes as told here: #132 (comment).
This should be ready to merge.
cc @anelson

[Veetaha]

I am still hesitating about naming:

from_mut_attrs(&mut Atttributes) / from_attrs_sink(&mut Attributes)
into_mut_attrs(self, &mut Attributes) / into_attrs_sink(self, &mut Attributes)

Wdyt?
Looks like mut_attrs fits better as per the API guidelines (mut_attrs reflects the &mut Attributes type) so I switched to it for now?

[Veetaha]

cc @softprops

[softprops]

Very cool. I'll take a closer look today

[Veetaha]

I recommend not releasing this one when it is merged to master.
I am working on support for fat enums and it will have yet one more breaking change...

[softprops]

I'm still groking the changes here and think I'll need another evening.

Overall this looks promising. Somethings might have been helpful to put into separate prs to keep the diff size down bit thats not a blocker for me.

As a goal id like to reuse existing traits where possible. If that means changing an argument type I don't mind bumping a version if the change is worth it.

Could you fill me in on whether that change was needed for the implementation an how or if it was a performance optimization. If it was the latter i'd appreciate a performance comparison to understand the impact and to help promote reason behind the argument type change for users

[softprops]

It might be slightly confusing to have two methods with similar names whose arguments are both mut. Can we just make the others methods arg a borrowed mut to keep the simpler name and surface area small.

[Veetaha]

These methods are not both mut.
from_attrs consumes the input and it is just the argument variable itself which is mutable, the implementors of this method are not required to make it mut too.
This is the same as the difference between Vec::drain(&mut self) and Vec::into_iter([mut] self), the mutability of self on into_iter doesn't matter.
Do you want to remove the consuming (by-value) method and instead just have fn from_attrs(&mut Attributes) ?

[softprops]

That would be great. I'm erring on merging sooner so that could be cleanup before the next release

[softprops]

Why introduce this when derive Attributes on a type?

[Veetaha]

This is an example of not using derive attributes and making a manual impl:

Below is an example of doing this manually for demonstration.

[softprops]

Can we separate these examples to distinguish the use case where you would find flatten most useful?

The current api aims to make a mapping between structs and dynamo item schema which is intuative enough that you can guess what the struct will look like when looking at the dynamo item and visa versa. I'd like to still promote that as the default path until there's a need or special case where you mind need flattening.

Let's make a second separate example describing where in when you might need this.

[Veetaha]

Flattening is required to support flat fat enums:

#[derive(Item)] 
struct MyItem {
    #[dynomite(partition_key)]
    id: String,
    #[dynomite(flatten)]
    my_enum: Foo,
}

#[derive(Attributes)]
#[dynomite(tag = "kind")]
enum Foo {
    Bar(Bar),
}

#[derive(Attributes)]
struct Bar {
    bar: u32
}

Without that flatten attribute the item would be serialized as:

{
    "id": "1234",
    "my_enum": {
         "kind": "Bar", // can't use neither `kind`, nor `bar` as a sort key or GSI partition key
         "bar": 42,
    }
}

which won't work if you want to make bar or kind fields a partition or sort key.
With flatten this becomes possible:

{
     "id": "1234"
     "kind": "Bar",
     "bar": 42,
}

I've started with flatten, so I can't give an example with a fat enum right now in this PR.
Otherwise flatten is useful to extract repetitive data, maybe even construct the items itself from disjoint projections,
or just have the id-less struct that contains only datums (this is useful when id is not yet known):

#[derive(Item)]
struct User {
    #[dynomite(partition_key)]
    id: Uuid,
    #[dynomite(flatten)]
    profile: UserProfile,
}

#[derive(Attributes)]
struct UserProfile {
    name: String,
    avatar: Url,
}

// Accepts data, no id field is required to be filled by the caller
async fn create_user(data: UserProfile) -> Result<User> {
     let user = User {
          id: Uuid::new_v4(),
          profile: data,
     };
     db.write_item({ user.clone()... }).await?;
     user
}

Though for the last point we may just add yet another proc macro to generate struct without the partition key field and update changeset structs with all fields as Option<T>.

In this example here I have shown extracting the data from id into a separate struct because I cannot show an example with a flat fat enum yet...

[Veetaha]

@softprops the switch from consuming the attributes from_attrs(HashMap<String, AttributeValue>) to taking by a mut reference: from_mut_attrs(&mut HashMap<String, AttributeValue>) is required for both easier implementation and performance.

The process of deserializing would otherwise need to clone and find the difference between the hashmaps for each flatten attribute:

use dynomite::{Attributes, FromAttributes, Attribute};

#[derive(Attributes)]
struct Foo {
    a: u32,
    #[derive(flatten)]
    bar: Bar,
    #[derive(flatten)]
    baz: Baz,
}

#[derive(Attributes)]
struct Bar {
    b: bool,
}

#[derive(Baz)]
struct Baz {
    c: u32,
}

// This code would be generated
impl FromAttributes for Foo {
    fn from_attrs(mut attrs: Attributes) -> Result<Self, dynomite::AttributeError> {
        let a: u32 = Attribute::from_attr(attrs.remove("a"))?;
        // clone here
        let bar: Bar = FromAttributes::from_attrs(attrs.clone())?;
        // find the difference, this needs the info about Bars fields here
        // this requires more codegen code to get them here and is wasteful by itself
        attrs.remove("b");
        let baz: Baz = FromAttributes::from_attrs(attrs)?;
        Foo {
            a,
            flat,
            baz,
        }
    }
}

// Instead this is generated:
impl FromAttributes for Foo {
    fn from_mut_attrs(attrs: &mut Attributes) -> Result<Self, dynomite::AttributeError> {
        let a: u32 = Attribute::from_attr(attrs.remove("a"))?;
        let bar: Bar = FromAttributes::from_attrs(attrs)?;
        let baz: Baz = FromAttributes::from_attrs(attrs)?;
        Foo {
            a,
            bar,
            baz.,
        }
    }
}

[Veetaha]

Nightly job started failing because of the updated error message, so I made unstable versions into optional jobs (a9022b3): rust-lang/rust#76406

[softprops]

I think I'm going to merge. It looks like at some point the compile tests started failing. Can we fix that up first? I'd feel more comfortable merging on green

[Veetaha]

I've disabled trybuild tests on beta and nightly, but they still run on stable