Adds a feature where even request with GET query strings having dependent values can be pipelined #96

adisingh007 · 2018-09-05T09:36:57Z

Closes #95

Now when the requests are:

{
    "requests": [
        {
             "method": "GET",
             "path": "/get/product?product_id=564"
        },
        {
             "method": "GET",
             "path": "/get/manufacturer",
             "query": {
                 "manufacturter_id": "$0.manufacturer_id"
             }
        }
    ]
}

inspite of being a query parameter(and not a path parameter), $0.manufacturer_id will now be parsed.

Hence, even pipe lined query parameters will now be parsed if dependent on previous requests.

cadecairos

Thanks for the PR @adisingh007, this is a great first step but there's a few things that need to be worked on before we can land this.

cadecairos · 2018-09-13T02:44:23Z

test/batch.js

+
+    it('Now parses URL query params in pipelined requests', async () => {
+
+        const res = await Internals.makeRequest(server, `{ "requests": [ {"method": "get", "path": "/string/${encodeURI('Aditya Pratap Singh')}"}, { "method": "get", "path": "/profile", "query": {"id": "$0"}}, { "method": "post", "path": "/echo", "payload": { "name": "$1.id" }} ] }`);


Please make the test string a non-identifying piece of data, rather than your full name.

cadecairos · 2018-09-13T02:44:51Z

test/batch.js

+
+        expect(res[0]).to.equal('Aditya Pratap Singh');
+        expect(res[1].id).to.equal('Aditya Pratap Singh');
+        expect(res[2].name).to.equal('Aditya Pratap Singh');


Same request here, and the lines above.

cadecairos · 2018-09-13T03:11:26Z

lib/batch.js

@@ -33,6 +33,10 @@ module.exports.config = function (settings) {

            request.payload.requests.every((req, idx) => {

+                if (req.query) {
+                    req.payload = req.query;


We should not convert query parameters into request payloads in order to pipeline data into query parameters.

Bassmaster should look at the request it's processing, and if it contains a query object, it should see if previous request responses need to be pipelined into it before constructing a URL and injecting it.

@cadecairos Hello Chris,
I have made another commit. Please have a look at it. Just how it was desired. Bassmaster looks at the request it is processing, and if the request contains a query object, it sees if the previous request responses needs to be pipelined into it before constructing a URL and injecting it. Wrote a test case too to cover the same.

Please review and let me know if anything else also needs to be done.

Thankyou,
Aditya

adisingh007 · 2018-09-17T01:32:44Z

@cadecairos I have made the required changes. Now it works as it was expected. Bassmaster now looks if the request has a query object and if it depends on responses of previous request. Just like it handles payloads depended on responses of previous request. Please review.

cadecairos

This is great! I have one requested change!

cadecairos · 2018-09-20T01:00:37Z

lib/batch.js


 internals.requestRegex = /(?:\/)(?:\$(\d+)\.)?([^\/\$]*)/g;

-internals.parsePayload = function (obj) {
+internals.parseParsable = function (obj) {


lets just call this parse

@cadecairos Done

…l functions

… both pipelinable query and pipelinable payload objects

cadecairos

Wonderful stuff, thanks @adisingh007

cadecairos suggested changes Sep 13, 2018

View reviewed changes

adisingh007 force-pushed the hapi#bassmaster#95 branch 3 times, most recently from deca5ce to 18453c5 Compare September 16, 2018 17:59

adisingh007 force-pushed the hapi#bassmaster#95 branch from 18453c5 to da14dbe Compare September 18, 2018 06:03

adisingh007 changed the title ~~Hapi#bassmaster#95~~ Adds a feature where even request with GET query strings having dependent values can be pipelined Sep 18, 2018

adisingh007 force-pushed the hapi#bassmaster#95 branch from da14dbe to 955c698 Compare September 18, 2018 18:15

cadecairos suggested changes Sep 20, 2018

View reviewed changes

adisingh007 added 2 commits September 20, 2018 07:13

Now query parameters support pipelined requests

7ee3597

Refactored the code. Changed naming of internal variables and interna…

baf3aaf

…l functions

adisingh007 force-pushed the hapi#bassmaster#95 branch from 3a8927c to baf3aaf Compare September 20, 2018 01:45

Modified test to demonstrate that the feature works for requests with…

edaf353

… both pipelinable query and pipelinable payload objects

cadecairos approved these changes Sep 30, 2018

View reviewed changes

cadecairos merged commit c371d51 into outmoded:master Sep 30, 2018

adisingh007 deleted the hapi#bassmaster#95 branch September 30, 2018 04:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds a feature where even request with GET query strings having dependent values can be pipelined #96

Adds a feature where even request with GET query strings having dependent values can be pipelined #96

adisingh007 commented Sep 5, 2018 •

edited

Loading

cadecairos left a comment

cadecairos Sep 13, 2018

cadecairos Sep 13, 2018

cadecairos Sep 13, 2018

adisingh007 Sep 16, 2018 •

edited

Loading

adisingh007 commented Sep 17, 2018 via email •

edited

Loading

cadecairos left a comment

cadecairos Sep 20, 2018

adisingh007 Sep 20, 2018

cadecairos left a comment


		it('Now parses URL query params in pipelined requests', async () => {

		const res = await Internals.makeRequest(server, `{ "requests": [ {"method": "get", "path": "/string/${encodeURI('Aditya Pratap Singh')}"}, { "method": "get", "path": "/profile", "query": {"id": "$0"}}, { "method": "post", "path": "/echo", "payload": { "name": "$1.id" }} ] }`);

Adds a feature where even request with GET query strings having dependent values can be pipelined #96

Adds a feature where even request with GET query strings having dependent values can be pipelined #96

Conversation

adisingh007 commented Sep 5, 2018 • edited Loading

cadecairos left a comment

Choose a reason for hiding this comment

cadecairos Sep 13, 2018

Choose a reason for hiding this comment

cadecairos Sep 13, 2018

Choose a reason for hiding this comment

cadecairos Sep 13, 2018

Choose a reason for hiding this comment

adisingh007 Sep 16, 2018 • edited Loading

Choose a reason for hiding this comment

adisingh007 commented Sep 17, 2018 via email • edited Loading

cadecairos left a comment

Choose a reason for hiding this comment

cadecairos Sep 20, 2018

Choose a reason for hiding this comment

adisingh007 Sep 20, 2018

Choose a reason for hiding this comment

cadecairos left a comment

Choose a reason for hiding this comment

adisingh007 commented Sep 5, 2018 •

edited

Loading

adisingh007 Sep 16, 2018 •

edited

Loading

adisingh007 commented Sep 17, 2018 via email •

edited

Loading