microsoft / kiota

OpenAPI based HTTP Client code generator
https://aka.ms/kiota/docs
MIT License
3.02k stars 210 forks source link

Go - Add utility function to copy maps #2421

Closed baywet closed 1 year ago

baywet commented 1 year ago

In indexers or "navigation function with parameters" we need to copy the current path parameters, add to that the additional parameters for the current segment before we pass it on to the next request builder. While the current solution works. It repeats a lot of code

// MessagesById gets an item from the github.com/microsoft/kiota-samples/msgraph-mail/go/utilities/.users.item.messages.item collection
func (m *UserItemRequestBuilder) MessagesById(id string)(*ItemMessagesMessageItemRequestBuilder) {
    urlTplParams := make(map[string]string)
    for idx, item := range m.PathParameters {
        urlTplParams[idx] = item
    }
    if id != "" {
        urlTplParams["message%2Did"] = id
    }
    return NewItemMessagesMessageItemRequestBuilderInternal(urlTplParams, m.BaseRequestBuilder.RequestAdapter)
}

Could be easily optimized to

// MessagesById gets an item from the github.com/microsoft/kiota-samples/msgraph-mail/go/utilities/.users.item.messages.item collection
func (m *UserItemRequestBuilder) MessagesById(id string)(*ItemMessagesMessageItemRequestBuilder) {
    urlTplParams := abstractions.CopyPathParameters(m.PathParameters)
    if id != "" {
        urlTplParams["message%2Did"] = id
    }
    return NewItemMessagesMessageItemRequestBuilderInternal(urlTplParams, m.BaseRequestBuilder.RequestAdapter)
}

since the id parameter can be multiple parameters and they can have a different type, and due to the limitation of generic types, optimizing further might require a significant effort.

depends on #2406

rkodev commented 1 year ago

@baywet Submitted a PR with a slightly more generic name CopyStringMap so that it can be used in other sections of the code without causing confusion

baywet commented 1 year ago

@rkodev thanks for adding the necessary method, just to be clear, I'm expecting you to make the generation change after #2406 gets merged. You might want to reopen this issue.