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

Support $.request.hash template #315

Closed
wants to merge 6 commits into from
Closed

Conversation

Pchelolo
Copy link
Contributor

A simple fix to support calculation of '{$.request.hash}' templates to calculate hashes of the request.

delete context.request.headers['x-request-id'];
}

var result = sha1(stringify(context.request));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should ensure consistent hashing. That means checking for standard request components (uri, headers, body, query) so as to avoid different hashes in situations where some are missing. E.g., these two requests will yield different hashes even though they should be treated equally:

request1:
  uri: /a/simple/path
  headers: {}
  body: ''

request2:
  uri: /a/simple/path
  body: {}

req.params = req.params || {};

// Need to remove x-request-id header if it's present
var reqId = req.headers && req.headers['x-request-id'];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary to check for req.headers now that we ensure the property exists.

@gwicke
Copy link
Member

gwicke commented Aug 27, 2015

It might be cleaner to implement this as a thin wrapper module on top of a key_value bucket, perhaps post_value? The hash would be calculated & returned from a POST request, and there would be one entry point to retrieve by hash. The bucket name would be passed through to the underlying key_value bucket.

@Pchelolo
Copy link
Contributor Author

@gwicke Yes, that's true, we've got out of the original plan. I will update the PR to implement a post_data module.

@Pchelolo
Copy link
Contributor Author

Actually, the new approach is completely different and not related to this PR, so I'd better close this one and create a new one with post_data module.

@Pchelolo Pchelolo closed this Aug 31, 2015
@Pchelolo Pchelolo deleted the hash branch December 21, 2015 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants