Skip to content

Commit 4884318

Browse files
committed
discovery addDocument JSON support
Fixed addDocument to allow custom filename added addJsonDocument method to make things easier refactoring and lots of tests Fixes #474
1 parent 06f8a0a commit 4884318

3 files changed

Lines changed: 252 additions & 41 deletions

File tree

discovery/v1.js

Lines changed: 69 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ const util = require('util');
2020
const requestFactory = require('../lib/requestwrapper');
2121
const BaseService = require('../lib/base_service');
2222
const pick = require('object.pick');
23+
const isStream = require('isstream');
2324

2425
/**
2526
*
@@ -443,15 +444,46 @@ DiscoveryV1.prototype.getCollectionFields = function(params, callback) {
443444
return requestFactory(parameters, callback);
444445
};
445446

447+
/**
448+
* Ensures a filename is always set because discovery ignores documents that do not have a filename set.
449+
* @param file
450+
* @private
451+
*/
452+
DiscoveryV1._ensureFilename = function(file) {
453+
// no changes needed for streams created by fs.ReadStream (or similar looking streams)
454+
if (isStream.isReadable(file) && file.path) {
455+
return file;
456+
}
457+
458+
// next handle request-style value/options objects
459+
if (file && file.hasOwnProperty('value') && file.hasOwnProperty('options')) {
460+
if (file.options.filename) {
461+
return file;
462+
}
463+
return {
464+
value: file.value,
465+
options: Object.assign({ filename: '_' }, file.options)
466+
};
467+
}
468+
469+
// finally, handle all other cases by wrapping them in a request-style value/options object
470+
return {
471+
value: file,
472+
options: {
473+
filename: '_'
474+
}
475+
};
476+
};
477+
446478
/**
447479
* Add a document to a collection
448-
* @param params
480+
* @param {object} params
449481
* @param {String} params.environment_id environment guid for the collection
450482
* @param {string} params.collection_id the guid of the collection to delete
451-
* @param {Buffer|ReadableStream|Object} params.file a file to post (smaller than 50mb)
483+
* @param {Buffer|ReadableStream|Object} params.file a file to post (smaller than 50mb). In some cases you will need to supply an object with {value: data, options: {contentType: /...'}}
452484
* @param {string} [params.configuration_id] config guid
453-
* @param {object|string} [params.metadata] JSON object with file metadata including content-type (will infer if missing)
454-
* @param callback
485+
* @param {object} [params.metadata] JSON object with file metadata
486+
* @param {function} callback
455487
* @return {ReadableStream|undefined}
456488
*/
457489
DiscoveryV1.prototype.addDocument = function(params, callback) {
@@ -460,18 +492,9 @@ DiscoveryV1.prototype.addDocument = function(params, callback) {
460492
const queryParams = pick(params, ['configuration_id']);
461493
const formDataParams = pick(params, ['file', 'metadata']);
462494

463-
// if we get a buffer or object, we need to include stuff about filename for the service
495+
// Discovery only accepts files that have a filename specified
464496
if (formDataParams.file) {
465-
if (
466-
typeof formDataParams.file.filename !== 'string' &&
467-
!(formDataParams.file.options && typeof formDataParams.file.options.filename !== 'string') &&
468-
!(formDataParams.file.path && typeof formDataParams.file.path !== 'string') &&
469-
!(formDataParams.file.name && typeof formDataParams.file.name !== 'string')
470-
) {
471-
const filedat = formDataParams.file;
472-
// the filename used below is because the name must exist
473-
formDataParams.file = { value: filedat, options: { filename: '_' } };
474-
}
497+
formDataParams.file = DiscoveryV1._ensureFilename(formDataParams.file);
475498
}
476499

477500
if (formDataParams.metadata && typeof formDataParams.metadata === 'object') {
@@ -487,12 +510,35 @@ DiscoveryV1.prototype.addDocument = function(params, callback) {
487510
formData: formDataParams,
488511
json: true
489512
},
490-
requiredParams: ['environment_id', 'collection_id', 'file'],
513+
requiredParams: ['environment_id', 'collection_id'],
491514
defaultOptions: this._options
492515
};
493516
return requestFactory(parameters, callback);
494517
};
495518

519+
/**
520+
* Helper method, similar to addDocument except that the file param expects a JSON object
521+
* @param {object} params
522+
* @param {String} params.environment_id environment guid for the collection
523+
* @param {string} params.collection_id the guid of the collection to delete
524+
* @param {Object} params.file non-stringified JSON object
525+
* @param {string} [params.configuration_id] config guid
526+
* @param {object} [params.metadata] JSON object with file metadata
527+
* @param {function} callback
528+
* @return {ReadableStream|undefined}
529+
*/
530+
DiscoveryV1.prototype.addJsonDocument = function(params, callback) {
531+
params = Object.assign({}, params, {
532+
file: {
533+
value: JSON.stringify(params.file),
534+
options: {
535+
filename: '_.json'
536+
}
537+
}
538+
});
539+
return this.addDocument(params, callback);
540+
};
541+
496542
/**
497543
* Update or partially update a document to create or replace an existing document
498544
* @param params
@@ -501,7 +547,7 @@ DiscoveryV1.prototype.addDocument = function(params, callback) {
501547
* @param {string} params.document_id the guid of the document to update
502548
* @param {Buffer|ReadableStream|Object} params.file a file to post (smaller than 50mb)
503549
* @param {string} [params.configuration_id] config guid
504-
* @param {string} [params.metadata] file metadata, including content-type (will infer if missing)
550+
* @param {object} [params.metadata] file metadata, including content-type (will infer if missing)
505551
* @param callback
506552
* @return {ReadableStream|undefined}
507553
*/
@@ -511,18 +557,13 @@ DiscoveryV1.prototype.updateDocument = function(params, callback) {
511557
const queryParams = pick(params, ['configuration_id']);
512558
const formDataParams = pick(params, ['file', 'metadata']);
513559

514-
// if we get a buffer or object, we need to include stuff about filename for the service
560+
// Discovery only accepts files that have a filename specified
515561
if (formDataParams.file) {
516-
if (
517-
typeof formDataParams.file.filename !== 'string' &&
518-
!(formDataParams.file.options && typeof formDataParams.file.options.filename !== 'string') &&
519-
!(formDataParams.file.path && typeof formDataParams.file.path !== 'string') &&
520-
!(formDataParams.file.name && typeof formDataParams.file.name !== 'string')
521-
) {
522-
const filedat = formDataParams.file;
523-
// the filename used below is because the name must exist
524-
formDataParams.file = { value: filedat, options: { filename: '_' } };
525-
}
562+
formDataParams.file = DiscoveryV1._ensureFilename(formDataParams.file);
563+
}
564+
565+
if (formDataParams.metadata && typeof formDataParams.metadata === 'object') {
566+
formDataParams.metadata = JSON.stringify(formDataParams.metadata);
526567
}
527568

528569
const parameters = {

test/integration/test.discovery.js

Lines changed: 110 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ const authHelper = require('./auth_helper.js');
66
const auth = authHelper.auth;
77
const describe = authHelper.describe; // this runs describe.skip if there is no auth.js file :)
88
const assert = require('assert');
9+
const async = require('async');
10+
const fs = require('fs');
911

1012
const THIRTY_SECONDS = 30000;
1113
const TWO_SECONDS = 2000;
@@ -18,11 +20,18 @@ describe.skip('discovery_integration', function() {
1820
let discovery;
1921
let environment_id; // = auth.discovery.environment_id;
2022
let configuration_id; // = auth.discovery.configuration_id;
21-
// var collection_id;
23+
let collection_id;
2224

2325
before(function() {
2426
nock.enableNetConnect();
25-
discovery = new DiscoveryV1(auth.discovery);
27+
discovery = new DiscoveryV1(
28+
Object.assign({}, auth.discovery, {
29+
version_date: DiscoveryV1.VERSION_DATE_2017_04_27
30+
})
31+
);
32+
environment_id = auth.discovery.environment_id;
33+
configuration_id = auth.discovery.configuration_id;
34+
collection_id = auth.discovery.collection_id;
2635
});
2736

2837
after(function() {
@@ -75,6 +84,7 @@ describe.skip('discovery_integration', function() {
7584
);
7685
});
7786

87+
// todo: delete the collection after the testing is complete
7888
it.skip('should createCollection()', function(done) {
7989
discovery.createCollection(
8090
{
@@ -92,15 +102,102 @@ describe.skip('discovery_integration', function() {
92102
}
93103
);
94104
});
95-
// it('should getCollections()', function(done) {
96-
//
97-
// });
98-
//
99-
// it('should getCollection()', function(done) {
100-
//
101-
// });
102-
//
103-
// it('should deleteCollection()', function(done) {
104-
//
105-
// });
105+
106+
it('getCollections()', function(done) {
107+
discovery.getCollections(
108+
{
109+
environment_id: environment_id,
110+
configuration_id: configuration_id
111+
},
112+
function(err, res) {
113+
assert.ifError(err);
114+
assert(res);
115+
// console.log(res);
116+
assert(Array.isArray(res.collections));
117+
done(err);
118+
}
119+
);
120+
});
121+
122+
describe('add-query-delete', function() {
123+
it('addDocument()', function(done) {
124+
const document_obj = {
125+
environment_id: environment_id,
126+
collection_id: collection_id,
127+
file: fs.createReadStream(__dirname + '../resources/sample-docx.docx')
128+
};
129+
130+
discovery.addDocument(document_obj, function(err, response) {
131+
assert.ifError(err);
132+
assert(response.document_id);
133+
done(err);
134+
});
135+
});
136+
137+
it('addJsonDocument()', function(done) {
138+
const document_obj = {
139+
environment_id: environment_id,
140+
collection_id: collection_id,
141+
file: {
142+
foo: 'bar',
143+
from: 'node-sdk integration test',
144+
test_date: new Date().toString()
145+
}
146+
};
147+
148+
discovery.addJsonDocument(document_obj, function(err, response) {
149+
assert.ifError(err);
150+
assert(response.document_id);
151+
done(err);
152+
});
153+
});
154+
155+
it('query()', function(done) {
156+
discovery.query(
157+
{
158+
environment_id: environment_id,
159+
collection_id: collection_id,
160+
query: ''
161+
},
162+
function(err, res) {
163+
assert.ifError(err);
164+
assert(res);
165+
assert.equal(typeof res.matching_results, 'number');
166+
assert(Array.isArray(res.results));
167+
done(err);
168+
}
169+
);
170+
});
171+
172+
it('delete all documents', function(done) {
173+
discovery.query(
174+
{
175+
environment_id: environment_id,
176+
collection_id: collection_id,
177+
query: ''
178+
},
179+
function(err, res) {
180+
assert.ifError(err);
181+
async.eachSeries(
182+
res.results,
183+
function(doc, next) {
184+
// console.log('deleting ', doc);
185+
discovery.deleteDocument(
186+
{
187+
environment_id: environment_id,
188+
collection_id: collection_id,
189+
document_id: doc.id
190+
},
191+
function(err, res) {
192+
// console.log('deleted', err, res);
193+
next(err);
194+
}
195+
);
196+
},
197+
done
198+
);
199+
}
200+
);
201+
});
202+
});
106203
});

test/unit/test.discovery.v1.js

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const assert = require('assert');
44
const DiscoveryV1 = require('../../discovery/v1');
55
const fs = require('fs');
66
const path = require('path');
7+
const stream = require('stream');
78

89
const nock = require('nock');
910

@@ -364,6 +365,78 @@ describe('discovery-v1', function() {
364365

365366
return result;
366367
}
368+
369+
describe('_ensureFilename()', function() {
370+
it('should pass through ReadStreams unmodified', function() {
371+
const src = fs.createReadStream(__dirname + '../resources/sample-docx.docx');
372+
assert.equal(DiscoveryV1._ensureFilename(src), src);
373+
});
374+
375+
it('should pass through value/options objects with a filename', function() {
376+
const src = {
377+
value: 'foo',
378+
options: {
379+
filename: 'foo.bar'
380+
}
381+
};
382+
const actual = DiscoveryV1._ensureFilename(src);
383+
assert.equal(actual, src);
384+
assert.deepEqual(actual, {
385+
value: 'foo',
386+
options: {
387+
filename: 'foo.bar'
388+
}
389+
});
390+
});
391+
392+
it('should create new object/values with a filename when missing', function() {
393+
const src = {
394+
value: '{"foo": "bar"}',
395+
options: {
396+
contentType: 'application/json'
397+
}
398+
};
399+
const actual = DiscoveryV1._ensureFilename(src);
400+
assert.deepEqual(actual, {
401+
value: '{"foo": "bar"}',
402+
options: {
403+
contentType: 'application/json',
404+
filename: '_'
405+
}
406+
});
407+
assert.notEqual(actual, src, 'it should be a new object, not a modification of the existing one');
408+
});
409+
410+
it('should wrap buffers', function() {
411+
const src = Buffer.from([1, 2, 3, 4]);
412+
assert.deepEqual(DiscoveryV1._ensureFilename(src), {
413+
value: src,
414+
options: {
415+
filename: '_'
416+
}
417+
});
418+
});
419+
420+
it('should wrap strings', function() {
421+
const src = 'foo';
422+
assert.deepEqual(DiscoveryV1._ensureFilename(src), {
423+
value: src,
424+
options: {
425+
filename: '_'
426+
}
427+
});
428+
});
429+
430+
it('should wrap streams', function() {
431+
const src = new stream.Readable();
432+
assert.deepEqual(DiscoveryV1._ensureFilename(src), {
433+
value: src,
434+
options: {
435+
filename: '_'
436+
}
437+
});
438+
});
439+
}); // end of _ensureFilename()
367440
});
368441
});
369442
});

0 commit comments

Comments
 (0)