Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal to add ExtraProps to the configuration #1990

Closed
Burgyn opened this issue Mar 5, 2024 · 5 comments · Fixed by #1843
Closed

Proposal to add ExtraProps to the configuration #1990

Burgyn opened this issue Mar 5, 2024 · 5 comments · Fixed by #1843
Labels
merged Issue has been merged to dev and is waiting for the next release Spring'24 Spring 2024 release
Milestone

Comments

@Burgyn
Copy link

Burgyn commented Mar 5, 2024

If the need arises on the Ocelot usage side for an extension, we usually have to work with some custom settings in the Ocelot downstream route configuration as well.

The current options are that we put the custom settings into a custom setup and use the downstream key as a reference to it, or we can add them to the original JSON setup and read them ourselves (duplicate read).

From an extensibility point of view, it would be nice if we could add additional properties that the programmer could then read.

Proposal

 "ExtraProps" : {
    "CachePolicies": ["policy1", "policy2"],
    "SwaggerKey": "todo"
  }

Whole usage

{
    "Routes": [
        {
          "DownstreamPathTemplate": "/todos/{id}",
          "DownstreamScheme": "https",
          "DownstreamHostAndPorts": [
            {
               "Host": "jsonplaceholder.typicode.com",
               "Port": 443
            }
          ],
         "UpstreamPathTemplate": "/todos/{id}",
         "UpstreamHttpMethod": [ "Get" ],
         // 👇 additional properties for better extensibility
         "ExtraProps" : {
            "CachePolicies": ["policy1", "policy2"],
            "SwaggerKey": "todo",
            "AnotherProp": "value"
          }
        }
    ]
}

Usage in middleware:

PreQueryStringBuilderMiddleware = async (context, next) =>
{
  DownstreamRoute? route = context.Items.DownstreamRoute();
  var cachePolicies = route.GetExtraProps<string[]>("CachePolicies");
  ...
}

I can prepare PR for this change. I just want to check if this is a change that fits the direction of the project.

Thank you in advance for your reply.


{
    "Routes": [
        {
          "DownstreamPathTemplate": "/todos/{id}",
          "DownstreamScheme": "https",
          "DownstreamHostAndPorts": [
            {
               "Host": "jsonplaceholder.typicode.com",
               "Port": 443
            }
          ],
         "UpstreamPathTemplate": "/todos/{id}",
         "UpstreamHttpMethod": [ "Get" ],
         // 👇 additional properties for better extensibility
         "CachePolicies": ["policy1", "policy2"],
         "SwaggerKey": "todo",
         "AnotherProp": "value"
        }
    ]
}

Theoretically, we can also do that extra properties will be on the main level. This is nicer from a usage point of view, but more complicated to implement.
I don't know what you prefer.

@raman-m
Copy link
Member

raman-m commented Mar 5, 2024

Duplicate of #738

@raman-m raman-m marked this as a duplicate of #738 Mar 5, 2024
@raman-m
Copy link
Member

raman-m commented Mar 5, 2024

Hi Miňo!
Thanks for PR creation! But unfortunately your issue is duplicate of #738 which can be closed by #1843 (ready on 80-90%)


I can prepare PR for this change. I just want to check if this is a change that fits the direction of the project.

No, thanks! See #1843
I invite you to review #1843 only...
Your issue will be linked to #1843 too.

@Burgyn
Copy link
Author

Burgyn commented Mar 5, 2024

Hi @raman-m,

thanks for linking.

M.

@Burgyn Burgyn closed this as completed Mar 5, 2024
@raman-m
Copy link
Member

raman-m commented May 21, 2024

Reopen for official closing

@raman-m raman-m reopened this May 21, 2024
@raman-m raman-m added the 2023 Annual 2023 release label May 21, 2024
@raman-m raman-m added this to the Annual 2023 milestone May 21, 2024
ggnaegi added a commit that referenced this issue May 21, 2024
* feat(configuration): adding route metadata

* feat(configuration): update docs

* feat(configuration): replace Dictionary<> by IDictionary<>, code cleaning

* feat(configuration): replace Dictionary<> by IDictionary<>

* feat(configuration): replace Dictionary<> by IDictionary<>

* feat(configuration): update the data type of FileDynamicRoute Metadata

* formatting

* feat(configuration): fix integration tests

* feat !1843 add extension methods for DownstreamRoute to get metadata

* feat !1843 add extension methods for DownstreamRoute

* feat !1843 update docs

* feat !1843 update docs

* feat !1843 cleanup split string logic

* SA1505: An opening brace should not be followed by a blank line

* IDE1006: Naming rule violation: These words must begin with upper case characters: should_xxx

* Fix compile errors after rebasing

* Fix unit tests + AAA pattern

* First Version, providing a generic extension method GetMetadata<T> with global configuration

* Adding ConvertToNumericType method to be able to use the NumberStyles enum

* adding first acceptance tests

* The tests are now passing again...

* adding latest test cases. That should be enough (includes global configuration changes too)

* Update metadata.rst

* adding the xml docs for IMetadataCreator and MetadataCreator

* renaming MetadataCreator to DefaultMetadataCreator

* number tests for .net 6 too

* Moving Metadata specific downstream route extensions to the Metadata folder

* cleanup

* applying some of the requested changes

* Final code review by @raman-m

* Add traits

* Fix docs build error

---------

Co-authored-by: Raman Maksimchuk <dotnet044@gmail.com>
Co-authored-by: Guillaume Gnaegi <58469901+ggnaegi@users.noreply.github.com>
@raman-m
Copy link
Member

raman-m commented May 21, 2024

Closed by #1843

@raman-m raman-m closed this as completed May 21, 2024
@raman-m raman-m added merged Issue has been merged to dev and is waiting for the next release Spring'24 Spring 2024 release and removed 2023 Annual 2023 release labels May 21, 2024
@raman-m raman-m modified the milestones: Annual 2023, Spring'24 May 22, 2024
raman-m added a commit that referenced this issue May 23, 2024
* feat(configuration): adding route metadata

* feat(configuration): update docs

* feat(configuration): replace Dictionary<> by IDictionary<>, code cleaning

* feat(configuration): replace Dictionary<> by IDictionary<>

* feat(configuration): replace Dictionary<> by IDictionary<>

* feat(configuration): update the data type of FileDynamicRoute Metadata

* formatting

* feat(configuration): fix integration tests

* feat !1843 add extension methods for DownstreamRoute to get metadata

* feat !1843 add extension methods for DownstreamRoute

* feat !1843 update docs

* feat !1843 update docs

* feat !1843 cleanup split string logic

* SA1505: An opening brace should not be followed by a blank line

* IDE1006: Naming rule violation: These words must begin with upper case characters: should_xxx

* Fix compile errors after rebasing

* Fix unit tests + AAA pattern

* First Version, providing a generic extension method GetMetadata<T> with global configuration

* Adding ConvertToNumericType method to be able to use the NumberStyles enum

* adding first acceptance tests

* The tests are now passing again...

* adding latest test cases. That should be enough (includes global configuration changes too)

* Update metadata.rst

* adding the xml docs for IMetadataCreator and MetadataCreator

* renaming MetadataCreator to DefaultMetadataCreator

* number tests for .net 6 too

* Moving Metadata specific downstream route extensions to the Metadata folder

* cleanup

* applying some of the requested changes

* Final code review by @raman-m

* Add traits

* Fix docs build error

---------

Co-authored-by: Raman Maksimchuk <dotnet044@gmail.com>
Co-authored-by: Guillaume Gnaegi <58469901+ggnaegi@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged Issue has been merged to dev and is waiting for the next release Spring'24 Spring 2024 release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants