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

Nested attributes with dynamic_jsonp_keys #79

Closed
michaelrkn opened this issue Dec 16, 2014 · 7 comments
Closed

Nested attributes with dynamic_jsonp_keys #79

michaelrkn opened this issue Dec 16, 2014 · 7 comments

Comments

@michaelrkn
Copy link
Contributor

I'm using the Balanced Payments API in a project. A typical request has parameters that look like this (converted to JSON for readability):

"data": {
  "name": "Example Brown 1",
  "number": "4111111111111111",
  "expiration_month": "12",
  "expiration_year": "2020",
  "cvv": "123",
  "address": {
    "postal_code": "11211"
  },
  "meta": {
    "capabilities_system_timezone": -8,
    "capabilities_user_agent": "Mozilla\/5.0 (Macintosh; PPC Mac OS X) AppleWebKit\/534.34 (KHTML, like Gecko) PhantomJS\/1.9.7 Safari\/534.34",
    "capabilities_language": "en-US",
    "capabilities_kp": 45,
    "capabilities_cli": 1,
    "capabilities_loaded": 1418668507849,
    "capabilities_screen_width": 2560,
    "capabilities_screen_length": 1440,
    "capabilities_hist": 1,
    "capabilities_cookie": "1418668507849.7760353146586567.1!0",
    "capabilities_cl": false,
    "capabilities_submitted": 1418668507899,
    "capabilities_scrollX": 0,
    "capabilities_scrollY": 0
  }
}

Because all of the keys are nested within the "data" key, using dynamic_jsonp_keys is very blunt - I have to either ignore everything or nothing. It would be nice to be able to specify nested keys to ignore, e.g.:

Billy.configure do |c|
  c.dynamic_jsonp_keys = ["callback", "data"["meta"]]
end

or something along those lines. I'm not exactly sure what the right interface would be.

The relevant code is at https://github.com/oesmith/puffing-billy/blob/master/lib/billy/cache.rb#L99.

@ronwsmith
Copy link
Collaborator

That config option manipulates the GET query string for the cache.

I think the fix you're looking for would be in the cache key generation here: https://github.com/oesmith/puffing-billy/blob/master/lib/billy/cache.rb#L80.

Seems like you want to remove keys from the request body before using the body to generate the cache key.

Want to take a stab at adding a new config option for that?

@michaelrkn
Copy link
Contributor Author

Actually, all of these parameters are in the URL. The gem uses Faraday for the requests, so I'm guessing putting the parameters in the URL is part of Faraday's implementation. Is this unusual? If so, is it too unusual to justify adding support for removing nested keys?

@ronwsmith
Copy link
Collaborator

@michaelrkn, whoa, Balanced Payments sends credit card details in the GET query string? That violates nearly every rule about when to use a GET. I haven't seen nested GET params in a very long time and have reservations about helping enable Balanced Payments' poor architecture if that's indeed the case.

@michaelrkn
Copy link
Contributor Author

Because the balanced.js library uses JSONP, it creates an async <script> tag to send the information and execute the callback:

function jsonp(path, callback) {
    var funct = "balanced_jsonp_" + Math.random().toString().substr(2);
    var tag = document.createElement("script");
    tag.type = "text/javascript";
    tag.async = true;
    tag.src = path.replace("{callback}", funct);
    var where = document.getElementsByTagName("script")[0];
    where.parentNode.insertBefore(tag, where);
    window[funct] = function(result) {
        try {
            callback(result);
        } catch (e) {
            if (typeof console !== "undefined" && console.error) {
                console.error(e);
            }
        }
        tag.parentNode.removeChild(tag);
    };
}

So, yes, it is performing a GET request, but that's because it's using JSONP. Honestly, I didn't actually understand how JSONP until doing a bit of research just now, but this seems to be the standard way to make requests with JSONP.

@ronwsmith
Copy link
Collaborator

@michaelrkn, is this still an issue? I see that the related issue in your repo was closed in June.

@michaelrkn
Copy link
Contributor Author

We switched from Balanced to Stripe and no longer need to ignore nested keys. I have no idea if Puffing Billy supports this now, but my feelings won't be hurt if you decide this isn't important and close the issue :)

@ronwsmith
Copy link
Collaborator

I'll close it for now until another Balanced user runs into the same issue.

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

No branches or pull requests

2 participants