required_params will be matched with actual params to check if the token
may be used for authorization. For example if { job_id: 44 } is passed
as a required param, the token will be rejected for GET /jobs/33
In order to protect us from rendering a resource simply converted to
json, without processing it with API data class, this commit changes
JSON responder behavior to render 404 if we can't find associated data
class. The only exception to that rule is when resource is already a
Hash, meaning that it was processed before - we sometimes return for
example simple Hash responses like { result: true }.
The Hash exception could allow to accidentally pass resource.as_json to
responder, but in travis-ci/travis-support@124b8b6 I disabled default
as_json method on AR::Base classes, so the risk of such mistake is
lowered.
This commit changes travis-api to always return 404 response if resource
is not available. Previously we were returning image/png with "unknown"
status instead if user used "*/*" Accept header, which was confusing.
This hack is temporary and should be removed when we find better
solution.
TL;DR: we can't handle redirects to S3 using CORS, so in case we want to
get logs from S3 without additional requests to API, we need to return
status that will not be automatically redirected (in this case 204 seems
like the best answer).
Longer rationale:
Old logs are hosted on S3 now and in case log is not available in the
database, we would like to redirect to the archived log. Although S3
support CORS, our use case breaks on some browsers:
* when request is triggered to /jobs/:id/log and log is archived, api
returns 302 redirect, Location header points to the log on S3
* browser transparently redirects to given url, but it sets Origin to
null, for security reasons
* "Origin: null" is ok, because we allow every origin by setting
AllowedOrigin to "*"
* S3 returns "Access-Control-Allow-Origin: null" header, which breaks
some browsers (I confirmed it for webkit based browsers)
In order to fix this, S3 would need to return * in
Access-Control-Allow-Origin header or we would need to tell the browser
to not follow redirect. Both solutions are not achiveable.
Another option would be to return log information in job payload - we
could send log_url field which should be either log url on amazon or
null, but in such case we would need to query artifacts table in each
job request. This is something that should be avoided as archived logs
are not frequently requested - slowing down every request to get info
for it would be a waste.
We can do it, because we use :transaction strategy with DatabaseCleaner,
which starts transaction before each test and rollbacks after it. That
way data before each test is consistent.
The big advantage of such approach is that tests are fast now - we need
to only load Scenario data once.
One of the drawbacks, on the other hand, is that we need to always load
this data, even if no integration tests need running.
We can try to be smart about it and check if any integration tests are
loaded.
If there is an error somewhere along the line (like in DB connection
management), it should not interfere with returning proper result for
OPTIONS request. Otherwise it's hard to guess why the actual request in
the browser was not properly sent.
Rubinius does not yet destructure block (or method) arguments in 1.9 mode. This change makes the specs pass on Rubinius. It's not clear to me why destructering is being used here. Ruby will already destructure a single Array-ish argument into separate block argument locals. In this case, unless you want, give `a, b` passed, `name, content = a` and NOT `name, content = a, b`, the unparenthesized (ie non-extra-level-destructuring) should work fine. If that makes any sense. :)
