Skip to content

GH-439 Make HttpMethod#all static #440

Merged
ryber merged 2 commits into
Kong:mainfrom
dzikoysk:gh-439/make-httpmethod-all-static
Apr 6, 2022
Merged

GH-439 Make HttpMethod#all static #440
ryber merged 2 commits into
Kong:mainfrom
dzikoysk:gh-439/make-httpmethod-all-static

Conversation

@dzikoysk

@dzikoysk dzikoysk commented Apr 6, 2022

Copy link
Copy Markdown
Contributor

@CLAassistant

CLAassistant commented Apr 6, 2022

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@dzikoysk

dzikoysk commented Apr 6, 2022

Copy link
Copy Markdown
Contributor Author

Oh, that's quite funny, because it looks like HttpMethod stores everything that have been requested:

    @Test
    void weirdVerbs() {
        Unirest.request("CHEESE", MockServer.CHEESE)
                .asObject(RequestCapture.class)
                .getBody()
                .assertMethod(HttpMethod.valueOf("CHEESE"))
                .assertBody("");
    }

Results later in:

Error:    
HttpMethodTest.shouldReturnAllMethods:51 
expected: <[PATCH, GET, OPTIONS, DELETE, POST, PUT, TRACE, HEAD]>
  but was: <[PATCH, CHEESE, GET, OPTIONS, DELETE, POST, PUT, TRACE, HEAD]>

@dzikoysk

dzikoysk commented Apr 6, 2022

Copy link
Copy Markdown
Contributor Author

It looks like custom methods are considered as a feature, so what do you think about extending HttpMethod with a property canonic/standard, so we can differ those methods? Or we can just remove this test if the registry it meant to keep non-standard values for the whole jvm instance.

@ryber

ryber commented Apr 6, 2022

Copy link
Copy Markdown
Collaborator

I'd just change the test to:

assertEquals(HttpMethod.GET.all(), HttpMethod.all());

@ryber

ryber commented Apr 6, 2022

Copy link
Copy Markdown
Collaborator

You could also check that it contains the default (but might have more which is fine).

but yes, there re some systems with custom methods. mostly old

@ryber ryber merged commit 6979341 into Kong:main Apr 6, 2022
@dzikoysk dzikoysk deleted the gh-439/make-httpmethod-all-static branch April 6, 2022 14:27
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.

3 participants