Skip to content

Conversation

@alexeyzimarev
Copy link
Member

@alexeyzimarev alexeyzimarev commented Nov 21, 2025

User description

Description

Changes the default of AddUrlSegment to replace the parameter rather than adding it.
The reason for this change is that URL segments are unique, so adding more parameters with the same name has no effect as only the first one will be used. It's different from query or header parameters which allow multiple values for the same name.

It might be considered as a "breaking change" because when AddUrlSegment is called multiple times for the same parameter name, only the first value is used today. However, it makes the library easier to use, and the existing behaviour is meaningless anyway.

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

PR Type

Enhancement


Description

  • Change AddUrlSegment to replace existing parameter instead of adding duplicate

  • URL segments are unique identifiers, so multiple values with same name are meaningless

  • Add tests for URL segment replacement and query parameter handling

  • Simplify library behavior for URL segment parameters


Diagram Walkthrough

flowchart LR
  A["AddUrlSegment called<br/>with same name"] -->|Previously| B["Add new parameter<br/>only first used"]
  A -->|Now| C["Replace existing<br/>parameter value"]
  D["Query parameters"] -->|Unchanged| E["Allow multiple<br/>same name"]
Loading

File Walkthrough

Relevant files
Enhancement
RestRequestExtensions.Url.cs
Replace AddParameter with AddOrUpdateParameter call           

src/RestSharp/Request/RestRequestExtensions.Url.cs

  • Changed AddUrlSegment method to call AddOrUpdateParameter instead of
    AddParameter
  • This ensures URL segment parameters are replaced rather than
    duplicated
  • Maintains same method signature and documentation
+1/-1     
Tests
UrlSegmentTests.cs
Add URL segment replacement behavior test                               

test/RestSharp.Tests/Parameters/UrlSegmentTests.cs

  • Added new test AddSameUrlSegmentTwice_ShouldReplaceFirst to verify
    replacement behavior
  • Test adds same URL segment twice and validates second value replaces
    first
  • Minor formatting cleanup of whitespace in existing tests
+15/-2   
UrlBuilderTests.Get.cs
Add query parameter handling tests                                             

test/RestSharp.Tests/UrlBuilderTests.Get.cs

  • Added test Multiple_query_parameters_with_same_name to verify query
    parameters allow duplicates
  • Added test Query_parameter_with_non_string_value to verify non-string
    parameter values work correctly
  • Tests confirm query parameters behave differently from URL segments
+26/-0   

- Multiple query parameters with the same name
- Query parameters with non-string values
@qodo-merge-for-open-source
Copy link
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@alexeyzimarev
Copy link
Member Author

Fixes #2300

@qodo-merge-for-open-source
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Carefully consider this breaking API change

The suggestion highlights that changing AddUrlSegment to update instead of add
is a breaking change. It recommends this be released as part of a major version
with clear release notes to avoid breaking users' applications.

Examples:

src/RestSharp/Request/RestRequestExtensions.Url.cs [29]
            => request.AddOrUpdateParameter(new UrlSegmentParameter(name, value, encode));

Solution Walkthrough:

Before:

// Old behavior: "first one wins"
// Multiple calls with the same name add multiple parameters,
// but only the first one is used for URL segment replacement.
public RestRequest AddUrlSegment(string name, string? value, bool encode = true)
    => request.AddParameter(new UrlSegmentParameter(name, value, encode));

// Usage:
request.AddUrlSegment("id", "123"); // This value is used
request.AddUrlSegment("id", "456"); // This value is effectively ignored
// Resulting URL path: /resource/123

After:

// New behavior: "last one wins" (update)
// The call now updates any existing parameter with the same name.
public RestRequest AddUrlSegment(string name, string? value, bool encode = true)
    => request.AddOrUpdateParameter(new UrlSegmentParameter(name, value, encode));

// Usage:
request.AddUrlSegment("id", "123");
request.AddUrlSegment("id", "456"); // This value replaces the previous one
// Resulting URL path: /resource/456
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies the change as a breaking API change and highlights the critical need for proper versioning and communication, which is a major concern for library maintainability and user trust.

High
General
Refactor test to follow standard practices

Refactor the AddSameUrlSegmentTwice_ShouldReplaceFirst test to follow a more
conventional structure. Set up the request with both URL segment additions,
build the URI once, and then assert the final state.

test/RestSharp.Tests/Parameters/UrlSegmentTests.cs [74-85]

 [Fact]
 public void AddSameUrlSegmentTwice_ShouldReplaceFirst() {
     var client  = new RestClient();
-    var request = new RestRequest("https://api.example.com/orgs/{segment}/something");
-    request.AddUrlSegment("segment", 1);
-    var url1 = client.BuildUri(request);
-    request.AddUrlSegment("segment", 2);
-    var url2 = client.BuildUri(request);
+    var request = new RestRequest("https://api.example.com/orgs/{segment}/something")
+        .AddUrlSegment("segment", 1)
+        .AddUrlSegment("segment", 2);
+
+    var url = client.BuildUri(request);
     
-    url1.AbsolutePath.Should().Be("/orgs/1/something");
-    url2.AbsolutePath.Should().Be("/orgs/2/something");
+    url.AbsolutePath.Should().Be("/orgs/2/something");
 }
  • Apply / Chat
Suggestion importance[1-10]: 4

__

Why: The suggestion correctly proposes refactoring the test to follow a more standard Arrange-Act-Assert pattern, which improves test code readability and maintainability, although the original test is functionally correct.

Low
  • More

@github-actions
Copy link

Test Results

   35 files     35 suites   15m 50s ⏱️
  457 tests   457 ✅ 0 💤 0 ❌
3 089 runs  3 089 ✅ 0 💤 0 ❌

Results for commit 9a54720.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants