Allow setting different responses per request #31

[infinite-spectrum]

issue: #31

Following is the summary of the changes done in the PR so far:

• As part of this PR, I've introduced a new way to handle Random/Repeatable responses.
• I've used burst terminology for repeatable responses.

Changed Items:

• Changed response field in an imposter to responses (array of response)

Added Items:

• Added burst field in response schema under imposter schema

  • this new field has a +integer value, which represents no of times a response should be repeated.
  • valid only in BURST mode, ignored in RANDOM mode
  • in BURST mode if ignored/missing, the default value is 1

• Added responseMode field in request schema under imposter schema

  • this new field can have two string values RANDOM or BURST
  • default value is RANDOM
  • any other values will be ignored and default values will be selected instead
  • if the field is missing/ignored, the default value will be considered

Input required:

• Migration strategy for existing imposter schema to new schema (response -> array of response)
• Determining default value for responseMode which is currently RANDOM

[infinite-spectrum]

@joanlopez

• What do you think about moving the responseMode to the imposter's level or to make the response field an object with mode and payloads fields within?

>> I'd go with 2nd option rather than the 1st one.

• Have you tested your implementation with concurrent requests? I might be wrong (as I said I had no time to do some tests neither look at it in-detail), but looks like you're increasing the requests counter with no protection against concurrent requests (ergo potential data races).

>> Nope, this is a miss from my end. I'll incorporate this one.

[joanlopez]

• What do you think about moving the responseMode to the imposter's level or to make the response field an object with mode and payloads fields within?

I'd go with 2nd option rather than the 1st one.

I'm still wondering about how big that change is. In comparison with tools like Mountebank, in Killgrave is much simpler to define your imposters, so with this kind of change (forcing the user to define an array, etc) we're losing the added value that Killgrave brings into the ecosystem.

That being said, do you think it could work to either:

• Make the imposters parser work with both schemas (older and newer).
• Base this new feature in a completely new field (response vs responses or similar).

What do you think? cc/ @aperezg

• Have you tested your implementation with concurrent requests? I might be wrong (as I said I had no time to do some tests neither look at it in-detail), but looks like you're increasing the requests counter with no protection against concurrent requests (ergo potential data races).

Nope, this is a miss from my end. I'll incorporate this one.

No worries at all, that's why code reviews are for. Feel free to ping me if you have any question related with that.

Again, thanks a lot for your engagement with the project and your valuable contributions 🙏🏻

[aperezg]

hello @infinite-spectrum will you continue with this PR? seems very good contibution so I hope to see this finish :D

[infinite-spectrum]

@aperezg, sorry for the late submission. I've fixed the code for concurrent requests. Please review and let me know any suggestions/issues.

[infinite-spectrum]

@aperezg, we have to discuss one question - How to deal with the schema change? (responses instead of response in the imposter file.)

Here are few ideas:

• Make current code to work with both imposter schemas. (@joanlopez's idea)
• Write a one-time utility to convert imposter from previous version to the current one

Any other ideas would be appreciated. Thank you

[aperezg]

Hello @infinite-spectrum, sorry for the late response 🙏 so for continue with the PR, I think that the best idea would be that joan said, support the responses and response :)