Skip to content
This repository has been archived by the owner on Jun 19, 2024. It is now read-only.

[feat] Response body stored (only for JSON responses) #100

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

blackadress
Copy link
Contributor

#9 partially solves this as it can only store JSON responses.

For JSON parsing I'm using the decoding part of this library https://github.com/rxi/json.lua adding it to the plugin

@NTBBloodbath
Copy link
Member

Hi, thanks for your interest in the project :)

Just a small question before merging, why use that json library instead of the built-in vim.json that is included in Neovim 0.6 onwards?

@blackadress
Copy link
Contributor Author

why use that json library instead of the built-in vim.json that is included in Neovim 0.6 onwards?

Because I didn't know it existed o.O

I'll check vim.json and refactor accordingly

@NTBBloodbath
Copy link
Member

Sweet, thank you!

It seems that our linter is raising some errors, can you please check our linter CI results and fix offenses if they're caused by this PR? Otherwise just let me know if they come from existing code in the repository and I'll fix them myself :)

Cheers

@blackadress
Copy link
Contributor Author

The linter doesn't like global lua variables, so I replaced it with neovim's global variables 😅, maybe there is a better way to initialize the variable? I just initialized it on rest-nvim/init.lua

vim.api.nvim_set_var("req_var_store", { __loaded = true })

When I tried to initialize it on rest-nvim.vim, just got all kind of weird unexpected behavior 🤔

@blackadress
Copy link
Contributor Author

@NTBBloodbath is there something else needed to merge this?

@guruor
Copy link

guruor commented Jul 5, 2022

Any update on this?

@blackadress
Copy link
Contributor Author

there is still a bug in there,
No such variable: "b:current_syntax"

This only happens from the second request onwards. and it happens here:

    local syntax_file = vim.fn.expand(string.format("$VIMRUNTIME/syntax/%s.vim", content_type))

    if vim.fn.filereadable(syntax_file) == 1 then
      vim.cmd(string.gsub(
        [[
        unlet b:current_syntax
        syn include @%s syntax/%s.vim
        syn region %sBody matchgroup=Comment start=+\v^#\+RESPONSE$+ end=+\v^#\+END$+ contains=@%s

        let b:current_syntax = "httpResult"
      ]],
        "%%s",
        content_type
      ))
    end

It confuses me, apparently the first time it gets the syntax_file it succeeds but then somehow it doesn't, any insights will be greatly appreciated.

@NTBBloodbath
Copy link
Member

NTBBloodbath commented Aug 18, 2022

Ah, can you pull upstream main branch changes in the PR? I did submit a fix for this that day and sorry for the inconveniences!

@blackadress
Copy link
Contributor Author

Done, haha I thought it was already synced

@MikaelElkiaer
Copy link

Trying this out right now. It is an awesome improvement!

I am utilizing this for a set-up like so:

keycloakHost: "http://localhost:8080"
keycloakRealm: "/realms/test"
clientId: "test"
accessToken: {{response.access_token}}

# well-known
GET {{keycloakRealm}}/.well-known/openid-configuration
Host: {{keycloakHost}}

# refresh token found in .env
#@response
POST {{keycloakRealm}}/protocol/openid-connect/token
Host: {{keycloakHost}}
Content-Type: application/x-www-form-urlencoded

grant_type=refresh_token
&client_id={{clientId}}
&refresh_token={{OAT_REFRESH_TOKEN}}

# route that requires a token
GET /api/test/auth
Host: http://localhost:8081
Authorization: Bearer {{accessToken}}

@guruor
Copy link

guruor commented Sep 5, 2022

This is much needed feature, thanks for adding it.
During my testing I found few issues with variable naming convention. Not sure if they are documented somewhere.

Check last two cases here:

page: {{api_response.page}}
pageNo: {{api_response.page}}
page-number: {{api_response.page}}
page_number: {{api_response.page}}
url: {{api_response.support.url}}

# normal var
foo: 4

#@api_response
GET https://reqres.in/api/users?page=5

GET https://reqres.in/api/users?page={{foo}}

GET https://reqres.in/api/users?page={{page}}

GET {{url}}

# Works with variable `pageNo`
GET https://reqres.in/api/users?page={{pageNo}}

# Doesn't work with variable `page-number`, replaces the value as page=%7B%7Bpage-number%7D%7D
GET https://reqres.in/api/users?page={{page-number}}

# Doesn't work with variable `page_number`, this error is produced [bad argument #2 to 'gsub' (string/function/table expected)]
GET https://reqres.in/api/users?page={{page_number}}

@guruor
Copy link

guruor commented Sep 5, 2022

One more thing I observed is that to access a stored variable in multiple http files, we need an explicit declaration in each http file, can this be moved to .env file.

Consider the snippet posted in this comment #100 (comment) as test1.http

test2.http

page: {{api_response.page}}

GET https://reqres.in/api/users?page={{page}}

GET https://reqres.in/api/users?page={{pageNo}}

Here, we are trying to reuse variables declared in test1.http in test2.http.
Variable page is accessible in test2.http because we redeclared it while pageNo is not accessible.

This can be a separate feature request, but posting it here since it is related and this feature is not yet merged.

@MikaelElkiaer
Copy link

I agree with @G0V1NDS. Related to his suggestion about using .env, I'd also add that it would be really useful to be able to set an initial value. E.g. page: {{response.page}} "0".

@blackadress
Copy link
Contributor Author

Welp, this is an issue #100 (comment) however I'm thinking this is not due to this PR, probably has to do with the way the plugin parses the variable names (I'm thinking this because the string 'api_response' is a valid variable that stores the json response and is properly stored)

As for being able to reuse variables declared in different http files the most sensible way of doing it would be as suggested in a '.env' separated from the files (currently the req_var aka the variable that stores the json response e.g. '#@api_response' is a global for the plugin but the other variables e.g. 'page:' are local to the file.

The other suggestion to have an initial value for variables sounds really useful but probably should be on its own issue. On this I would like the syntax page: {{response.page}} or 0

The suggestions sound cool but I'm quite busy this month so I'll get back on October, I still suggest to make an issue for each of the suggestions. As for the bug, I think it is from the main branch can you look into it @G0V1NDS, I think I may be able to look into it this weekend.

@MikaelElkiaer
Copy link

MikaelElkiaer commented Oct 20, 2022

Will this be going into main soon?

Never mind, just now noticed the stickied issue.

@MikaelElkiaer
Copy link

I am having some trouble with deconstructing arrays, and looking through the PR changes it does not seem to be handled.
Am I correct in this?

I assumed/hoped that I could do one of these:

a: {{response.array.0.prop}}
b: {{response.array[0].prop}}

@guruor
Copy link

guruor commented Feb 22, 2023

@blackadress I observed another issue today, not sure if it is related to your change.

float_value: {{api_response.var_name}}

If the value of var_name is of type float then it is not parsed.
If we use float_value in following APIs {{float_value}} is sent in the payload.

@guruor
Copy link

guruor commented Feb 26, 2023

Never mind, I was using the forked version, just tried the latest changes from official repo, it now has script-variables which works well for same use case. It doesn't have issue with float value for variables.

Though I started learning Neovim plugin development, will debug the variable parsing issue and will add tests for it.

@blackadress
Copy link
Contributor Author

Now it is possible to deconstruct arrays with this syntax:

{{response.array.0.prop}}

I'm a little bit torn on the syntax as I think using '[0]' would have been best, but '.0' was easier to implement

-- if so, then replace {{bar}} or {{bar.baz}} with the proper value else return the same string
-- only works if `bar` is a key in req_var_store
-- @param value_str the value to evaluate
M.replace_req_varibles = function(value_str)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lacks a a

@@ -240,8 +252,8 @@ local function end_request(bufnr, linenumber)
end
utils.move_cursor(bufnr, linenumber)

local next = vim.fn.search("^GET\\|^POST\\|^PUT\\|^PATCH\\|^DELETE\\|^###\\", "cn",
vim.fn.line("$"))
local next =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this due to the formatter ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

afaik

@teto
Copy link
Collaborator

teto commented Jul 11, 2023

same as #207 (comment) I think this should/could be implemented as a callback.
What is req_var_store and why do we need it ?

@teto
Copy link
Collaborator

teto commented Jul 11, 2023

do you think you could achieve something similar via #214 (it's ok to patch rest.nvim if it doesn't send enough information) ?

@blackadress
Copy link
Contributor Author

'req_var_store' is the variable in which I store the JSON variables e.g:

#@response
GET https://reqres.in/api/users?page=2

When executing this, the response is loaded in req_var_store like so:

req_var_store = { response = { ... full response ... } }

The other functionality is to check if it's loaded to validate on any try to get a key on req_var_store (now that I'm looking at it, this may be overkill)

Now about callbacks, I don't understand what you are saying, from #214 I get that you implemented callbacks to notify of the request being sent and finishing so wouldn't it just work? or do you want for this functionality to throw event when:

  • Starting the search to see if there is a valid JSON stored.
  • Ending the search with a valid/invalid response.
  • Loading a new variable to memory.

As I said I'm not following, I'd appreciate clarification.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants