v0.2.0

closes #19

Author: cvburgess

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## v0.2.0 - 08-07-2019
+
+- Fix issues with lexical scoping [#19]
+- Deprecate this.knex in favor of this.db
+- Mark batching as experimental
+- Update docs to reflect current development status
+
 ## v0.1.7 - 24-04-2019
 
 - Fix for responses from sqlite and mssql drivers [#9]
@@ -33,3 +40,11 @@
 
 - Initial publication
 - Adds SQLCache and SQLDataSource classes
+
+- [#2] https://github.com/cvburgess/SQLDataSource/issues/2
+- [#3] https://github.com/cvburgess/SQLDataSource/issues/3
+- [#5] https://github.com/cvburgess/SQLDataSource/issues/5
+- [#12] https://github.com/cvburgess/SQLDataSource/issues/12
+- [#15] https://github.com/cvburgess/SQLDataSource/issues/15
+- [#9] https://github.com/cvburgess/SQLDataSource/issues/9
+- [#19] https://github.com/cvburgess/SQLDataSource/issues/19

README.md

@@ -2,9 +2,15 @@
 
 This package combines the power of [Knex] with the ease of use of [Apollo DataSources].
 
-**NOTE: THIS PACKAGE HAS OFFICIAL SUPPORT FOR POSTGRESQL AND SQLITE ONLY**
+## BREAKING CHANGE IN v0.2.0
 
-Other DBs will work, but they will return the default response from `knex.raw()` which may or may not be what you are expecting. I am working on support for other DBs, but if you want to open a PR, that helps!
+Batching of queries is hacked together with `knex.raw()` - while this is not ideal, due to the way Knex works, I have not found a more consistent way to do this.
+
+As such, when you use `getBatched` or `getBatchedAndCached` the result will not be the pretty, normalized output you may expect from Knex, it will be the raw output from your DB driver of choice as if you had run the query with `knex.raw()`.
+
+**If you find a way to implement caching without using `knex.raw()` please open a PR!**
+
+While I would love to spend more time investigating this, unfortunately my time is limited at the moment.
 
 ## Getting Started
 
@@ -33,7 +39,7 @@ class MyDatabase extends SQLDataSource {
   constructor() {
     super();
     // Add your instance of Knex to the DataSource
-    this.knex = knex;
+    this.db = knex;
   }
 
   getUsers() {
@@ -43,13 +49,13 @@ class MyDatabase extends SQLDataSource {
     // A promise without any caching or batching
     return query;
 
-    // Batch the query with DataLoader
+    // Batch the query with DataLoader - RETURNS A RAW RESPONSE
     return this.getBatched(query);
 
     // Cache the result for 1 minute
     return this.getCached(query, MINUTE);
 
-    // Batch the query and cache the result for 1 minute
+    // Batch the query and cache the result for 1 minute - RETURNS A RAW RESPONSE
     return this.getBatchedAndCached(query, MINUTE);
   }
 }
@@ -85,6 +91,8 @@ This method accepts one parameter:
 
 - `knexQuery`: <knexObject> A knex object that has not been then'd
 
+**NOTE: This method will return the raw response from your DB driver.**
+
 ### Caching ( getCached )
 
 If you were to make the same query over the course of multiple requests to your server you could also be making needless requests to your server - especially for expensive queries.
@@ -116,6 +124,8 @@ const query = this.db.select().from("users");
 return this.getBatchedAndCached(query, MINUTE);
 ```
 
+**NOTE: This method will return the raw response from your DB driver.**
+
 ### initialize
 
 SQLDataSource creates a new SQLCache on every initialize call with the cache and context from Apollo Server so you never should be directly creating a new SQLCache.

SQLCache.js

@@ -4,38 +4,24 @@ const DataLoader = require("dataloader");
 class SQLCache {
   constructor(cache = new InMemoryLRUCache(), knex) {
     this.cache = cache;
-    this.knex = knex;
     this.loader = new DataLoader(rawQueries =>
       Promise.all(rawQueries.map(rawQuery => knex.raw(rawQuery)))
     );
   }
 
-  normalizeDBResult(result) {
-    switch (this.knex.client) {
-      case "postgres":
-        return result && result.rows;
-      case "mssql":
-        return result;
-      case "sqlite3":
-        return result;
-      // TODO: Test and implement remaining clients
-      case "mysql":
-      case "mysql2":
-      case "oracledb":
-      case "redshift":
-      default:
-        return result;
-    }
-  }
-
   getCacheKeyForQuery(query) {
     const queryString = query.toString();
     return `sqlcache:${queryString}`;
   }
 
   getBatched(query) {
+    // eslint-disable-next-line
+    console.warn(
+      "WARNING: batching is considered unstable and returns RAW responses"
+    );
+
     const queryString = query.toString();
-    return this.loader.load(queryString).then(this.normalizeDBResult);
+    return this.loader.load(queryString);
   }
 
   getCached(query, ttl) {

SQLDataSource.js

@@ -6,17 +6,25 @@ const { DEBUG } = process.env;
 
 let hasLogger = false;
 
+const configError = Error("You must set this.db to a valid knex instance");
+
 class SQLDataSource extends DataSource {
   initialize(config) {
     this.context = config.context;
-    this.db = this.knex;
+    this.db = this.db || this.knex;
+
+    if (!this.db) throw configError;
+    if (this.knex) {
+      // eslint-disable-next-line
+      console.warn("this.knex has been deprecated, use this.db instead");
+    }
 
     if (DEBUG && !hasLogger) {
       hasLogger = true; // Prevent duplicate loggers
       knexTinyLogger(this.db); // Add a logging utility for debugging
     }
 
-    this.sqlCache = new SQLCache(config.cache, this.knex);
+    this.sqlCache = new SQLCache(config.cache, this.db);
     this.getBatched = query => this.sqlCache.getBatched(query);
     this.getCached = (query, ttl) => this.sqlCache.getCached(query, ttl);
     this.getBatchedAndCached = (query, ttl) =>

SQLDataSource.test.js

@@ -0,0 +1,87 @@
+const Knex = require("knex");
+const SQLDataSource = require("./SQLDataSource");
+const mockConsole = require("jest-mock-console").default;
+
+const { TEST_PG_URL } = process.env;
+
+// Create and destroy connection pool for tests
+let knex;
+
+beforeAll(() => {
+  knex = Knex({ client: "pg", connection: TEST_PG_URL });
+});
+
+afterAll(() => {
+  return knex.destroy();
+});
+
+describe("Configuration", () => {
+  test("an error is thrown if this.db is not set", () => {
+    class TestDB extends SQLDataSource {
+      getFruit() {
+        const query = this.db
+          .select("*")
+          .from("fruit")
+          .where({ id: 1 });
+        return this.getCached(query, 1);
+      }
+    }
+
+    const testDB = new TestDB();
+    const initialize = () => testDB.initialize({});
+
+    expect(initialize).toThrow(Error);
+  });
+
+  test("an warning to be logged if batching is used", () => {
+    class TestDB extends SQLDataSource {
+      constructor() {
+        super();
+        this.db = knex;
+      }
+
+      getFruit() {
+        const query = this.db
+          .select("*")
+          .from("fruit")
+          .where({ id: 1 });
+        return this.getBatchedAndCached(query, 1);
+      }
+    }
+
+    const testDB = new TestDB();
+    testDB.initialize({});
+
+    const restoreConsole = mockConsole();
+    return testDB.getFruit().then(() => {
+      expect(console.warn).toHaveBeenCalled(); // eslint-disable-line
+      restoreConsole();
+    });
+  });
+});
+
+describe("PostgreSQL", () => {
+  test("Batching and caching is configured", () => {
+    class TestDB extends SQLDataSource {
+      constructor() {
+        super();
+        this.db = knex;
+      }
+
+      getFruit() {
+        const query = this.db
+          .select("*")
+          .from("fruit")
+          .where({ id: 1 });
+        return this.getCached(query, 1);
+      }
+    }
+
+    const testDB = new TestDB();
+    testDB.initialize({});
+
+    return testDB.getFruit().then(result => {
+      expect(result).toMatchSnapshot();
+    });
+  });
+});

__snapshots__/SQLDataSource.test.js.snap

@@ -0,0 +1,10 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`PostgreSQL Batching and caching is configured 1`] = `
+Array [
+  Object {
+    "id": 1,
+    "name": "apple",
+  },
+]
+`;

index.test.js

@@ -0,0 +1,6 @@
+// NOTE: Never use .toBe() for testing values use .toEqual()
+// https://jestjs.io/docs/en/expect#toequalvalue
+
+describe("jest", () => {
+  test("is configured", () => expect(true).toBeTruthy());
+});