Mapper: clarify error message when mapping info not found

Author: jorgebay

lib/mapping/mapper.js

@@ -92,11 +92,17 @@ class Mapper {
 
       if (mappingInfo === undefined) {
         if (!this.client.keyspace) {
-          throw new Error(
-            'You must set the Client keyspace or specify the keyspace of the model in the MappingOptions');
+          throw new Error(`No mapping information found for model '${name}'. ` +
+            `Mapper is unable to create default mappings without setting the keyspace`);
         }
 
         mappingInfo = ModelMappingInfo.createDefault(name, this.client.keyspace);
+        this.client.log('info', `Mapping information for model '${name}' not found, creating default mapping. ` +
+          `Keyspace: ${mappingInfo.keyspace}; Table: ${mappingInfo.tables[0].name}.`);
+      } else {
+        this.client.log('info', `Creating model mapper for '${name}' using mapping information. Keyspace: ${
+          mappingInfo.keyspace}; Table${mappingInfo.tables.length > 1? 's' : ''}: ${
+          mappingInfo.tables.map(t => t.name)}.`);
       }
 
       modelMapper = new ModelMapper(name, new MappingHandler(this.client, mappingInfo));

test/unit/mapping/mapper-tests.js

@@ -16,8 +16,10 @@
 
 'use strict';
 
-const assert = require('assert');
+const { assert } = require('chai');
+const sinon = require('sinon');
 const Mapper = require('../../../lib/mapping/mapper');
+const ModelMapper = require('../../../lib/mapping/model-mapper');
 const helper = require('../../test-helper');
 const mapperTestHelper = require('./mapper-unit-test-helper');
 
@@ -60,7 +62,40 @@ describe('Mapper', () => {
     it('should validate that the client keyspace is set when mapping options has not been specified', () => {
       const mapper = new Mapper({});
       assert.throws(() => mapper.forModel('Sample'),
-        /You must set the Client keyspace or specify the keyspace of the model in the MappingOptions/);
+        /No mapping information found for model 'Sample'\. Mapper .+ without setting the keyspace$/);
+    });
+
+    it('should log when creating a ModelMapper instance using mapping information', () => {
+      const client = sinon.spy({ log: () => {} });
+      const mapper = new Mapper(client, { models: { 'T': { tables: ['t1', 't2'], keyspace: 'ks1' }}});
+
+      // call forModel() multiple times
+      for (let i = 0; i < 10; i++) {
+        const m = mapper.forModel('T');
+        assert.instanceOf(m, ModelMapper);
+      }
+
+      // Only should be logged once
+      assert.ok(client.log.calledOnce);
+      const args = client.log.getCall(0).args;
+      assert.strictEqual(args[0], 'info');
+      assert.match(args[1], /Creating model mapper for 'T' using mapping information. Keyspace: ks1; Tables: t1,t2.$/);
+    });
+
+    it('should log when creating a ModelMapper instance using defaults', () => {
+      const client = sinon.spy({ log: () => {}, keyspace: 'abc' });
+      const mapper = new Mapper(client);
+
+      // call forModel() multiple times
+      for (let i = 0; i < 10; i++) {
+        const m = mapper.forModel('user');
+        assert.instanceOf(m, ModelMapper);
+      }
+
+      assert.ok(client.log.calledOnce);
+      const args = client.log.getCall(0).args;
+      assert.strictEqual(args[0], 'info');
+      assert.match(args[1], /Mapping information for model 'user' not found, creating .+Keyspace: abc; Table: user.$/);
     });
   });
 

test/unit/mapping/model-mapper-mutation-tests.js

@@ -66,6 +66,7 @@ describe('ModelMapper', () => {
       const client = {
         connect: () => Promise.resolve(),
         keyspace: 'ks1',
+        log: () => {},
         metadata: {
           getTable: () => Promise.reject(error)
         }
@@ -99,7 +100,7 @@ describe('ModelMapper', () => {
       }
     ]));
 
-    it('should warn when cache reaches 100 different queries', () => {
+    it('should warn when cache reaches 100 different queries', async () => {
       const clientInfo = mapperTestHelper.getClient(['id1'], [ 1 ], 'ks1');
       const modelMapper = mapperTestHelper.getModelMapper(clientInfo);
 
@@ -110,16 +111,17 @@ describe('ModelMapper', () => {
         promises.push(modelMapper.insert({ id1: 1, [`col${i % (cacheHighWaterMark-1)}`]: 1}));
       }
 
-      return Promise.all(promises)
-        // No warnings logged when there are 99 different queries
-        .then(() => assert.strictEqual(clientInfo.logMessages.length, 0))
-        // One more query
-        .then(() => modelMapper.insert({ id1: 1, anotherColumn: 1 }))
-        .then(() => {
-          assert.strictEqual(clientInfo.logMessages.length, 1);
-          assert.strictEqual(clientInfo.logMessages[0].level, 'warning');
-          helper.assertContains(clientInfo.logMessages[0].message, `ModelMapper cache reached ${cacheHighWaterMark}`);
-        });
+      await Promise.all(promises);
+
+      // No warnings logged when there are 99 different queries
+      assert.strictEqual(clientInfo.logMessages.filter(l => l.level === 'warning').length, 0);
+
+      // One more query
+      await modelMapper.insert({ id1: 1, anotherColumn: 1 });
+
+      const warnings = clientInfo.logMessages.filter(l => l.level === 'warning');
+      assert.strictEqual(warnings.length, 1);
+      helper.assertContains(warnings[0].message, `ModelMapper cache reached ${cacheHighWaterMark}`);
     });
 
     it('should use the mapping function in the insert values', () => testQueries({

test/unit/mapping/model-mapper-select-tests.js

@@ -285,7 +285,7 @@ describe('ModelMapper', () => {
   });
 
   describe('#mapWithQuery', () => {
-    it('should warn when cache reaches 100 different queries', () => {
+    it('should warn when cache reaches 100 different queries', async () => {
       const clientInfo = mapperTestHelper.getClient(['id1'], [ 1 ], 'ks1', emptyResponse);
       const modelMapper = mapperTestHelper.getModelMapper(clientInfo);
 
@@ -297,18 +297,19 @@ describe('ModelMapper', () => {
         promises.push(executor());
       }
 
-      return Promise.all(promises)
-        // No warnings logged when there are 99 different queries
-        .then(() => assert.strictEqual(clientInfo.logMessages.length, 0))
-        // One more query
-        .then(() => modelMapper.mapWithQuery(`query-limit`, () => [])())
-        .then(() => {
-          assert.strictEqual(clientInfo.logMessages.length, 1);
-          assert.strictEqual(clientInfo.logMessages[0].level, 'warning');
-          assert.strictEqual(clientInfo.logMessages[0].message,
-            `Custom queries cache reached ${cacheHighWaterMark} items, this could be caused by ` +
-            `hard-coding parameter values inside the query, which should be avoided`);
-        });
+      await Promise.all(promises);
+
+      // No warnings logged when there are 99 different queries
+      assert.strictEqual(clientInfo.logMessages.filter(l => l.level === 'warning').length, 0);
+
+      // One more query
+      await modelMapper.mapWithQuery(`query-limit`, () => [])();
+
+      const warnings = clientInfo.logMessages.filter(l => l.level === 'warning');
+      assert.strictEqual(warnings.length, 1);
+      assert.strictEqual(warnings[0].message,
+        `Custom queries cache reached ${cacheHighWaterMark} items, this could be caused by ` +
+        `hard-coding parameter values inside the query, which should be avoided`);
     });
   });
 });