Skip to content

Commit 2a941f7

Browse files
muraliQlogicGautamSharda
muraliQlogic
authored and
GautamSharda
committed
Issue 278 Fix (#315)
* Fix for Issue278 * revert * Issue#278 fix * converted createPrefixRange_ to public method
1 parent 44911a3 commit 2a941f7

2 files changed

Lines changed: 120 additions & 15 deletions

File tree

‎handwritten/bigtable/src/table.js‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,11 @@ Please use the format 'prezzy' or '${instance.name}/tables/prezzy'.`
103103
* @returns {object} range
104104
*
105105
* @example
106-
* Table.createPrefixRange_('start');
106+
* const Bigtable = require('@google-cloud/bigtable');
107+
* const bigtable = new Bigtable();
108+
* const instance = bigtable.instance('my-instance');
109+
* const table = instance.table('prezzy');
110+
* table.createPrefixRange('start');
107111
* // => {
108112
* // start: 'start',
109113
* // end: {
@@ -112,7 +116,7 @@ Please use the format 'prezzy' or '${instance.name}/tables/prezzy'.`
112116
* // }
113117
* // }
114118
*/
115-
static createPrefixRange_(start) {
119+
createPrefixRange(start) {
116120
const prefix = start.replace(new RegExp('[\xff]+$'), '');
117121
let endKey = '';
118122

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

403407
if (options.start || options.end) {
408+
if (options.ranges || options.prefix || options.prefixes) {
409+
throw new Error(
410+
'start/end should be used exclusively to ranges/prefix/prefixes.'
411+
);
412+
}
404413
ranges.push({
405414
start: options.start,
406415
end: options.end,
@@ -412,12 +421,22 @@ Please use the format 'prezzy' or '${instance.name}/tables/prezzy'.`
412421
}
413422

414423
if (options.prefix) {
415-
ranges.push(Table.createPrefixRange_(options.prefix));
424+
if (options.ranges || options.start || options.end || options.prefixes) {
425+
throw new Error(
426+
'prefix should be used exclusively to ranges/start/end/prefixes.'
427+
);
428+
}
429+
ranges.push(this.createPrefixRange(options.prefix));
416430
}
417431

418432
if (options.prefixes) {
433+
if (options.ranges || options.start || options.end || options.prefix) {
434+
throw new Error(
435+
'prefixes should be used exclusively to ranges/start/end/prefix.'
436+
);
437+
}
419438
options.prefixes.forEach(prefix => {
420-
ranges.push(Table.createPrefixRange_(prefix));
439+
ranges.push(this.createPrefixRange(prefix));
421440
});
422441
}
423442

‎handwritten/bigtable/test/table.js‎

Lines changed: 97 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -214,41 +214,41 @@ describe('Bigtable/Table', function() {
214214
});
215215
});
216216

217-
describe('createPrefixRange_', function() {
217+
describe('createPrefixRange', function() {
218218
it('should create a range from the prefix', function() {
219-
assert.deepStrictEqual(Table.createPrefixRange_('start'), {
219+
assert.deepStrictEqual(table.createPrefixRange('start'), {
220220
start: 'start',
221221
end: {
222222
value: 'staru',
223223
inclusive: false,
224224
},
225225
});
226226

227-
assert.deepStrictEqual(Table.createPrefixRange_('X\xff'), {
227+
assert.deepStrictEqual(table.createPrefixRange('X\xff'), {
228228
start: 'X\xff',
229229
end: {
230230
value: 'Y',
231231
inclusive: false,
232232
},
233233
});
234234

235-
assert.deepStrictEqual(Table.createPrefixRange_('xoo\xff'), {
235+
assert.deepStrictEqual(table.createPrefixRange('xoo\xff'), {
236236
start: 'xoo\xff',
237237
end: {
238238
value: 'xop',
239239
inclusive: false,
240240
},
241241
});
242242

243-
assert.deepStrictEqual(Table.createPrefixRange_('a\xffb'), {
243+
assert.deepStrictEqual(table.createPrefixRange('a\xffb'), {
244244
start: 'a\xffb',
245245
end: {
246246
value: 'a\xffc',
247247
inclusive: false,
248248
},
249249
});
250250

251-
assert.deepStrictEqual(Table.createPrefixRange_('com.google.'), {
251+
assert.deepStrictEqual(table.createPrefixRange('com.google.'), {
252252
start: 'com.google.',
253253
end: {
254254
value: 'com.google/',
@@ -258,15 +258,15 @@ describe('Bigtable/Table', function() {
258258
});
259259

260260
it('should create an inclusive bound when the prefix is empty', function() {
261-
assert.deepStrictEqual(Table.createPrefixRange_('\xff'), {
261+
assert.deepStrictEqual(table.createPrefixRange('\xff'), {
262262
start: '\xff',
263263
end: {
264264
value: '',
265265
inclusive: true,
266266
},
267267
});
268268

269-
assert.deepStrictEqual(Table.createPrefixRange_(''), {
269+
assert.deepStrictEqual(table.createPrefixRange(''), {
270270
start: '',
271271
end: {
272272
value: '',
@@ -561,13 +561,99 @@ describe('Bigtable/Table', function() {
561561
table.createReadStream(options);
562562
});
563563

564+
it('should throw if ranges and start is set together', function() {
565+
const options = {
566+
ranges: [
567+
{
568+
start: 'a',
569+
end: 'b',
570+
},
571+
{
572+
start: 'c',
573+
end: 'd',
574+
},
575+
],
576+
start: 'a',
577+
};
578+
assert.throws(function() {
579+
table.createReadStream(options, assert.ifError);
580+
}, /start\/end should be used exclusively to ranges\/prefix\/prefixes\./);
581+
});
582+
583+
it('should throw if ranges and end is set together', function() {
584+
const options = {
585+
ranges: [
586+
{
587+
start: 'a',
588+
end: 'b',
589+
},
590+
{
591+
start: 'c',
592+
end: 'd',
593+
},
594+
],
595+
end: 'a',
596+
};
597+
assert.throws(function() {
598+
table.createReadStream(options, assert.ifError);
599+
}, /start\/end should be used exclusively to ranges\/prefix\/prefixes\./);
600+
});
601+
602+
it('should throw if ranges and prefix is set together', function() {
603+
const options = {
604+
ranges: [
605+
{
606+
start: 'a',
607+
end: 'b',
608+
},
609+
{
610+
start: 'c',
611+
end: 'd',
612+
},
613+
],
614+
prefix: 'a',
615+
};
616+
assert.throws(function() {
617+
table.createReadStream(options, assert.ifError);
618+
}, /prefix should be used exclusively to ranges\/start\/end\/prefixes\./);
619+
});
620+
621+
it('should throw if ranges and prefixes is set together', function() {
622+
const options = {
623+
ranges: [
624+
{
625+
start: 'a',
626+
end: 'b',
627+
},
628+
{
629+
start: 'c',
630+
end: 'd',
631+
},
632+
],
633+
prefixes: [{prefix: 'a'}],
634+
};
635+
assert.throws(function() {
636+
table.createReadStream(options, assert.ifError);
637+
}, /prefixes should be used exclusively to ranges\/start\/end\/prefix\./);
638+
});
639+
640+
it('should throw if prefix and start is set together', function() {
641+
const options = {
642+
start: 'a',
643+
prefix: 'a',
644+
};
645+
assert.throws(function() {
646+
table.createReadStream(options, assert.ifError);
647+
}, /start\/end should be used exclusively to ranges\/prefix\/prefixes\./);
648+
});
649+
564650
describe('prefixes', function() {
565651
beforeEach(function() {
566652
FakeFilter.createRange = common.util.noop;
567653
});
568654

569655
afterEach(function() {
570-
Table.createPrefixRange_.restore();
656+
table.createPrefixRange.restore();
571657
});
572658

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

582668
const prefixSpy = sinon
583-
.stub(Table, 'createPrefixRange_')
669+
.stub(table, 'createPrefixRange')
584670
.callsFake(function() {
585671
return fakePrefixRange;
586672
});
@@ -614,7 +700,7 @@ describe('Bigtable/Table', function() {
614700
{start: 'def', end: 'deg'},
615701
];
616702
const prefixSpy = sinon
617-
.stub(Table, 'createPrefixRange_')
703+
.stub(table, 'createPrefixRange')
618704
.callsFake(function() {
619705
const callIndex = prefixSpy.callCount - 1;
620706
return prefixRanges[callIndex];

0 commit comments

Comments
 (0)
⚡