Skip to content

Implement a new configuration merging logic#2951

Merged
imobachgs merged 7 commits intoapi-v2from
config-merge
Dec 17, 2025
Merged

Implement a new configuration merging logic#2951
imobachgs merged 7 commits intoapi-v2from
config-merge

Conversation

@imobachgs
Copy link
Copy Markdown
Contributor

@imobachgs imobachgs commented Dec 16, 2025

Problem

In the new API, when merging collections (e.g., a Vec), both of them are merged together. For instance, if you send a PATCH with a list of patterns for installation, it would add them to the existing list.

The way to reset the list of patterns is to use PUT, but it forces to send the whole configuration.

The reason for this behavior is that we rely on merge_struct for merging the configuration. It was kind of an urgent solution, but we might need to search for a better alternative (or implement our own).

Solution

Replace the whole list, so we can add, update, and remove elements using a PATCH request. It uses the merge crate. When reviewing the code, take into account that when calling the merge function, the receiver is the configuration to update, while the argument to this function is the original one.

config.update(original);

Notes

Most of the unit tests are generated by Gemini, although some minimal changes were required.

Open question

  • How to proceed in the storage section?

* This new logic replaces collections instead of appending the new
  values.
* It relies on the "merge" crate.
#[derive(Clone, Debug, Default, Serialize, Deserialize, Merge, utoipa::ToSchema)]
pub struct Config {
#[serde(default, skip_serializing_if = "Vec::is_empty")]
#[merge(strategy = merge::vec::overwrite_empty)]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so it means, if I intentionally send empty list of files, it won't be overwritten and instead it will keep old list of files? I find this strange.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think it is wrong. But IMHO the real problem is that we should use an Option here too. Otherwise, you cannot differentiate when you want to reset the list.

assert_eq!(pre_scripts.len(), 1);
let script = pre_scripts.get(0).unwrap();
assert_eq!(&script.base.name, "updated");
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I miss tests for files case.

pub struct NetworkConnectionsCollection(pub Vec<NetworkConnection>);
#[derive(Clone, Debug, Default, Serialize, Deserialize, Merge, utoipa::ToSchema, PartialEq)]
pub struct NetworkConnectionsCollection(
#[merge(strategy = merge::vec::overwrite_empty)] pub Vec<NetworkConnection>,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would avoid having this on one line, is possible to have it on two?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and question is also here how it will behave when I send intentionally empty list of network connections, will it overwrite or not?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I left the formatting to rustfmt and it does it in this way.

}],
};
updated2.merge(original2.clone());
assert_eq!(updated2.answers.len(), 1);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and this exactly show why I do not think that overwrite_empty is good one for arrays as it means, that you cannot set intentionally empty list.

@imobachgs imobachgs merged commit 6e1f486 into api-v2 Dec 17, 2025
18 of 19 checks passed
@imobachgs imobachgs deleted the config-merge branch December 17, 2025 07:31
imobachgs added a commit that referenced this pull request Jan 10, 2026
Merge the new HTTP API. Each PR has been already reviewed, so it should
be safe to merge it.

* #1829
* #2508
* #2772
* #2826
* #2848
* #2860
* #2863
* #2866
* #2867
* #2869
* #2870
* #2871
* #2872
* #2873
* #2874
* #2875
* #2876
* #2877
* #2880
* #2881
* #2882
* #2884
* #2885
* #2886
* #2891
* #2892
* #2893
* #2894
* #2895
* #2896
* #2897
* #2898
* #2899
* #2900
* #2901
* #2902
* #2903
* #2904
* #2908
* #2909
* #2910
* #2912
* #2913
* #2914
* #2915
* #2916
* #2917
* #2918
* #2920
* #2921
* #2923
* #2924
* #2926
* #2928
* #2929
* #2930
* #2933
* #2934
* #2935
* #2936
* #2938
* #2939
* #2942
* #2943
* #2944
* #2945
* #2946
* #2947
* #2948
* #2950
* #2951
* #2952
* #2954
* #2955
* #2956
* #2957
* #2958
* #2959
* #2960
* #2961
* #2963
* #2964
* #2965
* #2967
* #2968
* #2969
* #2970
* #2971
* #2972
* #2974
* #2975
* #2977
* #2978
* #2980
* #2982
* #2983
* #2984
* #2988
* #2989
* #2991
* #2992
* #2993
* #2994
* #2995
* #2996
* #2997
* #2999
@imobachgs imobachgs mentioned this pull request Mar 17, 2026
imobachgs added a commit that referenced this pull request Mar 17, 2026
Prepare to release version 19.

* #1829
* #2508
* #2772
* #2818
* #2826
* #2848
* #2860
* #2863
* #2864
* #2866
* #2867
* #2869
* #2870
* #2871
* #2872
* #2873
* #2874
* #2875
* #2876
* #2877
* #2880
* #2881
* #2882
* #2884
* #2885
* #2886
* #2891
* #2892
* #2893
* #2894
* #2895
* #2896
* #2897
* #2898
* #2899
* #2900
* #2901
* #2902
* #2903
* #2904
* #2908
* #2909
* #2910
* #2912
* #2913
* #2914
* #2915
* #2916
* #2917
* #2918
* #2920
* #2921
* #2923
* #2924
* #2926
* #2928
* #2929
* #2930
* #2933
* #2934
* #2935
* #2936
* #2937
* #2938
* #2939
* #2942
* #2943
* #2944
* #2945
* #2946
* #2947
* #2948
* #2949
* #2950
* #2951
* #2952
* #2954
* #2955
* #2956
* #2957
* #2958
* #2959
* #2960
* #2961
* #2963
* #2964
* #2965
* #2967
* #2968
* #2969
* #2970
* #2971
* #2972
* #2974
* #2975
* #2977
* #2978
* #2980
* #2981
* #2982
* #2983
* #2984
* #2988
* #2989
* #2990
* #2991
* #2992
* #2993
* #2994
* #2995
* #2996
* #2997
* #2998
* #2999
* #3000
* #3001
* #3002
* #3004
* #3005
* #3006
* #3007
* #3008
* #3009
* #3011
* #3012
* #3013
* #3014
* #3015
* #3016
* #3018
* #3019
* #3020
* #3021
* #3022
* #3023
* #3024
* #3025
* #3026
* #3027
* #3028
* #3029
* #3030
* #3031
* #3033
* #3034
* #3035
* #3036
* #3037
* #3039
* #3040
* #3041
* #3042
* #3043
* #3044
* #3045
* #3046
* #3047
* #3048
* #3049
* #3050
* #3051
* #3052
* #3053
* #3054
* #3055
* #3056
* #3057
* #3058
* #3060
* #3061
* #3062
* #3063
* #3064
* #3065
* #3066
* #3067
* #3068
* #3069
* #3070
* #3071
* #3072
* #3073
* #3074
* #3075
* #3076
* #3077
* #3078
* #3079
* #3086
* #3087
* #3088
* #3089
* #3090
* #3091
* #3092
* #3093
* #3094
* #3095
* #3096
* #3097
* #3098
* #3099
* #3100
* #3101
* #3102
* #3103
* #3104
* #3105
* #3106
* #3107
* #3108
* #3109
* #3110
* #3112
* #3113
* #3114
* #3115
* #3116
* #3117
* #3118
* #3119
* #3120
* #3122
* #3123
* #3124
* #3127
* #3128
* #3129
* #3130
* #3131
* #3133
* #3134
* #3135
* #3136
* #3137
* #3138
* #3139
* #3140
* #3141
* #3142
* #3143
* #3144
* #3145
* #3146
* #3147
* #3148
* #3149
* #3150
* #3151
* #3152
* #3153
* #3154
* #3155
* #3157
* #3158
* #3159
* #3160
* #3161
* #3162
* #3163
* #3164
* #3165
* #3166
* #3167
* #3168
* #3169
* #3170
* #3174
* #3175
* #3176
* #3177
* #3178
* #3179
* #3181
* #3182
* #3184
* #3185
* #3186
* #3188
* #3189
* #3190
* #3191
* #3192
* #3194
* #3195
* #3196
* #3197
* #3198
* #3199
* #3200
* #3201
* #3202
* #3203
* #3205
* #3206
* #3208
* #3209
* #3210
* #3213
* #3214
* #3215
* #3216
* #3217
* #3218
* #3219
* #3220
* #3222
* #3223
* #3224
* #3225
* #3226
* #3227
* #3228
* #3229
* #3230
* #3231
* #3232
* #3233
* #3234
* #3235
* #3236
* #3237
* #3238
* #3239
* #3240
* #3241
* #3242
* #3243
* #3244
* #3246
* #3247
* #3248
* #3250
* #3251
* #3252
* #3253
* #3254
* #3255
* #3256
* #3257
* #3258
* #3259
* #3260
* #3261
* #3262
* #3263
* #3265
* #3266
* #3267
* #3268
* #3269
* #3270
* #3271
* #3272
* #3273
* #3274
* #3275
* #3276
* #3277
* #3278
* #3279
* #3280
* #3281
* #3282
* #3283
* #3284
* #3285
* #3286
* #3287
* #3288
* #3289
* #3290
* #3291
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants