Unverified Commit a073af1f authored by Kefu Chai's avatar Kefu Chai Committed by GitHub
Browse files

fix(s3s/ops): differentiate Get and List operations by id parameter (#392) (#398) 



Fixed routing issue where GetBucket*Configuration and ListBucket*Configurations
operations shared the same query tag (analytics, intelligent-tiering, inventory,
metrics) but weren't properly differentiated. The first check would always match,
making List operations unreachable.

Now correctly routes based on presence of 'id' parameter:
- GET /?analytics&id=xxx → GetBucketAnalyticsConfiguration
- GET /?analytics → ListBucketAnalyticsConfigurations

Applied to: analytics, intelligent-tiering, inventory, and metrics operations.

Fixes #392

Signed-off-by: default avatarKefu Chai <tchaikov@gmail.com>
parent 2c20f0b2

codegen/src/v1/ops.rs

+30 −3
Original line number Diff line number Diff line
@@ -993,10 +993,37 @@ fn codegen_router(ops: &Operations, rust_types: &RustTypes) { 
                                (true, false) => {

                                    let tag = route.query_tag.as_deref().unwrap();



                                    // Special handling for operations that share the same query tag

                                    // but are differentiated by the presence of an 'id' parameter

                                    let needs_id_check = matches!(

                                        route.op.name.as_str(),

                                        "GetBucketAnalyticsConfiguration"

                                            | "GetBucketIntelligentTieringConfiguration"

                                            | "GetBucketInventoryConfiguration"

                                            | "GetBucketMetricsConfiguration"

                                    );

                                    let needs_no_id_check = matches!(

                                        route.op.name.as_str(),

                                        "ListBucketAnalyticsConfigurations"

                                            | "ListBucketIntelligentTieringConfigurations"

                                            | "ListBucketInventoryConfigurations"

                                            | "ListBucketMetricsConfigurations"

                                    );



                                    if needs_id_check {

                                        g!("if qs.has(\"{tag}\") && qs.has(\"id\") {{");

                                        succ(route, true);

                                        g!("}}");

                                    } else if needs_no_id_check {

                                        g!("if qs.has(\"{tag}\") && !qs.has(\"id\") {{");

                                        succ(route, true);

                                        g!("}}");

                                    } else {

                                        g!("if qs.has(\"{tag}\") {{");

                                        succ(route, true);

                                        g!("}}");

                                    }

                                }

                                (false, true) => {

                                    let (n, v) = qp.first().unwrap();

                                    g!("if super::check_query_pattern(qs, \"{n}\",\"{v}\") {{");

crates/s3s/src/ops/generated.rs

+8 −8
Original line number Diff line number Diff line
@@ -6562,16 +6562,16 @@ pub fn resolve_route( 
            S3Path::Root => Ok((&ListBuckets as &'static dyn super::Operation, false)),

            S3Path::Bucket { .. } => {

                if let Some(qs) = qs {

                    if qs.has("analytics") {

                    if qs.has("analytics") && qs.has("id") {

                        return Ok((&GetBucketAnalyticsConfiguration as &'static dyn super::Operation, false));

                    }

                    if qs.has("intelligent-tiering") {

                    if qs.has("intelligent-tiering") && qs.has("id") {

                        return Ok((&GetBucketIntelligentTieringConfiguration as &'static dyn super::Operation, false));

                    }

                    if qs.has("inventory") {

                    if qs.has("inventory") && qs.has("id") {

                        return Ok((&GetBucketInventoryConfiguration as &'static dyn super::Operation, false));

                    }

                    if qs.has("metrics") {

                    if qs.has("metrics") && qs.has("id") {

                        return Ok((&GetBucketMetricsConfiguration as &'static dyn super::Operation, false));

                    }

                    if qs.has("accelerate") {
@@ -6631,16 +6631,16 @@ pub fn resolve_route( 
                    if qs.has("publicAccessBlock") {

                        return Ok((&GetPublicAccessBlock as &'static dyn super::Operation, false));

                    }

                    if qs.has("analytics") {

                    if qs.has("analytics") && !qs.has("id") {

                        return Ok((&ListBucketAnalyticsConfigurations as &'static dyn super::Operation, false));

                    }

                    if qs.has("intelligent-tiering") {

                    if qs.has("intelligent-tiering") && !qs.has("id") {

                        return Ok((&ListBucketIntelligentTieringConfigurations as &'static dyn super::Operation, false));

                    }

                    if qs.has("inventory") {

                    if qs.has("inventory") && !qs.has("id") {

                        return Ok((&ListBucketInventoryConfigurations as &'static dyn super::Operation, false));

                    }

                    if qs.has("metrics") {

                    if qs.has("metrics") && !qs.has("id") {

                        return Ok((&ListBucketMetricsConfigurations as &'static dyn super::Operation, false));

                    }

                    if qs.has("uploads") {

crates/s3s/src/ops/generated_minio.rs

+8 −8
Original line number Diff line number Diff line
@@ -6577,16 +6577,16 @@ pub fn resolve_route( 
            S3Path::Root => Ok((&ListBuckets as &'static dyn super::Operation, false)),

            S3Path::Bucket { .. } => {

                if let Some(qs) = qs {

                    if qs.has("analytics") {

                    if qs.has("analytics") && qs.has("id") {

                        return Ok((&GetBucketAnalyticsConfiguration as &'static dyn super::Operation, false));

                    }

                    if qs.has("intelligent-tiering") {

                    if qs.has("intelligent-tiering") && qs.has("id") {

                        return Ok((&GetBucketIntelligentTieringConfiguration as &'static dyn super::Operation, false));

                    }

                    if qs.has("inventory") {

                    if qs.has("inventory") && qs.has("id") {

                        return Ok((&GetBucketInventoryConfiguration as &'static dyn super::Operation, false));

                    }

                    if qs.has("metrics") {

                    if qs.has("metrics") && qs.has("id") {

                        return Ok((&GetBucketMetricsConfiguration as &'static dyn super::Operation, false));

                    }

                    if qs.has("accelerate") {
@@ -6646,16 +6646,16 @@ pub fn resolve_route( 
                    if qs.has("publicAccessBlock") {

                        return Ok((&GetPublicAccessBlock as &'static dyn super::Operation, false));

                    }

                    if qs.has("analytics") {

                    if qs.has("analytics") && !qs.has("id") {

                        return Ok((&ListBucketAnalyticsConfigurations as &'static dyn super::Operation, false));

                    }

                    if qs.has("intelligent-tiering") {

                    if qs.has("intelligent-tiering") && !qs.has("id") {

                        return Ok((&ListBucketIntelligentTieringConfigurations as &'static dyn super::Operation, false));

                    }

                    if qs.has("inventory") {

                    if qs.has("inventory") && !qs.has("id") {

                        return Ok((&ListBucketInventoryConfigurations as &'static dyn super::Operation, false));

                    }

                    if qs.has("metrics") {

                    if qs.has("metrics") && !qs.has("id") {

                        return Ok((&ListBucketMetricsConfigurations as &'static dyn super::Operation, false));

                    }

                    if qs.has("uploads") {