Skip to content

Commit

Permalink
SmartiProxy: Use params option instead building HTTP query strings (R…
Browse files Browse the repository at this point in the history 
…ocketChat#529)

* using params option instead building complex URLs
* correct js docs
* removed unused code
  • Loading branch information
@ruKurz @mrsimpson
ruKurz authored and mrsimpson committed Sep 28, 2018
1 parent 61398e0 commit b136c5c
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 35 deletions.
11 changes: 8 additions & 3 deletions packages/assistify-ai/server/SmartiProxy.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,25 @@ export class SmartiProxy {
*
* @param {String} method - the HTTP method to use
* @param {String} path - the path to call
* @param {Object} [body=null] - the payload to pass (optional)
* @param {Object} [parameters=null] - the http query params (optional)
* @param {String} [body=null] - the payload to pass (optional)
* @param {Function} onError=null - custom error handler
*
* @returns {Object}
*/
static propagateToSmarti(method, path, body = null, onError = null) {
static propagateToSmarti(method, path, parameters = null, body = null, onError = null) {
const url = `${ SmartiProxy.smartiUrl }${ path }`;
const header = {
'X-Auth-Token': SmartiProxy.smartiAuthToken,
'Content-Type': 'application/json; charset=utf-8'
};
try {
SystemLogger.debug('Sending request to Smarti', method, 'to', url, 'body', JSON.stringify(body));
const response = HTTP.call(method, url, {data: body, headers: header});
const response = HTTP.call(method, url, {
params: parameters,
data: body,
headers: header
});
if (response.statusCode < 400) {
return response.data || response.content; //.data if it's a json-response
} else {
Expand Down
18 changes: 9 additions & 9 deletions packages/assistify-ai/server/lib/SmartiAdapter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,17 +92,17 @@ export class SmartiAdapter {
if (message.editedAt) {
SystemLogger.debug('Trying to update existing message...');
// update existing message
request_result = SmartiProxy.propagateToSmarti(verbs.put, `conversation/${ conversationId }/message/${ requestBodyMessage.id }`, requestBodyMessage, (error) => {
request_result = SmartiProxy.propagateToSmarti(verbs.put, `conversation/${ conversationId }/message/${ requestBodyMessage.id }`, null, requestBodyMessage, (error) => {
// 404 is expected if message doesn't exist
if (!error.response || error.response.statusCode === 404) {
SystemLogger.debug('Message not found!');
SystemLogger.debug('Adding new message to conversation...');
request_result = SmartiProxy.propagateToSmarti(verbs.post, `conversation/${ conversationId }/message`, requestBodyMessage);
request_result = SmartiProxy.propagateToSmarti(verbs.post, `conversation/${ conversationId }/message`, null, requestBodyMessage);
}
});
} else {
SystemLogger.debug('Adding new message to conversation...');
request_result = SmartiProxy.propagateToSmarti(verbs.post, `conversation/${ conversationId }/message`, requestBodyMessage);
request_result = SmartiProxy.propagateToSmarti(verbs.post, `conversation/${ conversationId }/message`, null, requestBodyMessage);
}

if (request_result) {
Expand Down Expand Up @@ -150,7 +150,7 @@ export class SmartiAdapter {
const conversationId = SmartiAdapter.getConversationId(room._id);

if (conversationId) {
const res = SmartiProxy.propagateToSmarti(verbs.put, `/conversation/${ conversationId }/meta.status`, 'Complete');
const res = SmartiProxy.propagateToSmarti(verbs.put, `/conversation/${ conversationId }/meta.status`, null, 'Complete');
if (!res) {
Meteor.defer(() => SmartiAdapter._markRoomAsUnsynced(room._id));
}
Expand Down Expand Up @@ -188,7 +188,7 @@ export class SmartiAdapter {
let conversationId = null;
// uncached conversation
SystemLogger.debug('Trying Smarti legacy service to retrieve conversation...');
const conversation = SmartiProxy.propagateToSmarti(verbs.get, `legacy/rocket.chat?channel_id=${ roomId }`, null, (error) => {
const conversation = SmartiProxy.propagateToSmarti(verbs.get, `legacy/rocket.chat?channel_id=${ roomId }`, null, null, (error) => {
// 404 is expected if no mapping exists in Smarti
if (error.response.statusCode === 404) {
SystemLogger.warn(`No Smarti conversationId found (Server Error 404) for room: ${ roomId }`);
Expand Down Expand Up @@ -237,7 +237,7 @@ export class SmartiAdapter {

// conversation updated or created => request analysis results
SystemLogger.debug(`Smarti - conversation updated or created -> get analysis result asynch [ callback=${ SmartiAdapter.rocketWebhookUrl } ] for conversation: ${ conversationId } and room: ${ roomId }`);
SmartiProxy.propagateToSmarti(verbs.get, `conversation/${ conversationId }/analysis?callback=${ SmartiAdapter.rocketWebhookUrl }`); // asynch
SmartiProxy.propagateToSmarti(verbs.get, `conversation/${ conversationId }/analysis`, { callback: SmartiAdapter.rocketWebhookUrl }); // asynch
}

/**
Expand Down Expand Up @@ -376,7 +376,7 @@ export class SmartiAdapter {
const conversationId = SmartiAdapter.getConversationId(room._id);
if (conversationId) {
SystemLogger.debug(`Conversation found ${ conversationId } - delete and create new conversation`);
SmartiProxy.propagateToSmarti(verbs.delete, `conversation/${ conversationId }`, null);
SmartiProxy.propagateToSmarti(verbs.delete, `conversation/${ conversationId }`);
}

// get the messages of the room and create a conversation from it
Expand Down Expand Up @@ -456,7 +456,7 @@ export class SmartiAdapter {
}

// post the conversation
const conversation = SmartiProxy.propagateToSmarti(verbs.post, 'conversation', conversationBody, (error) => {
const conversation = SmartiProxy.propagateToSmarti(verbs.post, 'conversation', null, conversationBody, (error) => {
SystemLogger.error(`Smarti - unexpected server error: ${ JSON.stringify(error, null, 2) } occured when creating a new conversation: ${ JSON.stringify(conversationBody, null, 2) }`);
});
if (!conversation && !conversation.id) {
Expand Down Expand Up @@ -563,7 +563,7 @@ export class SmartiAdapter {

static _smartiAvailable() {
// if Smarti is not available stop immediately
const resp = SmartiProxy.propagateToSmarti(verbs.get, 'system/health', null, (error) => {
const resp = SmartiProxy.propagateToSmarti(verbs.get, 'system/health', null, null, (error) => {
if (error.statusCode !== 200) {
const e = new Meteor.Error('Smarti not reachable!');
SystemLogger.error('Stop synchronizing with Smarti immediately:', e);
Expand Down
38 changes: 15 additions & 23 deletions packages/assistify-ai/server/methods/SmartiWidgetBackend.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ import {SystemLogger} from 'meteor/rocketchat:logger';
import {SmartiProxy, verbs} from '../SmartiProxy';
import {SmartiAdapter} from '../lib/SmartiAdapter';

const querystring = require('querystring');

/** @namespace RocketChat.RateLimiter.limitFunction */

/**
Expand Down Expand Up @@ -44,7 +42,7 @@ Meteor.methods({
return !RocketChat.authz.hasPermission(userId, 'send-many-messages');
}
}
)(verbs.get, `conversation/${ conversationId }/analysis`, null, (error) => {
)(verbs.get, `conversation/${ conversationId }/analysis`, null, null, (error) => {
// 404 is expected if no mapping exists
if (error.response && error.response.statusCode === 404) {
return null;
Expand All @@ -70,40 +68,34 @@ Meteor.methods({
return !RocketChat.authz.hasPermission(userId, 'send-many-messages');
}
}
)(verbs.get, `conversation/${ conversationId }/analysis/template/${ templateIndex }/result/${ creator }?start=${ start }&rows=${ rows }`);
)(verbs.get, `conversation/${ conversationId }/analysis/template/${ templateIndex }/result/${ creator }`, { start, rows });
},

searchConversations(queryParams) {

const _getAclQuery = function() {

function unique(value, index, array) {
return array.indexOf(value) === index;
}
function unique(value, index, array) {
return array.indexOf(value) === index;
}

const solrFilterBooleanLimit = 256; // there is a limit for boolean expressinos in a filter query of default 1024 and an additional limiter by the HTTP-server. Experiments showed this limit as magic number.
const findOptions = { limit: solrFilterBooleanLimit, sort: { ts: -1 }, fields: { _id: 1 } };
const subscribedRooms = RocketChat.models.Subscriptions.find({'u._id': Meteor.userId()}, { limit: solrFilterBooleanLimit, sort: { ts: -1 }, fields: { rid: 1 } }).fetch().map(subscription => subscription.rid);
const publicChannels = RocketChat.authz.hasPermission(Meteor.userId(), 'view-c-room') ? RocketChat.models.Rooms.find({t: 'c'}, findOptions).fetch().map(room => room._id) : [];
const livechats = RocketChat.authz.hasPermission(Meteor.userId(), 'view-l-room') ? RocketChat.models.Rooms.find({t: 'l'}, findOptions).fetch().map(room => room._id) : [];
const solrFilterBooleanLimit = 256; // there is a limit for boolean expressinos in a filter query of default 1024 and an additional limiter by the HTTP-server. Experiments showed this limit as magic number.
const findOptions = { limit: solrFilterBooleanLimit, sort: { ts: -1 }, fields: { _id: 1 } };
const subscribedRooms = RocketChat.models.Subscriptions.find({'u._id': Meteor.userId()}, { limit: solrFilterBooleanLimit, sort: { ts: -1 }, fields: { rid: 1 } }).fetch().map(subscription => subscription.rid);
const publicChannels = RocketChat.authz.hasPermission(Meteor.userId(), 'view-c-room') ? RocketChat.models.Rooms.find({t: 'c'}, findOptions).fetch().map(room => room._id) : [];
const livechats = RocketChat.authz.hasPermission(Meteor.userId(), 'view-l-room') ? RocketChat.models.Rooms.find({t: 'l'}, findOptions).fetch().map(room => room._id) : [];

const accessibleRooms = livechats.concat(subscribedRooms).concat(publicChannels);
const accessibleRooms = livechats.concat(subscribedRooms).concat(publicChannels);

const filterCriteria = `${ accessibleRooms.filter(unique).slice(0, solrFilterBooleanLimit).join(' OR ') }`;
return filterCriteria
? `&fq=meta_channel_id:(${ filterCriteria })`
: '&fq=meta_channel_id:""'; //fallback: if the user's not authorized to view any room, filter for "nothing"
};
let fq = `${ accessibleRooms.filter(unique).slice(0, solrFilterBooleanLimit).join(' OR ') }`;
fq = fq ? { fq: `meta_channel_id:(${ fq })` } : { fq: 'meta_channel_id:""'}; //fallback: if the user's not authorized to view any room, filter for "nothing"
const params = Object.assign(queryParams, fq);

const queryString = querystring.stringify(queryParams);
SystemLogger.debug('QueryString: ', queryString);
const searchResult = RocketChat.RateLimiter.limitFunction(
SmartiProxy.propagateToSmarti, 5, 1000, {
userId(userId) {
return !RocketChat.authz.hasPermission(userId, 'send-many-messages');
}
}
)(verbs.get, `conversation/search?${ queryString }${ _getAclQuery() }`);
)(verbs.get, 'conversation/search', params);
SystemLogger.debug('SearchResult: ', JSON.stringify(searchResult, null, 2));
return searchResult;
}
Expand Down

0 comments on commit b136c5c

Please sign in to comment.