Applying review

Signed-off-by: Yuri Roncella <[email protected]>

packages/jaeger-ui/src/actions/jaeger-api.js

@@ -54,6 +54,6 @@ export const fetchDependencies = createAction('@JAEGER_API/FETCH_DEPENDENCIES',
 
 export const uploadTraces = createAction(
   '@JAEGER_API/UPLOAD_TRACES',
-  fileList => fileReader.readJSONFile(fileList),
+  fileList => fileReader.readJsonFile(fileList),
   fileList => ({ fileList })
 );

packages/jaeger-ui/src/actions/jaeger-api.test.js

@@ -123,11 +123,11 @@ it('uploadTraces should return a promise', () => {
   expect(isPromise(payload)).toBeTruthy();
 });
 
-it('uploadTraces should call readJSONFile', () => {
+it('uploadTraces should call readJsonFile', () => {
   const fileList = { data: {}, filename: 'whatever' };
   const mock = sinon.mock(fileReader);
   const called = mock
-    .expects('readJSONFile')
+    .expects('readJsonFile')
     .once()
     .withExactArgs(fileList);
   jaegerApiActions.uploadTraces(fileList);

packages/jaeger-ui/src/api/jaeger.js

@@ -86,10 +86,10 @@ const JaegerAPI = {
     return getJSON(`${this.apiRoot}traces`, { query });
   },
   fetchServices() {
-    return getJSON(`${this.apiRoot}services`).catch(() => []);
+    return getJSON(`${this.apiRoot}services`);
   },
   fetchServiceOperations(serviceName) {
-    return getJSON(`${this.apiRoot}services/${encodeURIComponent(serviceName)}/operations`).catch(() => []);
+    return getJSON(`${this.apiRoot}services/${encodeURIComponent(serviceName)}/operations`);
   },
   fetchDependencies(endTs = new Date().getTime(), lookback = DEFAULT_DEPENDENCY_LOOKBACK) {
     return getJSON(`${this.apiRoot}dependencies`, { query: { endTs, lookback } });

packages/jaeger-ui/src/api/jaeger.test.js

@@ -69,44 +69,6 @@ it('fetchTrace() throws an error on a >= 400 status code', done => {
   });
 });
 
-it('fetchServices() returns [] on a >= 400 status code', done => {
-  const status = 400;
-  const statusText = 'some-status';
-  const msg = 'some-message';
-  const errorData = { errors: [{ msg, code: status }] };
-
-  fetchMock.mockReturnValue(
-    Promise.resolve({
-      status,
-      statusText,
-      text: () => Promise.resolve(JSON.stringify(errorData)),
-    })
-  );
-  JaegerAPI.fetchServices().then(services => {
-    expect(services).toEqual([]);
-    done();
-  });
-});
-
-it('fetchOperations() returns [] on a >= 400 status code', done => {
-  const status = 400;
-  const statusText = 'some-status';
-  const msg = 'some-message';
-  const errorData = { errors: [{ msg, code: status }] };
-
-  fetchMock.mockReturnValue(
-    Promise.resolve({
-      status,
-      statusText,
-      text: () => Promise.resolve(JSON.stringify(errorData)),
-    })
-  );
-  JaegerAPI.fetchServiceOperations().then(operations => {
-    expect(operations).toEqual([]);
-    done();
-  });
-});
-
 it('fetchTrace() throws an useful error derived from a text payload', done => {
   const status = 400;
   const statusText = 'some-status';

packages/jaeger-ui/src/components/SearchTracePage/FileUploader.js

@@ -1,4 +1,6 @@
-// Copyright (c) 2017 Uber Technologies, Inc.
+// @flow
+
+// Copyright (c) 2019 Uber Technologies, Inc.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -15,35 +17,21 @@
 import * as React from 'react';
 import { Upload, Icon } from 'antd';
 import PropTypes from 'prop-types';
-import { connect } from 'react-redux';
-import { bindActionCreators } from 'redux';
-
-import * as jaegerApiActions from '../../actions/jaeger-api';
 
 const Dragger = Upload.Dragger;
 
-export function FileUploaderImpl(props) {
-  const { uploadTraces } = props;
+type FileUploaderProps = {
+  uploadTraces: PropTypes.func.isRequired,
+};
+
+export default function FileUploader(props: FileUploaderProps) {
   return (
-    <Dragger accept=".json" customRequest={uploadTraces} multiple>
+    <Dragger accept=".json" customRequest={props.uploadTraces} multiple>
       <p className="ant-upload-drag-icon">
-        <Icon type="inbox" />
+        <Icon type="file-add" />
       </p>
       <p className="ant-upload-text">Click or drag files to this area.</p>
       <p className="ant-upload-hint">Support JSON files containig one or more traces.</p>
     </Dragger>
   );
 }
-
-FileUploaderImpl.propTypes = {
-  uploadTraces: PropTypes.func.isRequired,
-};
-
-function mapDispatchToProps(dispatch) {
-  const { uploadTraces } = bindActionCreators(jaegerApiActions, dispatch);
-  return {
-    uploadTraces,
-  };
-}
-
-export default connect(null, mapDispatchToProps)(FileUploaderImpl);

packages/jaeger-ui/src/components/SearchTracePage/FileUploader.test.js

@@ -1,4 +1,4 @@
-// Copyright (c) 2017 Uber Technologies, Inc.
+// Copyright (c) 2019 Uber Technologies, Inc.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -13,18 +13,18 @@
 // limitations under the License.
 
 import React from 'react';
-import { mount } from 'enzyme';
+import { shallow } from 'enzyme';
 
-import { FileUploaderImpl as FileUploader } from './FileUploader';
+import FileUploader from './FileUploader';
 
 describe('<FileUploader />', () => {
   let wrapper;
 
   beforeEach(() => {
-    wrapper = mount(<FileUploader />);
+    wrapper = shallow(<FileUploader />);
   });
 
-  it('does not explode', () => {
-    expect(wrapper).toBeDefined();
+  it('matches the snapshot', () => {
+    expect(wrapper).toMatchSnapshot();
   });
 });

packages/jaeger-ui/src/components/SearchTracePage/__snapshots__/FileUploader.test.js.snap

@@ -0,0 +1,26 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`<FileUploader /> matches the snapshot 1`] = `
+<Dragger
+  accept=".json"
+  multiple={true}
+>
+  <p
+    className="ant-upload-drag-icon"
+  >
+    <Icon
+      type="file-add"
+    />
+  </p>
+  <p
+    className="ant-upload-text"
+  >
+    Click or drag files to this area.
+  </p>
+  <p
+    className="ant-upload-hint"
+  >
+    Support JSON files containig one or more traces.
+  </p>
+</Dragger>
+`;

packages/jaeger-ui/src/components/SearchTracePage/index.js

@@ -88,6 +88,7 @@ export class SearchTracePageImpl extends Component {
       services,
       traceResults,
       queryOfResults,
+      uploadTraces,
     } = this.props;
     const hasTraceResults = traceResults && traceResults.length > 0;
     const showErrors = errors && !loadingTraces;
@@ -103,7 +104,7 @@ export class SearchTracePageImpl extends Component {
                     {!loadingServices && services ? <SearchForm services={services} /> : <LoadingIndicator />}
                   </TabPane>
                   <TabPane tab="Load from File" key="fileUploader">
-                    <FileUploader />
+                    <FileUploader uploadTraces={uploadTraces} />
                   </TabPane>
                 </Tabs>
               </div>
@@ -186,6 +187,7 @@ SearchTracePageImpl.propTypes = {
       message: PropTypes.string,
     })
   ),
+  uploadTraces: PropTypes.func,
 };
 
 const stateTraceXformer = getLastXformCacher(stateTrace => {
@@ -262,10 +264,13 @@ export function mapStateToProps(state) {
 }
 
 function mapDispatchToProps(dispatch) {
-  const { fetchMultipleTraces, fetchServiceOperations, fetchServices, searchTraces } = bindActionCreators(
-    jaegerApiActions,
-    dispatch
-  );
+  const {
+    fetchMultipleTraces,
+    fetchServiceOperations,
+    fetchServices,
+    searchTraces,
+    uploadTraces,
+  } = bindActionCreators(jaegerApiActions, dispatch);
   const { cohortAddTrace, cohortRemoveTrace } = bindActionCreators(traceDiffActions, dispatch);
   return {
     cohortAddTrace,
@@ -274,6 +279,7 @@ function mapDispatchToProps(dispatch) {
     fetchServiceOperations,
     fetchServices,
     searchTraces,
+    uploadTraces,
   };
 }
 

packages/jaeger-ui/src/reducers/trace.js

@@ -126,7 +126,7 @@ function searchErred(state, { meta, payload }) {
 
 function uploadStarted(state) {
   const search = {
-    results: [].concat(state.search.results),
+    results: state.search.results,
     state: fetchedState.LOADING,
   };
   return { ...state, search };
@@ -135,15 +135,15 @@ function uploadStarted(state) {
 function uploadDone(state, { payload }) {
   const processed = payload.data.map(transformTraceData);
   const resultTraces = {};
-  const results = [].concat(state.search.results);
+  const results = new Set(state.search.results);
   for (let i = 0; i < processed.length; i++) {
     const data = processed[i];
     const id = data.traceID;
     resultTraces[id] = { data, id, state: fetchedState.DONE };
-    results.push(id);
+    results.add(id);
   }
   const traces = { ...state.traces, ...resultTraces };
-  const search = { ...state.search, results, state: fetchedState.DONE };
+  const search = { ...state.search, results: Array.from(results), state: fetchedState.DONE };
   return { ...state, search, traces };
 }
 

packages/jaeger-ui/src/utils/fileReader.js

@@ -1,4 +1,6 @@
-// Copyright (c) 2017 Uber Technologies, Inc.
+// @flow
+
+// Copyright (c) 2019 Uber Technologies, Inc.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -13,20 +15,19 @@
 // limitations under the License.
 
 const fileReader = {
-  readJSONFile(fileList) {
+  readJsonFile(fileList: { file: File }) {
     return new Promise((resolve, reject) => {
       const reader = new FileReader();
-      reader.onloadend = () => {
-        resolve(reader.result);
-      };
-      reader.onerror = () => {
-        reject();
-      };
-      reader.onabort = () => {
-        reject();
+      reader.onload = () => {
+        if (typeof reader.result === 'string') {
+          return resolve(JSON.parse(reader.result));
+        }
+        return reject('Invalid result type');
       };
+      reader.onerror = reject;
+      reader.onabort = reject;
       reader.readAsText(fileList.file);
-    }).then(result => JSON.parse(result));
+    });
   },
 };
 

packages/jaeger-ui/src/utils/fileReader.test.js

@@ -1,4 +1,4 @@
-// Copyright (c) 2017 Uber Technologies, Inc.
+// Copyright (c) 2019 Uber Technologies, Inc.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -17,24 +17,24 @@ import sinon from 'sinon';
 
 import fileReader from './fileReader.js';
 
-it('readJSONFile returns a promise', () => {
+it('readJsonFile returns a promise', () => {
   const fileList = { data: {}, filename: 'whatever' };
 
-  const promise = fileReader.readJSONFile(fileList);
+  const promise = fileReader.readJsonFile(fileList);
   expect(isPromise(promise)).toBeTruthy();
 });
 
-it('readJSONFile fails to load a fail', async () => {
+it('readJsonFile fails to load a fail', async () => {
   expect.assertions(1);
   const fileList = { data: {}, filename: 'whatever' };
   try {
-    await fileReader.readJSONFile(fileList);
+    await fileReader.readJsonFile(fileList);
   } catch (e) {
     expect(true).toBeTruthy();
   }
 });
 
-it('readJSONFile fails when fileList is wrong', async () => {
+it('readJsonFile fails when fileList is wrong', async () => {
   expect.assertions(2);
 
   const mock = sinon.mock(window);
@@ -49,7 +49,7 @@ it('readJSONFile fails when fileList is wrong', async () => {
   };
 
   try {
-    await fileReader.readJSONFile(fileList);
+    await fileReader.readJsonFile(fileList);
   } catch (e) {
     expect(true).toBeTruthy();
     expect(called.verify()).toBeTruthy();