Skip to content

Commit

Permalink
Issue 278 Fix (googleapis#315)
Browse files Browse the repository at this point in the history 
* Fix for Issue278

* revert

* Issue#278 fix

* converted createPrefixRange_ to public method
  • Loading branch information
@muraliQlogic @sduskis
muraliQlogic authored and sduskis committed Oct 5, 2018
1 parent d6cbf8f commit 6fb4ca7
Show file tree
Hide file tree
Showing 2 changed files with 120 additions and 15 deletions.
27 changes: 23 additions & 4 deletions src/table.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,11 @@ Please use the format 'prezzy' or '${instance.name}/tables/prezzy'.`
* @returns {object} range
*
* @example
* Table.createPrefixRange_('start');
* const Bigtable = require('@google-cloud/bigtable');
* const bigtable = new Bigtable();
* const instance = bigtable.instance('my-instance');
* const table = instance.table('prezzy');
* table.createPrefixRange('start');
* // => {
* // start: 'start',
* // end: {
Expand All @@ -112,7 +116,7 @@ Please use the format 'prezzy' or '${instance.name}/tables/prezzy'.`
* // }
* // }
*/
static createPrefixRange_(start) {
createPrefixRange(start) {
const prefix = start.replace(new RegExp('[\xff]+$'), '');
let endKey = '';

Expand Down Expand Up @@ -401,6 +405,11 @@ Please use the format 'prezzy' or '${instance.name}/tables/prezzy'.`
let numRequestsMade = 0;

if (options.start || options.end) {
if (options.ranges || options.prefix || options.prefixes) {
throw new Error(
'start/end should be used exclusively to ranges/prefix/prefixes.'
);
}
ranges.push({
start: options.start,
end: options.end,
Expand All @@ -412,12 +421,22 @@ Please use the format 'prezzy' or '${instance.name}/tables/prezzy'.`
}

if (options.prefix) {
ranges.push(Table.createPrefixRange_(options.prefix));
if (options.ranges || options.start || options.end || options.prefixes) {
throw new Error(
'prefix should be used exclusively to ranges/start/end/prefixes.'
);
}
ranges.push(this.createPrefixRange(options.prefix));
}

if (options.prefixes) {
if (options.ranges || options.start || options.end || options.prefix) {
throw new Error(
'prefixes should be used exclusively to ranges/start/end/prefix.'
);
}
options.prefixes.forEach(prefix => {
ranges.push(Table.createPrefixRange_(prefix));
ranges.push(this.createPrefixRange(prefix));
});
}

Expand Down
108 changes: 97 additions & 11 deletions test/table.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,41 +214,41 @@ describe('Bigtable/Table', function() {
});
});

describe('createPrefixRange_', function() {
describe('createPrefixRange', function() {
it('should create a range from the prefix', function() {
assert.deepStrictEqual(Table.createPrefixRange_('start'), {
assert.deepStrictEqual(table.createPrefixRange('start'), {
start: 'start',
end: {
value: 'staru',
inclusive: false,
},
});

assert.deepStrictEqual(Table.createPrefixRange_('X\xff'), {
assert.deepStrictEqual(table.createPrefixRange('X\xff'), {
start: 'X\xff',
end: {
value: 'Y',
inclusive: false,
},
});

assert.deepStrictEqual(Table.createPrefixRange_('xoo\xff'), {
assert.deepStrictEqual(table.createPrefixRange('xoo\xff'), {
start: 'xoo\xff',
end: {
value: 'xop',
inclusive: false,
},
});

assert.deepStrictEqual(Table.createPrefixRange_('a\xffb'), {
assert.deepStrictEqual(table.createPrefixRange('a\xffb'), {
start: 'a\xffb',
end: {
value: 'a\xffc',
inclusive: false,
},
});

assert.deepStrictEqual(Table.createPrefixRange_('com.google.'), {
assert.deepStrictEqual(table.createPrefixRange('com.google.'), {
start: 'com.google.',
end: {
value: 'com.google/',
Expand All @@ -258,15 +258,15 @@ describe('Bigtable/Table', function() {
});

it('should create an inclusive bound when the prefix is empty', function() {
assert.deepStrictEqual(Table.createPrefixRange_('\xff'), {
assert.deepStrictEqual(table.createPrefixRange('\xff'), {
start: '\xff',
end: {
value: '',
inclusive: true,
},
});

assert.deepStrictEqual(Table.createPrefixRange_(''), {
assert.deepStrictEqual(table.createPrefixRange(''), {
start: '',
end: {
value: '',
Expand Down Expand Up @@ -561,13 +561,99 @@ describe('Bigtable/Table', function() {
table.createReadStream(options);
});

it('should throw if ranges and start is set together', function() {
const options = {
ranges: [
{
start: 'a',
end: 'b',
},
{
start: 'c',
end: 'd',
},
],
start: 'a',
};
assert.throws(function() {
table.createReadStream(options, assert.ifError);
}, /start\/end should be used exclusively to ranges\/prefix\/prefixes\./);
});

it('should throw if ranges and end is set together', function() {
const options = {
ranges: [
{
start: 'a',
end: 'b',
},
{
start: 'c',
end: 'd',
},
],
end: 'a',
};
assert.throws(function() {
table.createReadStream(options, assert.ifError);
}, /start\/end should be used exclusively to ranges\/prefix\/prefixes\./);
});

it('should throw if ranges and prefix is set together', function() {
const options = {
ranges: [
{
start: 'a',
end: 'b',
},
{
start: 'c',
end: 'd',
},
],
prefix: 'a',
};
assert.throws(function() {
table.createReadStream(options, assert.ifError);
}, /prefix should be used exclusively to ranges\/start\/end\/prefixes\./);
});

it('should throw if ranges and prefixes is set together', function() {
const options = {
ranges: [
{
start: 'a',
end: 'b',
},
{
start: 'c',
end: 'd',
},
],
prefixes: [{prefix: 'a'}],
};
assert.throws(function() {
table.createReadStream(options, assert.ifError);
}, /prefixes should be used exclusively to ranges\/start\/end\/prefix\./);
});

it('should throw if prefix and start is set together', function() {
const options = {
start: 'a',
prefix: 'a',
};
assert.throws(function() {
table.createReadStream(options, assert.ifError);
}, /start\/end should be used exclusively to ranges\/prefix\/prefixes\./);
});

describe('prefixes', function() {
beforeEach(function() {
FakeFilter.createRange = common.util.noop;
});

afterEach(function() {
Table.createPrefixRange_.restore();
table.createPrefixRange.restore();
});

it('should transform the prefix into a range', function(done) {
Expand All @@ -580,7 +666,7 @@ describe('Bigtable/Table', function() {
const fakePrefix = 'abc';

const prefixSpy = sinon
.stub(Table, 'createPrefixRange_')
.stub(table, 'createPrefixRange')
.callsFake(function() {
return fakePrefixRange;
});
Expand Down Expand Up @@ -614,7 +700,7 @@ describe('Bigtable/Table', function() {
{start: 'def', end: 'deg'},
];
const prefixSpy = sinon
.stub(Table, 'createPrefixRange_')
.stub(table, 'createPrefixRange')
.callsFake(function() {
const callIndex = prefixSpy.callCount - 1;
return prefixRanges[callIndex];
Expand Down

0 comments on commit 6fb4ca7

Please sign in to comment.