[Code] Remove regex git url validation and add more unit tests for re…

…pository utils (#33919) * [Code] more unit tests * [Code] fix ci breaks
elastic · Mar 27, 2019 · 7cd7792 · 7cd7792
1 parent b1680df
commit 7cd7792
Show file tree

Hide file tree

Showing 12 changed files with 113 additions and 46 deletions.
diff --git a/x-pack/plugins/code/common/git_url_utils.test.ts b/x-pack/plugins/code/common/git_url_utils.test.ts
@@ -30,7 +30,7 @@ test('Git url validation', () => {
 
   // An url without protocol
   expect(isValidGitUrl('/Users/elastic/elasticsearch')).toBeFalsy();
-  expect(isValidGitUrl('github.com/elastic/elasticsearch')).toBeFalsy();
+  expect(isValidGitUrl('github.com/elastic/elasticsearch')).toBeTruthy();
 
   // An valid git url but without whitelisted host
   expect(isValidGitUrl('https://github.com/elastic/elasticsearch.git', ['gitlab.com'])).toBeFalsy();

diff --git a/x-pack/plugins/code/common/git_url_utils.ts b/x-pack/plugins/code/common/git_url_utils.ts
@@ -11,26 +11,23 @@ export function isValidGitUrl(
   hostWhitelist?: string[],
   protocolWhitelist?: string[]
 ): boolean {
-  const regex = /(?:git|ssh|https?|git@[-\w.]+):(\/\/)?(.*?)(\.git)?(\/?|\#[-\d\w._]+?)$/;
-  const isPatternValid = regex.test(url);
+  try {
+    const repo = GitUrlParse(url);
 
-  if (!isPatternValid) {
-    return false;
-  }
-
-  const repo = GitUrlParse(url);
+    let isHostValid = true;
+    if (hostWhitelist && hostWhitelist.length > 0) {
+      const hostSet = new Set(hostWhitelist);
+      isHostValid = hostSet.has(repo.source);
+    }
 
-  let isHostValid = true;
-  if (hostWhitelist && hostWhitelist.length > 0) {
-    const hostSet = new Set(hostWhitelist);
-    isHostValid = hostSet.has(repo.source);
-  }
+    let isProtocolValid = true;
+    if (protocolWhitelist && protocolWhitelist.length > 0) {
+      const protocolSet = new Set(protocolWhitelist);
+      isProtocolValid = protocolSet.has(repo.protocol);
+    }
 
-  let isProtocolValid = true;
-  if (protocolWhitelist && protocolWhitelist.length > 0) {
-    const protocolSet = new Set(protocolWhitelist);
-    isProtocolValid = protocolSet.has(repo.protocol);
+    return isHostValid && isProtocolValid;
+  } catch (error) {
+    return false;
   }
-
-  return isHostValid && isProtocolValid;
 }
diff --git a/x-pack/plugins/code/common/repository_utils.test.ts b/x-pack/plugins/code/common/repository_utils.test.ts
@@ -4,6 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
+import { FileTreeItemType } from '../model';
 import { RepositoryUtils } from './repository_utils';
 
 test('Repository url parsing', () => {
@@ -107,3 +108,72 @@ test('Normalize repository index name', () => {
   const indexName4 = RepositoryUtils.normalizeRepoUriToIndexName('github.com/elastic/kibana-code');
   expect(indexName3 === indexName4).toBeFalsy();
 });
+
+test('Parse repository uri', () => {
+  expect(RepositoryUtils.orgNameFromUri('github.com/elastic/kibana')).toEqual('elastic');
+  expect(RepositoryUtils.repoNameFromUri('github.com/elastic/kibana')).toEqual('kibana');
+  expect(RepositoryUtils.repoFullNameFromUri('github.com/elastic/kibana')).toEqual(
+    'elastic/kibana'
+  );
+
+  // For invalid repository uri
+  expect(() => {
+    RepositoryUtils.orgNameFromUri('foo/bar');
+  }).toThrowError('Invalid repository uri.');
+  expect(() => {
+    RepositoryUtils.repoNameFromUri('foo/bar');
+  }).toThrowError('Invalid repository uri.');
+  expect(() => {
+    RepositoryUtils.repoFullNameFromUri('foo/bar');
+  }).toThrowError('Invalid repository uri.');
+});
+
+test('Repository local path', () => {
+  expect(RepositoryUtils.repositoryLocalPath('/tmp', 'github.com/elastic/kibana')).toEqual(
+    '/tmp/github.com/elastic/kibana'
+  );
+  expect(RepositoryUtils.repositoryLocalPath('tmp', 'github.com/elastic/kibana')).toEqual(
+    'tmp/github.com/elastic/kibana'
+  );
+});
+
+test('Parse location to url', () => {
+  expect(
+    RepositoryUtils.locationToUrl({
+      uri: 'git://github.com/elastic/eui/blob/master/generator-eui/app/component.js',
+      range: {
+        start: {
+          line: 4,
+          character: 17,
+        },
+        end: {
+          line: 27,
+          character: 1,
+        },
+      },
+    })
+  ).toEqual('/github.com/elastic/eui/blob/master/generator-eui/app/component.js!L5:17');
+});
+
+test('Get all files from a repository file tree', () => {
+  expect(
+    RepositoryUtils.getAllFiles({
+      name: 'foo',
+      type: FileTreeItemType.Directory,
+      path: '/foo',
+      children: [
+        {
+          name: 'bar',
+          type: FileTreeItemType.File,
+          path: '/foo/bar',
+        },
+        {
+          name: 'boo',
+          type: FileTreeItemType.File,
+          path: '/foo/boo',
+        },
+      ],
+      childrenCount: 2,
+    })
+  ).toEqual(['/foo/bar', '/foo/boo']);
+});
diff --git a/x-pack/plugins/code/common/repository_utils.ts b/x-pack/plugins/code/common/repository_utils.ts
@@ -36,29 +36,29 @@ export class RepositoryUtils {
     const segs = repoUri.split('/');
     if (segs && segs.length === 3) {
       return segs[1];
-    } else {
-      return 'invalid';
     }
+
+    throw new Error('Invalid repository uri.');
   }
 
   // From uri 'origin/org/name' to 'name'
   public static repoNameFromUri(repoUri: RepositoryUri): string {
     const segs = repoUri.split('/');
     if (segs && segs.length === 3) {
       return segs[2];
-    } else {
-      return 'invalid';
     }
+
+    throw new Error('Invalid repository uri.');
   }
 
   // From uri 'origin/org/name' to 'org/name'
   public static repoFullNameFromUri(repoUri: RepositoryUri): string {
     const segs = repoUri.split('/');
     if (segs && segs.length === 3) {
       return segs[1] + '/' + segs[2];
-    } else {
-      return 'invalid';
     }
+
+    throw new Error('Invalid repository uri.');
   }
 
   // Return the local data path of a given repository.

diff --git a/x-pack/plugins/code/public/components/file_tree/file_tree.test.tsx b/x-pack/plugins/code/public/components/file_tree/file_tree.test.tsx
@@ -17,7 +17,7 @@ const location: Location = createLocation({
   pathname: '/github.com/google/guava/tree/master/guava/src/com/google',
 });
 
-const match: match<MainRouteParams> = createMatch({
+const m: match<MainRouteParams> = createMatch({
   path: '/:resource/:org/:repo/:pathType(blob|tree)/:revision/:path*:goto(!.*)?',
   url: '/github.com/google/guava/tree/master/guava/src/com/google',
   isExact: true,
@@ -40,7 +40,7 @@ test('render correctly', () => {
         node={props.node}
         openedPaths={props.openedPaths}
         history={history}
-        match={match}
+        match={m}
         location={location}
         closeTreePath={mockFunction}
         openTreePath={mockFunction}

diff --git a/...omponents/query_bar/components/typeahead/__snapshots__/suggestion_component.test.tsx.snap b/...omponents/query_bar/components/typeahead/__snapshots__/suggestion_component.test.tsx.snap
@@ -1,7 +1,7 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`render file item 1`] = `
-<Component
+<SuggestionComponent
   ariaId="suggestion-1-1"
   innerRef={[Function]}
   onClick={[Function]}
@@ -51,11 +51,11 @@ exports[`render file item 1`] = `
       </div>
     </div>
   </div>
-</Component>
+</SuggestionComponent>
 `;
 
 exports[`render repository item 1`] = `
-<Component
+<SuggestionComponent
   ariaId="suggestion-1-1"
   innerRef={[Function]}
   onClick={[Function]}
@@ -102,11 +102,11 @@ exports[`render repository item 1`] = `
       </div>
     </div>
   </div>
-</Component>
+</SuggestionComponent>
 `;
 
 exports[`render symbol item 1`] = `
-<Component
+<SuggestionComponent
   ariaId="suggestion-1-1"
   innerRef={[Function]}
   onClick={[Function]}
@@ -197,5 +197,5 @@ exports[`render symbol item 1`] = `
       </div>
     </div>
   </div>
-</Component>
+</SuggestionComponent>
 `;
diff --git a/...mponents/query_bar/components/typeahead/__snapshots__/suggestions_component.test.tsx.snap b/...mponents/query_bar/components/typeahead/__snapshots__/suggestions_component.test.tsx.snap
@@ -212,7 +212,7 @@ exports[`render full suggestions component 1`] = `
                   </div>
                 </div>
               </EuiFlexGroup>
-              <Component
+              <SuggestionComponent
                 ariaId="suggestion-0-0"
                 innerRef={[Function]}
                 key="tokenClass - 0-0 - java.lang.String"
@@ -304,7 +304,7 @@ exports[`render full suggestions component 1`] = `
                     </div>
                   </div>
                 </div>
-              </Component>
+              </SuggestionComponent>
             </div>
             <div
               className="kbnTypeahead__items codeSearch-suggestion__group"
@@ -387,7 +387,7 @@ exports[`render full suggestions component 1`] = `
                   </div>
                 </div>
               </EuiFlexGroup>
-              <Component
+              <SuggestionComponent
                 ariaId="suggestion-1-0"
                 innerRef={[Function]}
                 key=" - 1-0 - src/foo/bar.java"
@@ -432,7 +432,7 @@ exports[`render full suggestions component 1`] = `
                     </div>
                   </div>
                 </div>
-              </Component>
+              </SuggestionComponent>
             </div>
             <div
               className="kbnTypeahead__items codeSearch-suggestion__group"
@@ -517,7 +517,7 @@ exports[`render full suggestions component 1`] = `
                   </div>
                 </div>
               </EuiFlexGroup>
-              <Component
+              <SuggestionComponent
                 ariaId="suggestion-2-0"
                 innerRef={[Function]}
                 key=" - 2-0 - elastic/kibana"
@@ -560,8 +560,8 @@ exports[`render full suggestions component 1`] = `
                     </div>
                   </div>
                 </div>
-              </Component>
-              <Component
+              </SuggestionComponent>
+              <SuggestionComponent
                 ariaId="suggestion-2-1"
                 innerRef={[Function]}
                 key=" - 2-1 - elastic/elasticsearch"
@@ -604,7 +604,7 @@ exports[`render full suggestions component 1`] = `
                     </div>
                   </div>
                 </div>
-              </Component>
+              </SuggestionComponent>
               <div
                 className="codeSearch-suggestion__link"
               >

diff --git a/x-pack/test/functional/apps/code/code_intelligence.ts b/x-pack/test/functional/apps/code/code_intelligence.ts
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import expect from 'expect.js';
+import expect from '@kbn/expect';
 import { TestInvoker } from './lib/types';
 
 // tslint:disable:no-default-export

diff --git a/x-pack/test/functional/apps/code/explore_repository.ts b/x-pack/test/functional/apps/code/explore_repository.ts
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import expect from 'expect.js';
+import expect from '@kbn/expect';
 import { TestInvoker } from './lib/types';
 
 // tslint:disable:no-default-export

diff --git a/x-pack/test/functional/apps/code/manage_repositories.ts b/x-pack/test/functional/apps/code/manage_repositories.ts
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import expect from 'expect.js';
+import expect from '@kbn/expect';
 import { TestInvoker } from './lib/types';
 
 // tslint:disable:no-default-export

diff --git a/x-pack/test/functional/apps/code/search.ts b/x-pack/test/functional/apps/code/search.ts
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import expect from 'expect.js';
+import expect from '@kbn/expect';
 import { TestInvoker } from './lib/types';
 
 // tslint:disable:no-default-export

diff --git a/x-pack/test/functional/apps/code/with_security.ts b/x-pack/test/functional/apps/code/with_security.ts
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import expect from 'expect.js';
+import expect from '@kbn/expect';
 import { TestInvoker } from './lib/types';
 
 // tslint:disable:no-default-export