Changing the properties type would be a breaking change :( We might need a breaking change for upping the mrsv so maybe it's a good time for it.
If we use the feature preserve_order are we causing any issues to users? Seems this may force preserve_order for serde_json on a user workspace?

Arguably the other BTreeMap extra_properties etc could be changed too?

Yeah, I guess so

Also with a bit more work, it could be an option of the Apiv2Schema derive. Let me know!

You mean control wether we preserve order or not?

Hey,

Changing the properties type would be a breaking change :( We might need a breaking change for upping the mrsv so maybe it's a good time for it.

Indeed! My bad, I didn't think about it because it's actually quite hard / I didn't directly see the use case, to directly use the derived Schema struct (more just below), but you're technically right which is the best kind of right as we all know :).

Re. why it's hard: see e.g. my test. Since particular schemas are not named (they could be if e.g. Apiv2Schema defined an associated type Type Schema = T which would force every implementor of Apiv2Schema to define a return type for their Schema), I can't just deserialize the response to a typed struct (therefore all the expect). Because my Dog schema is just a DefaultSchemaRaw, but the deserializing of DefaultSchemaRaw yields an empty schema, no matter what is in the JSON.

In the end: I'm not in a particular hurry, for my use case I'm OK waiting for a 0.10 as long as it's…not next decade 😂

If we use the feature preserve_order are we causing any issues to users? Seems this may force preserve_order for serde_json on a user workspace?

Again you might be right. It will definitely "preserve order" for everything serialized by paperclip at the very least. I guess it's a noop because every other thing in paperclip seems to either be a scalar type or a BTreeMap (where it will just preserve the forced Ord order of keys, just like now), but will it contaminate a user project? I must admit IDK.

Arguably the other BTreeMap extra_properties etc could be changed too?

Yeah, I guess so

I didn't touch them because I dont use extra_properties or others, but OK makes sense to "keep declaration order" consistently if we go that route.

Also with a bit more work, it could be an option of the Apiv2Schema derive. Let me know!

You mean control wether we preserve order or not?

Yes I meant something like

/// Order fields like I declare them
#[derive(Apiv2Schema(order = "struct")]
struct Dog {
    ...
}

/// Or just #[derive(Apiv2Schema)] because retrocompat
#[derive(Apiv2Schema(order = "alphabetical")]
struct Cat {
    ...
}

but will it contaminate a user project? I must admit IDK.

I'm not 100% sure either, but rust features are additive so if someone is using resolver 1 AFAIK then serde_json deps get merged with all combined features added in a single build. But again I'm not certain either.

Yes I meant something like
.....

That looks awesome! Would allow for preserving behaviour in case someone prefers alphabetical for some reason.

That looks awesome! Would allow for preserving behaviour in case someone prefers alphabetical for some reason.

Unfortunately that can't work, or I didn't find a way. At the end of the day, when you're serializing in json with serde, you're going through serde's abstraction of calling Serialize methods which take a Serializer, and serde_json's Serializer gives you its own Map type to serialize a "serde map", and that Map implem is a BTreeMap if you don't use serde's preserve_order.

I see two approaches to this. Both have as first step: add a preserve_order feature to paperclip, and like serde_json, either stick with BTreeMap or use IndexMap for properties. This has the advantage of not being a breaking change, unlike my first version.

Then either:

A. activate serde_json's preserve_order feature if paperclip's one is, and document it;
B. document that users should activate serde_json's preserve_order feature too on their own to get the actual effect

I personaly have a strong preference for A for the same reason as #540, if you don't already have serde_json in your dependencies it will be weird, and I think anyway, as much as I want to have my json fields in order for human-readability, if it breaks your automatic workflow… what bloody json libraries rely on field order??

That looks awesome! Would allow for preserving behaviour in case someone prefers alphabetical for some reason.

Unfortunately that can't work, or I didn't find a way. At the end of the day, when you're serializing in json with serde, you're going through serde's abstraction of calling Serialize methods which take a Serializer, and serde_json's Serializer gives you its own Map type to serialize a "serde map", and that Map implem is a BTreeMap if you don't use serde's preserve_order.

Alright seems like it's a no go then, thanks for trying!

I see two approaches to this. Both have as first step: add a preserve_order feature to paperclip, and like serde_json, either stick with BTreeMap or use IndexMap for properties. This has the advantage of not being a breaking change, unlike my first version.

Then either:

A. activate serde_json's preserve_order feature if paperclip's one is, and document it; B. document that users should activate serde_json's preserve_order feature too on their own to get the actual effect

I personaly have a strong preference for A for the same reason as #540, if you don't already have serde_json in your dependencies it will be weird, and I think anyway, as much as I want to have my json fields in order for human-readability, if it breaks your automatic workflow… what bloody json libraries rely on field order??

Yeah, I think A is fine. It should be obvious from the feature on the toml that it activates the serde_json's equivalent feature.

I added a PropertiesMap type alias for everything that's (extra) properties, and a preserve_order feature that changes it to IndexMap, tell me what you think!

I didn't change the other BTreeMap actually because, as much as I think people order their struct properties by some logic as I do, I think for e.g. the map of types -> schema, since it's autogenerated and mixes all the types involved in documented endpoints, it probably depends on some rust compiler schenanigans or something, so I think alphabetical order does make sense in fact, rather than insertion order.

Right now my test does not test the preserve_order case because I didn't change the CI. Usually I use cargo hack to test all combinations but here it doesn't even start to execute tests on my machine, it gets killed by the oom-killer while computing the combinations 🤣 I could add a small two-case matrix to existing steps with/without the feature? It feels like it would give that feature a super big first-class importance that it maybe doesn't deserve compared to other features 🤔. But also it changes some "public" API so it is indeed not innocuous.

I added a PropertiesMap type alias for everything that's (extra) properties, and a preserve_order feature that changes it to IndexMap, tell me what you think!

This may have a potentially nasty side effect. If in a workspace, we have a crate using paperclip and then a new crate starts depending on preserver_order, that would potentially cause a breakage on the first crate because now where it expects BTreeMap, we have IndexMap?
In which case maybe it's better we make a breaking change now and simply use IndexMap outright?

Right now my test does not test the preserve_order case because I didn't change the CI. Usually I use cargo hack to test all combinations but here it doesn't even start to execute tests on my machine, it gets killed by the oom-killer while computing the combinations 🤣 I could add a small two-case matrix to existing steps with/without the feature? It feels like it would give that feature a super big first-class importance that it maybe doesn't deserve compared to other features 🤔. But also it changes some "public" API so it is indeed not innocuous.

All combinations might be tricky, because of actix3 and actix4 already have the issue highlighted above, they're not additive 😞
Maybe it's a good time to drop actix3 as well if we're doing a breaking change!