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.

Sign up for GitHub

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

storage: introduce requesterPays #2392

Merged
merged 14 commits into from
Jun 29, 2017
Merged

storage: introduce requesterPays #2392

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
736b035
storage: introduce requesterPays
stephenplusplus Jun 14, 2017
d3d6a58
test index.js
stephenplusplus Jun 16, 2017
2277afa
test bucket.js
stephenplusplus Jun 16, 2017
7dea494
lint fix
stephenplusplus Jun 16, 2017
7c57293
test file.js
stephenplusplus Jun 16, 2017
7a1585b
update gcs-resumable-upload
stephenplusplus Jun 16, 2017
d2afdc2
add system tests
stephenplusplus Jun 23, 2017
4ab2587
debug
stephenplusplus Jun 26, 2017
376e285
bring in label methods
stephenplusplus Jun 26, 2017
4756aee
export non-whitelist CI account service account JSON
stephenplusplus Jun 26, 2017
9242e38
configure for appveyor
stephenplusplus Jun 26, 2017
a25a18c
old console.log
stephenplusplus Jun 26, 2017
175e6f9
use raw response payload as error message
stephenplusplus Jun 27, 2017
121ce05
elaborate on the requester pays enabling documentation
stephenplusplus Jun 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 28 additions & 13 deletions packages/storage/src/file.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,8 @@ File.prototype.createReadStream = function(options) {
var crc32c = true;
var md5 = false;

var refreshedMetadata = false;

if (is.string(options.validation)) {
options.validation = options.validation.toLowerCase();
crc32c = options.validation === 'crc32c';
Expand All @@ -470,13 +472,11 @@ File.prototype.createReadStream = function(options) {
// returned to the user.
function makeRequest() {
var reqOpts = {
uri: format('{downloadBaseUrl}/{bucketName}/{fileName}', {
downloadBaseUrl: STORAGE_DOWNLOAD_BASE_URL,
bucketName: self.bucket.name,
fileName: encodeURIComponent(self.name)
}),
uri: '',

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

gzip: true,
qs: {}
qs: {
alt: 'media'
}
};

if (self.generation) {
Expand Down Expand Up @@ -509,6 +509,13 @@ File.prototype.createReadStream = function(options) {
function onResponse(err, body, res) {
if (err) {
requestStream.unpipe(throughStream);

// Get error message from the body.
res.pipe(concat(function(body) {
err.message = body.toString();
throughStream.destroy(err);
}));

return;
}

Expand All @@ -525,21 +532,29 @@ File.prototype.createReadStream = function(options) {
// This is hooked to the `complete` event from the request stream. This is
// our chance to validate the data and let the user know if anything went
// wrong.
function onComplete(err, body, res) {
function onComplete(err) {
if (err) {
throughStream.destroy(err);
return;
}

if (rangeRequest) {
return;
}

var hashes = {};
res.headers['x-goog-hash'].split(',').forEach(function(hash) {
var hashType = hash.split('=')[0].trim();
hashes[hashType] = hash.substr(hash.indexOf('=') + 1);
});
if (!refreshedMetadata) {
refreshedMetadata = true;

self.getMetadata(function() {
onComplete(err);
});

return;
}

var hashes = {
crc32c: self.metadata.crc32c,
md5: self.metadata.md5Hash
};

// If we're doing validation, assume the worst-- a data integrity
// mismatch. If not, these tests won't be performed, and we can assume the
Expand Down
9 changes: 6 additions & 3 deletions packages/storage/system-test/storage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -904,7 +904,7 @@ describe('storage', function() {
file.copy('new-file.txt', options, done);
}));

it.only('file#createReadStream', doubleTest(function(options, done) {
it('file#createReadStream', doubleTest(function(options, done) {
file.createReadStream(options)
.on('error', done)
.on('end', done)
Expand All @@ -915,7 +915,7 @@ describe('storage', function() {
file.createResumableUpload(options, done);
}));

it.skip('file#download', doubleTest(function(options, done) {
it('file#download', doubleTest(function(options, done) {
file.download(options, done);
}));

Expand Down Expand Up @@ -1256,7 +1256,10 @@ describe('storage', function() {

it('should not download from the unencrypted file', function(done) {
unencryptedFile.download(function(err) {
assert.strictEqual(err.message, 'Bad Request');
assert.strictEqual(err.message, [
'The target object is encrypted by a',
'customer-supplied encryption key.'
].join(' '));
done();
});
});
Expand Down
127 changes: 79 additions & 48 deletions packages/storage/test/file.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,12 @@ describe('File', function() {
return FakeFailedRequest;
}

beforeEach(function() {
requestOverride = function() {
return through();
};
});

it('should throw if both a range and validation is given', function() {
assert.throws(function() {
file.createReadStream({
Expand Down Expand Up @@ -606,14 +612,14 @@ describe('File', function() {

describe('authenticating', function() {
it('should create an authenticated request', function(done) {
var expectedPath = format('https://{host}/{b}/{o}', {
host: 'storage.googleapis.com',
b: file.bucket.name,
o: encodeURIComponent(file.name)
});

file.requestStream = function(opts) {
assert.equal(opts.uri, expectedPath);
assert.deepEqual(opts, {
uri: '',
gzip: true,
qs: {
alt: 'media'
}
});
setImmediate(function() {
done();
});
Expand All @@ -640,7 +646,7 @@ describe('File', function() {

beforeEach(function() {
file.requestStream = function(opts) {
var stream = (requestOverride || request)(opts);
var stream = requestOverride(opts);

setImmediate(function() {
stream.emit('error', ERROR);
Expand Down Expand Up @@ -711,6 +717,7 @@ describe('File', function() {
});

it('should unpipe stream from an error on the response', function(done) {
var rawResponseStream = through();
var requestStream = through();
var readStream = file.createReadStream();

Expand All @@ -728,7 +735,7 @@ describe('File', function() {
assert.strictEqual(requestStream._readableState.pipes, readStream);

// Triggers the unpipe.
callback(new Error());
callback(new Error(), null, rawResponseStream);

setImmediate(function() {
assert.strictEqual(requestStream._readableState.pipesCount, 0);
Expand Down Expand Up @@ -776,40 +783,65 @@ describe('File', function() {
})
.resume();
});

it('should parse a response stream for a better error', function(done) {
var rawResponsePayload = 'error message from body';
var rawResponseStream = through();
var requestStream = through();

handleRespOverride = function(err, res, body, callback) {
callback(ERROR, null, res);

setImmediate(function() {
rawResponseStream.end(rawResponsePayload);
});
};

file.requestStream = function() {
setImmediate(function() {
requestStream.emit('response', rawResponseStream);
});
return requestStream;
};

file.createReadStream()
.once('error', function(err) {
assert.strictEqual(err, ERROR);
assert.strictEqual(err.message, rawResponsePayload);
done();
})
.resume();
});
});
});

describe('validation', function() {
var data = 'test';

var fakeResponse = {
crc32c: {
headers: { 'x-goog-hash': 'crc32c=####wA==' }
},
md5: {
headers: { 'x-goog-hash': 'md5=CY9rzUYh03PK3k6DJie09g==' }
}
};

beforeEach(function() {
file.metadata.mediaLink = 'http://uri';

file.request = function(opts, cb) {
if (cb) {
(cb.onAuthenticated || cb)(null, {});
} else {
return (requestOverride || requestCached)(opts);
}
file.getMetadata = function(callback) {
file.metadata = {
crc32c: '####wA==',
md5Hash: 'CY9rzUYh03PK3k6DJie09g=='
};
callback();
};
});

it('should destroy the stream on error', function(done) {
var rawResponseStream = through();
var error = new Error('Error.');

file.requestStream = getFakeSuccessfulRequest('data');

handleRespOverride = function(err, resp, body, callback) {
callback(error);
callback(error, null, rawResponseStream);

setImmediate(function() {
rawResponseStream.end();
});
};

file.createReadStream({ validation: 'crc32c' })
Expand All @@ -826,8 +858,7 @@ describe('File', function() {
});

it('should validate with crc32c', function(done) {
file.requestStream =
getFakeSuccessfulRequest(data, fakeResponse.crc32c);
file.requestStream = getFakeSuccessfulRequest(data, {});

file.createReadStream({ validation: 'crc32c' })
.on('error', done)
Expand All @@ -836,10 +867,7 @@ describe('File', function() {
});

it('should emit an error if crc32c validation fails', function(done) {
file.requestStream = getFakeSuccessfulRequest(
'bad-data',
fakeResponse.crc32c
);
file.requestStream = getFakeSuccessfulRequest('bad-data', {});

file.createReadStream({ validation: 'crc32c' })
.on('error', function(err) {
Expand All @@ -850,7 +878,7 @@ describe('File', function() {
});

it('should validate with md5', function(done) {
file.requestStream = getFakeSuccessfulRequest(data, fakeResponse.md5);
file.requestStream = getFakeSuccessfulRequest(data, {});

file.createReadStream({ validation: 'md5' })
.on('error', done)
Expand All @@ -859,8 +887,7 @@ describe('File', function() {
});

it('should emit an error if md5 validation fails', function(done) {
file.requestStream =
getFakeSuccessfulRequest('bad-data', fakeResponse.md5);
file.requestStream = getFakeSuccessfulRequest('bad-data', {});

file.createReadStream({ validation: 'md5' })
.on('error', function(err) {
Expand All @@ -871,9 +898,14 @@ describe('File', function() {
});

it('should default to crc32c validation', function(done) {
file.requestStream = getFakeSuccessfulRequest(data, {
headers: { 'x-goog-hash': 'crc32c=fakefakefake' }
});
file.getMetadata = function(callback) {
file.metadata = {
crc32c: file.metadata.crc32c
};
callback();
};

file.requestStream = getFakeSuccessfulRequest(data, {});

file.createReadStream()
.on('error', function(err) {
Expand All @@ -884,9 +916,7 @@ describe('File', function() {
});

it('should ignore a data mismatch if validation: false', function(done) {
file.requestStream = getFakeSuccessfulRequest(data, {
headers: { 'x-goog-hash': 'crc32c=fakefakefake' }
});
file.requestStream = getFakeSuccessfulRequest(data, {});

file.createReadStream({ validation: false })
.resume()
Expand All @@ -896,10 +926,7 @@ describe('File', function() {

describe('destroying the through stream', function() {
it('should destroy after failed validation', function(done) {
file.requestStream = getFakeSuccessfulRequest(
'bad-data',
fakeResponse.md5
);
file.requestStream = getFakeSuccessfulRequest('bad-data', {});

var readStream = file.createReadStream({ validation: 'md5' });
readStream.destroy = function(err) {
Expand All @@ -910,10 +937,14 @@ describe('File', function() {
});

it('should destroy if MD5 is requested but absent', function(done) {
file.requestStream = getFakeSuccessfulRequest(
'bad-data',
fakeResponse.crc32c
);
file.getMetadata = function(callback) {
file.metadata = {
crc32c: file.metadata.crc32c
};
callback();
};

file.requestStream = getFakeSuccessfulRequest('bad-data', {});

var readStream = file.createReadStream({ validation: 'md5' });
readStream.destroy = function(err) {
Expand Down