betfair / cougar

Cougar is a framework for making building network exposed service interfaces easy.
http://betfair.github.io/cougar
Apache License 2.0
27 stars 18 forks source link

Allow GETs on operations with complex types #26

Open eswdd opened 10 years ago

eswdd commented 10 years ago

1) As an internal developer I want APIs to be easy to code against So that my dev/maintenance tasks are easier, and produce cleaner code

2) As a internal/external developer/tester I want to be able to exercise APIs with a browser where possible So that I have low barriers to integration and testing

At the moment we have (2) at the expense of (1), e.g. one call has 21 parameters, so a call can look like this:

service.doSomething(null, null, null, something, null, null, null, ... (and so on).

To facilitate (1) and (2) simultaneously, allow GETs for complex types.

Possible implementations already discussed somewhat:

a) All items in non-aggregator types must be stringable and not have any naming collisions (or be qualified appropriately) b) Put JSON object on query string

Also:

Not very readable (subjective):

service.executeQuery(true, 43843, “1234”, null, null, OrderType.EXECUTABLE, null, null);

Much more readable:

            Query req = new Query();
            req.setAllTransactions(true);
            req.setAccountId(43843);
            req.setReferenceNumber(“1234”)
            req.setOrderType(OrderType.EXECUTABLE);
            service.executeQuery(req);

Can be wrapped with sugar:

            service.executeQuery(
                        new Query()
                                    .allTransactions()
                                    .forAccountId(43843)
                                    .withReferenceNumber(“1234”)
                                    .ofOrderType(OrderType.EXECUTABLE)
                                    .build()
                        )
            );

Request objects can also be constructed in Spring, cloned, passed around easily, and so on.

Request objects also don’t suffer from the major (IMO) problem of breaking generated clients at compile time if I add an optional parameter. The way we are doing it, all clients have to add a ‘null’ onto all their calls.

Why don’t we use request objects in our IDDs I wonder? Is there a good reason?

The flat signature in an IDD is often so it can still be called in a browser easily, we have no way to have complex request objects represented in a GET request at the moment.

It would be nice to remove that limitation. Instead of generating a synthetic request for a flat IDD signature, I wonder whether it would be better to make a more OO IDD and use dot notation for specifying object properties on the URI:

?request.thing=1&request.otherThing=1,2,3,4&request.nestedThing.property=foo

Perhaps even to make this backwards compatible it could assume that if you don’t specify a full path that matches anything it should scan the hierarchy and insert the value in any property that matches by name (so thing=1 would still get injected into request).

bfmike commented 10 years ago

You might want to request a security review before progressing too far on the implementation of this idea. Some of it is fine, whereas other parts are dangerous and could lead to parameter spoofing, sensitive internal objects being trashed and maybe even arbitrary code execution.

If you are going to build request objects and then present them to the cougar application then "?request.thing=1" on the URI should result in a request object being created, with a thing=1 attribute and that request object being injected into the main request object.

And I bet you thought I wasn't reading all these ideas..!

eswdd commented 10 years ago

"If you are going to build request objects and then present them to the cougar application then "?request.thing=1" on the URI should result in a request object being created, with a thing=1 attribute and that request object being injected into the main request object."

This is I think what was implicitly intended by the original requester.

atropineal commented 10 years ago

This is I think what was implicitly intended by the original requester.

I was the original requester, and that is correct.