console: allow same named queries and unnamed queries on allowlist file upload

https://github.com/hasura/graphql-engine-mono/pull/1906

GitOrigin-RevId: bdd752f49213a2056f39050d40d3dc299dc07819

CHANGELOG.md

@@ -8,6 +8,7 @@
 - server: support EdDSA algorithm and key type for JWT
 - server: fix GraphQL type for single-row returning functions (close #7109)
 - console: add support for creation of indexes for Postgres data sources
+- console: allow same named queries and unnamed queries on allowlist file upload
 
 ## v2.0.6
 

console/src/components/Services/Settings/AllowedQueries/AddAllowedQuery.tsx

@@ -5,7 +5,7 @@ import styles from './AllowedQueries.scss';
 import ExpandableEditor from '../../../Common/Layout/ExpandableEditor/Editor';
 import Tooltip from '../../../Common/Tooltip/Tooltip';
 
-import { readFile, parseQueryString } from './utils';
+import { readFile, parseQueryString, renameDuplicates } from './utils';
 import { showErrorNotification } from '../../Common/Notification';
 import { addAllowedQueries } from '../../../../metadata/actions';
 import { allowedQueriesCollection } from '../../../../metadata/utils';
@@ -20,10 +20,11 @@ const defaultManualQuery: AllowedQueriesCollection = {
 
 type AddAllowedQueryProps = {
   dispatch: Dispatch;
+  allowedQueries: AllowedQueriesCollection[];
 };
 
 const AddAllowedQuery: React.FC<AddAllowedQueryProps> = props => {
-  const { dispatch } = props;
+  const { dispatch, allowedQueries } = props;
 
   const [manualQuery, setManualQuery] = useState<AllowedQueriesCollection>(
     defaultManualQuery
@@ -44,7 +45,8 @@ const AddAllowedQuery: React.FC<AddAllowedQueryProps> = props => {
     const addFileQueries = (content: string) => {
       try {
         const fileQueries = parseQueryString(content);
-        dispatch(addAllowedQueries(fileQueries, toggle));
+        const updatedQueries = renameDuplicates(fileQueries, allowedQueries);
+        dispatch(addAllowedQueries(updatedQueries, toggle));
       } catch (error) {
         dispatch(
           showErrorNotification('Uploading operations failed', error.message)
@@ -73,7 +75,11 @@ const AddAllowedQuery: React.FC<AddAllowedQueryProps> = props => {
     <div>
       <div>
         <div className={styles.add_mar_bottom_mid}>
-          <b>Operation name:</b>
+          <b>Query name:</b>
+          <Tooltip
+            message="This is an identifier for the query in the collection. 
+          This should be unique in the collection and can be different from the operation name of the query."
+          />
         </div>
         <input
           type="text"

console/src/components/Services/Settings/AllowedQueries/AllowedQueries.tsx

@@ -27,7 +27,10 @@ const AllowedQueries: React.FC<Props> = props => {
         <div className={`${styles.add_mar_top} ${styles.wd60}`}>
           <AllowedQueriesNotes />
           <hr className="my-lg" />
-          <AddAllowedQuery dispatch={dispatch} />
+          <AddAllowedQuery
+            dispatch={dispatch}
+            allowedQueries={allowedQueries}
+          />
           <hr className="my-lg" />
           <AllowedQueriesList
             dispatch={dispatch}

console/src/components/Services/Settings/AllowedQueries/AllowedQueriesList.tsx

@@ -14,6 +14,7 @@ import {
 import { AllowedQueriesCollection } from '../../../../metadata/reducer';
 import { Dispatch } from '../../../../types';
 import { getCollectionNames, checkLastQuery } from './utils';
+import Tooltip from '../../../Common/Tooltip/Tooltip';
 
 type AllowedQueriesListProps = {
   dispatch: Dispatch;
@@ -34,6 +35,7 @@ const AllowedQueriesList: React.FC<AllowedQueriesListProps> = props => {
     return allowedQueries.map((query, i) => {
       const queryName = query.name;
       const collectionName = query.collection;
+      const queryId = `${queryName}_${collectionName}_${i}`;
 
       const collapsedLabel = () => (
         <div>
@@ -45,17 +47,17 @@ const AllowedQueriesList: React.FC<AllowedQueriesListProps> = props => {
       const expandedLabel = collapsedLabel;
 
       const queryEditorExpanded = () => {
-        const modifiedQuery = modifiedQueries[queryName] || { ...query };
+        const modifiedQuery = modifiedQueries[queryId] || { ...query };
 
         const handleNameChange = (e: React.ChangeEvent<HTMLInputElement>) => {
           const newModifiedQueries = { ...modifiedQueries };
-          newModifiedQueries[queryName].name = e.target.value;
+          newModifiedQueries[queryId].name = e.target.value;
           setModifiedQueries(newModifiedQueries);
         };
 
         const handleQueryChange = (val: string) => {
           const newModifiedQueries = { ...modifiedQueries };
-          newModifiedQueries[queryName].query = val;
+          newModifiedQueries[queryId].query = val;
           setModifiedQueries(newModifiedQueries);
         };
 
@@ -63,7 +65,11 @@ const AllowedQueriesList: React.FC<AllowedQueriesListProps> = props => {
           <div>
             <div>
               <div className={styles.add_mar_bottom_mid}>
-                <b>Operation name:</b>
+                <b>Query name:</b>
+                <Tooltip
+                  message="This is an identifier for the query in the collection. 
+                This should be unique in the collection and can be different from the operation name of the query."
+                />
               </div>
               <input
                 type="text"
@@ -96,13 +102,13 @@ const AllowedQueriesList: React.FC<AllowedQueriesListProps> = props => {
 
       const editorExpandCallback = () => {
         const newModifiedQueries = { ...modifiedQueries };
-        newModifiedQueries[queryName] = { ...query };
+        newModifiedQueries[queryId] = { ...query };
         setModifiedQueries(newModifiedQueries);
       };
 
       const editorCollapseCallback = () => {
         const newModifiedQueries = { ...modifiedQueries };
-        delete newModifiedQueries[queryName];
+        delete newModifiedQueries[queryId];
         setModifiedQueries(newModifiedQueries);
       };
 
@@ -110,7 +116,7 @@ const AllowedQueriesList: React.FC<AllowedQueriesListProps> = props => {
         dispatch(
           updateAllowedQuery(
             queryName,
-            modifiedQueries[queryName],
+            modifiedQueries[queryId],
             collectionName
           )
         );
@@ -126,7 +132,7 @@ const AllowedQueriesList: React.FC<AllowedQueriesListProps> = props => {
       };
 
       return (
-        <div key={queryName}>
+        <div key={queryId}>
           <ExpandableEditor
             editorExpanded={queryEditorExpanded}
             property={`query-${i}`}

console/src/components/Services/Settings/AllowedQueries/utils.ts

@@ -1,5 +1,6 @@
 import { parse, print, visit, DefinitionNode } from 'graphql';
 import { AllowedQueriesCollection } from '../../../../metadata/reducer';
+import { allowedQueriesCollection } from '../../../../metadata/utils';
 
 export type NewDefinitionNode = DefinitionNode & {
   name?: {
@@ -69,6 +70,7 @@ const getQueryString = (
   return queryString;
 };
 
+// parses the query string and returns an array of queries
 export const parseQueryString = (queryString: string) => {
   const queries: { name: string; query: string }[] = [];
 
@@ -99,28 +101,16 @@ export const parseQueryString = (queryString: string) => {
   );
 
   queryDefs.forEach(queryDef => {
-    if (!queryDef.name) {
-      throw new Error(`Operation without name found: ${print(queryDef)}`);
-    }
+    const queryName = queryDef.name ? queryDef.name.value : `unnamed`;
 
     const query = {
-      name: queryDef.name.value,
+      name: queryName,
       query: getQueryString(queryDef, fragmentDefs, definitionHash),
     };
 
     queries.push(query);
   });
 
-  const queryNames = queries.map(q => q.name);
-  const duplicateNames = queryNames.filter(
-    (q, i) => queryNames.indexOf(q) !== i
-  );
-  if (duplicateNames.length > 0) {
-    throw new Error(
-      `Operations with duplicate names found: ${duplicateNames.join(', ')}`
-    );
-  }
-
   return queries;
 };
 
@@ -137,6 +127,35 @@ export const getQueriesInCollection = (
   return queries;
 };
 
+// check if the uploaded queries have same names within the file, or among the already present queries
+export const renameDuplicates = (
+  fileQueries: { name: string; query: string }[],
+  allQueries: AllowedQueriesCollection[]
+) => {
+  // we only allow addition to allowedQueriesCollection from console atm
+  const allowListQueries = getQueriesInCollection(
+    allowedQueriesCollection,
+    allQueries
+  );
+
+  const queryNames = new Set();
+  allowListQueries.forEach(query => queryNames.add(query.name));
+
+  const updatedQueries = fileQueries.map(query => {
+    let queryName = query.name;
+    if (queryNames.has(queryName)) {
+      let num = 1;
+      while (queryNames.has(queryName)) {
+        queryName = `${query.name}_${num++}`;
+      }
+    }
+    queryNames.add(queryName);
+    return { name: queryName, query: query.query };
+  });
+
+  return updatedQueries;
+};
+
 export const checkLastQuery = (
   collectionName: string,
   queries: AllowedQueriesCollection[]

console/src/components/Services/Settings/__tests__/__snapshots__/settings.test.ts.snap

@@ -0,0 +1,97 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`AllowedQueries_Utils.ts renameDuplicates should rename duplicate queries 1`] = `
+Array [
+  Object {
+    "name": "getAuthors_1",
+    "query": "query getAuthors {
+  author {
+    id
+    name
+  }
+}",
+  },
+  Object {
+    "name": "getAuthors_2",
+    "query": "query getAuthors {
+  author {
+    id
+    name
+    address
+  }
+}",
+  },
+  Object {
+    "name": "getAuthors_3",
+    "query": "query getAuthors {
+  author {
+    id
+    name
+    age
+  }
+}",
+  },
+  Object {
+    "name": "unnamed",
+    "query": "{
+  student {
+    id
+    name
+    age
+  }
+}",
+  },
+  Object {
+    "name": "unnamed_1",
+    "query": "{
+  student {
+    id
+    name
+    roll
+  }
+}",
+  },
+  Object {
+    "name": "unnamed_2",
+    "query": "{
+  student {
+    id
+    name
+    address
+  }
+}",
+  },
+  Object {
+    "name": "getArticles",
+    "query": "query getArticles {
+  article {
+    id
+    title
+  }
+}",
+  },
+  Object {
+    "name": "getArticle",
+    "query": "query getArticle {
+  article {
+    id
+    title
+    ...frag
+  }
+}
+
+fragment frag on Starship {
+  name
+}",
+  },
+  Object {
+    "name": "addArticles",
+    "query": "mutation addArticles {
+  insert_articles {
+    id
+    title
+  }
+}",
+  },
+]
+`;

console/src/components/Services/Settings/__tests__/fixtures/allow-list.ts

@@ -0,0 +1,94 @@
+export const uploadedFileData = `# will be ignored by the allow-list
+type Starship {
+  id: ID!
+  name: String!
+  length(unit: LengthUnit = METER): Float
+}
+
+# will be ignored by the allow-list
+scalar parsec
+
+# will be ignored by the allow-list
+enum Episode {
+  NEWHOPE
+  EMPIRE
+  JEDI
+}
+
+# will be stored in the allow-list
+query getAuthors{
+  author {
+    id
+    name
+  }
+}
+
+query getAuthors{
+  author {
+    id
+    name
+    address
+  }
+}
+
+query getAuthors{
+  author {
+    id
+    name
+    age
+  }
+}
+
+query {
+  student {
+    id
+    name
+    age
+  }
+}
+
+query {
+  student {
+    id
+    name
+    roll
+  }
+}
+
+query {
+  student {
+    id
+    name
+    address
+  }
+}
+
+fragment frag on Starship {
+  name
+}
+
+# will be stored in the allow-list
+query getArticles {
+  article {
+    id
+    title
+  }
+}
+
+# will be stored in the allow-list after patching in the fragment 
+query getArticle {
+  article {
+    id
+    title
+    ...frag
+  }
+}
+
+# will be stored in the allow-list
+mutation addArticles {
+  insert_articles {
+    id
+    title
+  }
+}
+`;

console/src/components/Services/Settings/__tests__/settings.test.ts

@@ -0,0 +1,20 @@
+import { allowedQueriesCollection } from '../../../../metadata/utils';
+import { renameDuplicates, parseQueryString } from '../AllowedQueries/utils';
+import { uploadedFileData } from './fixtures/allow-list';
+
+describe('AllowedQueries_Utils.ts', () => {
+  describe('renameDuplicates', () => {
+    it('should rename duplicate queries', () => {
+      const allowedListQueries = [
+        {
+          name: 'getAuthors',
+          query: 'query getAuthors { author {id name} }',
+          collection: allowedQueriesCollection,
+        },
+      ];
+      const fileQueries = parseQueryString(uploadedFileData);
+      const updatedQueries = renameDuplicates(fileQueries, allowedListQueries);
+      expect(updatedQueries).toMatchSnapshot();
+    });
+  });
+});