Revert "Only get Selected fields for 'query' calls" (#8)

ScentreGroup · Jun 5, 2018 · 267dbee · 267dbee
1 parent 05aa1e1
commit 267dbee
Show file tree

Hide file tree

Showing 8 changed files with 20 additions and 126 deletions.
diff --git a/.gitignore b/.gitignore
@@ -2,7 +2,3 @@
 /coverage
 /dump
 /dev/config.json
-.idea
-yarn.lock
-package-lock.json
-.DS_store
diff --git a/package.json b/package.json
@@ -51,7 +51,6 @@
   },
   "dependencies": {
     "dataloader": "^1.3.0",
-    "graphql-fields": "^1.0.2",
     "https-proxy-agent": "^2.1.1",
     "lodash.camelcase": "^4.3.0",
     "lodash.chunk": "^4.2.0",

diff --git a/src/backref-types.js b/src/backref-types.js
@@ -30,18 +30,15 @@ function createBackrefFieldConfig (backref, Type) {
       skip: {type: GraphQLInt},
       limit: {type: GraphQLInt},
     },
-    resolve: (entryId, args, ctx, info) => {
+    resolve: (entryId, args, ctx) => {
       let q = `fields.${backref.fieldId}.sys.id[in]=${entryId}`;
       if (args.q) q = q + `&${args.q}`;
 
-      return ctx.entryLoader.query(
-        backref.ctId, {
-          q,
-          skip: args.skip,
-          limit: args.limit,
-        },
-        info
-      );
+      return ctx.entryLoader.query(backref.ctId, {
+        q,
+        skip: args.skip,
+        limit: args.limit,
+      })
     }
   };
 }

diff --git a/src/entry-loader.js b/src/entry-loader.js
@@ -4,43 +4,24 @@ const _get = require('lodash.get');
 const chunk = require('lodash.chunk');
 const qs = require('querystring');
 const DataLoader = require('dataloader');
-const graphqlFields = require('graphql-fields');
 
 const INCLUDE_DEPTH = 1;
 const CHUNK_SIZE = 70;
 // Chunk size may be unreliable. If wrong, may get "Bad Request" from graphQL
 const DEFAULT_LIMIT = 50;
 const MAX_LIMIT = 1000;
 const FORBIDDEN_QUERY_PARAMS = ['skip', 'limit', 'include', 'content_type', 'locale'];
-const MAX_FIELDS = 50;
-const IGNORE_FIELDS = ['sys','_backrefs', '__typename'];
 
 module.exports = createEntryLoader;
 
-const getSelectedFields = info => {
-  if (!info) {
-    return null;
-  }
-  const topLevelFields =
-    Object.keys(graphqlFields(info)).filter(key => !IGNORE_FIELDS.includes(key));
-
-  if (topLevelFields.length < 1 || topLevelFields.length > MAX_FIELDS) {
-    // There is a limit to the number of fields we can select. If too many get everything
-    return null;
-  }
-  const contentfulFields = topLevelFields.map(fieldKey => `fields.${fieldKey}`).join(',');
-
-  return `sys,${contentfulFields}`;
-};
-
 function createEntryLoader (http) {
   const loader = new DataLoader(load);
   const assets = {};
 
   return {
     get: getOne,
     getMany: loader.loadMany.bind(loader),
-    query: (ctId, args, info) => query(ctId, args, info).then(res => res.items),
+    query: (ctId, args) => query(ctId, args).then(res => res.items),
     count: (ctId, args) => query(ctId, args).then(res => res.total),
     queryAll,
     getIncludedAsset: id => assets[id],
@@ -79,7 +60,7 @@ function createEntryLoader (http) {
     });
   }
 
-  function query (ctId, {q = '', skip = 0, limit = DEFAULT_LIMIT} = {}, info) {
+  function query (ctId, {q = '', skip = 0, limit = DEFAULT_LIMIT} = {}) {
     const parsed = qs.parse(q);
     Object.keys(parsed).forEach(key => {
       if (FORBIDDEN_QUERY_PARAMS.includes(key)) {
@@ -94,10 +75,6 @@ function createEntryLoader (http) {
       content_type: ctId
     }, parsed);
 
-    const selectedFields = getSelectedFields(info);
-    if (selectedFields) {
-      params.select = selectedFields;
-    }
     return http.get('/entries', params).then(prime);
   }
 

diff --git a/src/field-config.js b/src/field-config.js
@@ -35,10 +35,10 @@ module.exports = {
 function createFieldConfig (Type, field, resolveFn) {
   return {
     type: Type,
-    resolve: (entity, _, ctx, info) => {
+    resolve: (entity, _, ctx) => {
       const fieldValue = _get(entity, ['fields', field.id], NOTHING);
       if (fieldValue !== NOTHING) {
-        return resolveFn ? resolveFn(fieldValue, ctx, info) : fieldValue;
+        return resolveFn ? resolveFn(fieldValue, ctx) : fieldValue;
       }
     }
   };

diff --git a/src/schema.js b/src/schema.js
@@ -75,13 +75,13 @@ function createQueryFields (spaceGraph) {
         limit: {type: GraphQLInt},
         ids: {type: new GraphQLList(IDType)},
       },
-      resolve: (_, args, ctx, info) => {
-        const {ids} = args;
+      resolve: (_, args, ctx) => {
+        const {ids} = args
         if (ids) {
-          return ctx.entryLoader.getMany(ids, info)
+          return ctx.entryLoader.getMany(ids)
           .then(objs => objs.filter(obj => obj && obj.sys.contentType.sys.id === ct.id))
         } else {
-          return ctx.entryLoader.query(ct.id, args, info);
+          return ctx.entryLoader.query(ct.id, args)
         }
       }
     };

diff --git a/test/entry-loader.test.js b/test/entry-loader.test.js
@@ -65,63 +65,6 @@ test('entry-loader: querying entries', function (t) {
   });
 });
 
-test('entry-loader: querying entries with resolve Info', function (t) {
-  t.plan(3);
-  const {httpStub, loader} = prepare();
-  const resolveInfo = {
-    fieldNodes: [{
-      selectionSet: {
-        selections: [
-          { name: { value: 'title' } },
-          { name: { value: 'slug' } },
-        ]
-      }
-    }],
-  };
-  loader.query('ctid', {q: 'fields.someNum=123&fields.test[exists]=true'}, resolveInfo)
-  .then(res => {
-    t.deepEqual(res, ITEMS);
-    t.equal(httpStub.get.callCount, 1);
-    t.deepEqual(httpStub.get.lastCall.args, ['/entries', {
-      skip: 0,
-      limit: 50,
-      include: 1,
-      content_type: 'ctid',
-      'fields.someNum': '123',
-      'fields.test[exists]': 'true',
-      select: 'sys,fields.title,fields.slug',
-    }]);
-  });
-});
-
-test('entry-loader: querying entries with only sys field requested', function (t) {
-  t.plan(3);
-  const {httpStub, loader} = prepare();
-  const resolveInfo = {
-    fieldNodes: [{
-      selectionSet: {
-        selections: [
-          { name: { value: 'sys' } },
-          { name: { value: '__typename' } },
-        ]
-      }
-    }],
-  };
-  loader.query('ctid', {q: 'fields.someNum=123&fields.test[exists]=true'}, resolveInfo)
-  .then(res => {
-    t.deepEqual(res, ITEMS);
-    t.equal(httpStub.get.callCount, 1);
-    t.deepEqual(httpStub.get.lastCall.args, ['/entries', {
-      skip: 0,
-      limit: 50,
-      include: 1,
-      content_type: 'ctid',
-      'fields.someNum': '123',
-      'fields.test[exists]': 'true',
-    }]);
-  });
-});
-
 test('entry-loader: counting entries', function (t) {
   t.plan(3);
   const {httpStub, loader} = prepare();

diff --git a/test/schema.test.js b/test/schema.test.js
@@ -8,7 +8,6 @@ const {
   GraphQLSchema,
   GraphQLObjectType
 } = require('graphql');
-const graphqlFields = require('graphql-fields');
 
 const {
   createSchema,
@@ -70,13 +69,11 @@ test('schema: querying generated schema', function (t) {
     .then(res => [entryLoader, res]);
   };
 
-  t.plan(30);
+  t.plan(22);
 
   testQuery('{ posts { title } }', {query: sinon.stub().resolves([post])})
   .then(([entryLoader, res]) => {
-    t.deepEqual(entryLoader.query.firstCall.args[0], 'postct');
-    t.deepEqual(entryLoader.query.firstCall.args[1], {});
-    t.deepEqual(graphqlFields(entryLoader.query.firstCall.args[2]), { title: {} });
+    t.deepEqual(entryLoader.query.firstCall.args, ['postct', {}]);
     t.equal(res.errors, undefined);
     t.deepEqual(res.data.posts, [{title: 'Hello world'}]);
   });
@@ -85,12 +82,10 @@ test('schema: querying generated schema', function (t) {
     '{ categories(skip: 2, limit: 3, q: "fields.name=test") { name } }',
     {query: sinon.stub().resolves([category])}
   ).then(([entryLoader, res]) => {
-    t.deepEqual(entryLoader.query.firstCall.args[0], 'catct');
     t.deepEqual(
-      entryLoader.query.firstCall.args[1],
-      {skip: 2, limit: 3, q: 'fields.name=test'}
+      entryLoader.query.firstCall.args,
+      ['catct', {skip: 2, limit: 3, q: 'fields.name=test'}]
     );
-    t.deepEqual(graphqlFields(entryLoader.query.firstCall.args[2]), { name: {} });
     t.equal(res.errors, undefined);
     t.deepEqual(res.data.categories, [{name: 'test'}]);
   });
@@ -110,9 +105,7 @@ test('schema: querying generated schema', function (t) {
     '{ posts { title } category(id: "c1") { name } }',
     {query: sinon.stub().resolves([post]), get: sinon.stub().resolves(category)}
   ).then(([entryLoader, res]) => {
-    t.deepEqual(entryLoader.query.firstCall.args[0], 'postct');
-    t.deepEqual(entryLoader.query.firstCall.args[1], {});
-    t.deepEqual(graphqlFields(entryLoader.query.firstCall.args[2]), { title: {} });
+    t.deepEqual(entryLoader.query.firstCall.args, ['postct', {}]);
     t.deepEqual(entryLoader.get.firstCall.args, ['c1', 'catct']);
     t.equal(res.errors, undefined);
     t.deepEqual(res.data, {posts: [{title: 'Hello world'}], category: {name: 'test'}});
@@ -123,18 +116,7 @@ test('schema: querying generated schema', function (t) {
     {query: sinon.stub().onCall(0).resolves([category]).onCall(1).resolves([post])}
   ).then(([entryLoader, res]) => {
     t.equal(res.errors, undefined);
-    t.deepEqual(entryLoader.query.firstCall.args[0], 'catct');
-    t.deepEqual(entryLoader.query.firstCall.args[1], {});
-    t.deepEqual(
-      graphqlFields(entryLoader.query.firstCall.args[2]),
-      {
-        _backrefs: {
-          posts__via__category: {
-            title: {},
-          },
-        },
-      }
-    );
+    t.deepEqual(entryLoader.query.firstCall.args, ['catct', {}]);
     t.deepEqual(entryLoader.query.lastCall.args[0], 'postct');
     t.deepEqual(res.data, {categories: [{_backrefs: {posts__via__category: [{title: 'Hello world'}]}}]});
   });