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

Allow variables in bodyFile path #175

Open
eloo-abi opened this issue Aug 19, 2024 · 3 comments · May be fixed by #176
Open

Allow variables in bodyFile path #175

eloo-abi opened this issue Aug 19, 2024 · 3 comments · May be fixed by #176

Comments

@eloo-abi
Copy link

Hi,

i would be great if we could allow variables in bodyFile path so it would be possible to have different responses for different path variables without creating a lot of imposters.

A possible config could look like this:

[
  {
    "request": {
      "method": "GET",
      "endpoint": "/orders/{order_id:.*}"
    },
    "response": {
      "status": 200,
      "headers": {
        "Content-Type": "application/json"
      },
      "bodyFile": "../bodies/order_{order_id}.json"
    }
  }
]

Best regards

@eloo-abi eloo-abi linked a pull request Aug 19, 2024 that will close this issue
@joanlopez
Copy link
Member

Hey @eloo-abi, thanks for the feature requests (and your contribution!) 🚀

However, I think that before moving forward with this development, would be nice to define better the expected behavior under different conditions, and reach consensus. For instance, what should happen when:

  • There's a syntax error (reference to a variable that isn't defined)
  • There's a variable with multiple values

Thanks!

@eloo-abi
Copy link
Author

Hi,

can you give an example what a variable with multiple values is?
I really don't get what you mean here.

There's a syntax error (reference to a variable that isn't defined)
Then it will behave like the current implementation if the bodyFile could not be found.

The implementation here is super simple and just adjusts the bodyFile path.
If the path is not valid it will exactly behave as if someone has entered a wrong path manually.

@joanlopez
Copy link
Member

joanlopez commented Aug 21, 2024

Sorry, by looking at the PR, I got confused between mux methods, and I was thinking about query params (see Vars() vs Query() 😓 ), that's why I thought about cases where a variable can have multiple values (for instance, like ?id=5&id=3 or ?id[]=5&id[]=3). Apologies 🙏🏻

Please, update the PR with some tests, considering the edge cases I mentioned and I think we'll be able to merge it!
Again, thanks for your contribution! 🙇🏻

eloo-abi added a commit to eloo-abi/killgrave that referenced this issue Aug 21, 2024
Merge in OSSC/killgrave from feature/friendsofgoGH-175-reuse-path-variable-for-bodyfile to feat/friendsofgoGH-175-reuse-path-variable-for-bodyfile

* commit '18e28e7f7c16c2a4fc670f2aae6b744a82c1be51':
  remove unused line
  test: add tests for variables in body file
eloo-abi added a commit to eloo-abi/killgrave that referenced this issue Sep 13, 2024
Merge in OSSC/killgrave from feature/friendsofgoGH-175-reuse-path-variable-for-bodyfile to feat/friendsofgoGH-175-reuse-path-variable-for-bodyfile

* commit 'c9dbb4b7afa8c138d678ba338d7f4be397297034':
  fix: adjust test data and handle errors
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 a pull request may close this issue.

2 participants