ui: Change signature of request/respond methods to take serialized an…

…d unserialized data (#6285)

* Change signature of request/respond methods to take serialized,data

Both `requestFor` and `respondFor` methods now take both serialized data
and unserialized data.

We also leave it upto the adapter author to add (or omit) the serialized
data from the request payload, instead of it being automatically added
within the http client.

Included here is a small refactor to dry out the http adapter slightly.

Tests where changed only slightly to follow the new method signature.

We spotted missing tests for token cloning, and the self API methods,
so whilst refactoring this we added the test for those while we are here.

* Drys out custom adapter methods and irons out a few wrinkles

Custom adapter methods (like clone and self) needed the full promise
chain written out. This can be repetitive and harder to refactor things
and make things consitent. We've added a possibly temporary
`application.request` method here to dry this out slightly. This may be
moved in the future once we decide exactly where it should live.

Now everything is much simpler, a few wrinkles are more visible, so we
iron out these wrinkles to make everything as consistent as possible.

ui-v2/app/adapters/acl.js

@@ -1,9 +1,6 @@
 import Adapter, { DATACENTER_QUERY_PARAM as API_DATACENTER_KEY } from './application';
-import { get } from '@ember/object';
-import EmberError from '@ember/error';
-import { PRIMARY_KEY, SLUG_KEY } from 'consul-ui/models/acl';
+import { SLUG_KEY } from 'consul-ui/models/acl';
 import { FOREIGN_KEY as DATACENTER_KEY } from 'consul-ui/models/dc';
-import { OK as HTTP_OK, UNAUTHORIZED as HTTP_UNAUTHORIZED } from 'consul-ui/utils/http/status';
 
 export default Adapter.extend({
   requestForQuery: function(request, { dc, index }) {
@@ -25,65 +22,45 @@ export default Adapter.extend({
       ${{ index }}
     `;
   },
-  requestForCreateRecord: function(request, data) {
+  requestForCreateRecord: function(request, serialized, data) {
     // https://www.consul.io/api/acl.html#create-acl-token
     return request`
       PUT /v1/acl/create?${{ [API_DATACENTER_KEY]: data[DATACENTER_KEY] }}
+
+      ${serialized}
     `;
   },
-  requestForUpdateRecord: function(request, data) {
-    // the id is in the data, don't add it in here
+  requestForUpdateRecord: function(request, serialized, data) {
+    // the id is in the data, don't add it into the URL
     // https://www.consul.io/api/acl.html#update-acl-token
     return request`
       PUT /v1/acl/update?${{ [API_DATACENTER_KEY]: data[DATACENTER_KEY] }}
+
+      ${serialized}
     `;
   },
-  requestForDeleteRecord: function(request, data) {
+  requestForDeleteRecord: function(request, serialized, data) {
     // https://www.consul.io/api/acl.html#delete-acl-token
     return request`
       PUT /v1/acl/destroy/${data[SLUG_KEY]}?${{ [API_DATACENTER_KEY]: data[DATACENTER_KEY] }}
     `;
   },
-  requestForCloneRecord: function(request, data) {
+  requestForCloneRecord: function(request, serialized, data) {
     // https://www.consul.io/api/acl.html#clone-acl-token
     return request`
       PUT /v1/acl/clone/${data[SLUG_KEY]}?${{ [API_DATACENTER_KEY]: data[DATACENTER_KEY] }}
     `;
   },
   clone: function(store, type, id, snapshot) {
-    const serializer = store.serializerFor(type.modelName);
-    const unserialized = this.snapshotToJSON(snapshot, type);
-    const serialized = serializer.serialize(snapshot, {});
-    return get(this, 'client')
-      .request(request => this.requestForClone(request, unserialized), serialized)
-      .then(respond => serializer.respondForQueryRecord(respond, unserialized));
-  },
-  handleResponse: function(status, headers, payload, requestData) {
-    let response = payload;
-    const method = requestData.method;
-    if (status === HTTP_OK) {
-      const url = this.parseURL(requestData.url);
-      switch (true) {
-        case response === true:
-          response = this.handleBooleanResponse(url, response, PRIMARY_KEY, SLUG_KEY);
-          break;
-        case this.isQueryRecord(url):
-          response = this.handleSingleResponse(url, response[0], PRIMARY_KEY, SLUG_KEY);
-          break;
-        case this.isUpdateRecord(url, method):
-        case this.isCreateRecord(url, method):
-        case this.isCloneRecord(url, method):
-          response = this.handleSingleResponse(url, response, PRIMARY_KEY, SLUG_KEY);
-          break;
-        default:
-          response = this.handleBatchResponse(url, response, PRIMARY_KEY, SLUG_KEY);
-      }
-    } else if (status === HTTP_UNAUTHORIZED) {
-      const e = new EmberError();
-      e.code = status;
-      e.message = payload;
-      throw e;
-    }
-    return this._super(status, headers, response, requestData);
+    return this.request(
+      function(adapter, request, serialized, unserialized) {
+        return adapter.requestForCloneRecord(request, serialized, unserialized);
+      },
+      function(serializer, response, serialized, unserialized) {
+        return serializer.respondForCreateRecord(response, serialized, unserialized);
+      },
+      snapshot,
+      type.modelName
+    );
   },
 });

ui-v2/app/adapters/application.js

@@ -1,8 +1,44 @@
 import Adapter from './http';
 import { inject as service } from '@ember/service';
-
+import { get } from '@ember/object';
 export const DATACENTER_QUERY_PARAM = 'dc';
 export default Adapter.extend({
   repo: service('settings'),
   client: service('client/http'),
+  // TODO: kinda protected for the moment
+  // decide where this should go either read/write from http
+  // should somehow use this or vice versa
+  request: function(req, resp, obj, modelName) {
+    const client = get(this, 'client');
+    const store = get(this, 'store');
+    const adapter = this;
+
+    let unserialized, serialized;
+    const serializer = store.serializerFor(modelName);
+    // workable way to decide whether this is a snapshot
+    // Snapshot is private so we can't do instanceof here
+    if (obj.constructor.name === 'Snapshot') {
+      unserialized = obj.attributes();
+      serialized = serializer.serialize(obj, {});
+    } else {
+      unserialized = obj;
+      serialized = unserialized;
+    }
+
+    return client
+      .request(function(request) {
+        return req(adapter, request, serialized, unserialized);
+      })
+      .catch(function(e) {
+        return adapter.error(e);
+      })
+      .then(function(response) {
+        // TODO: When HTTPAdapter:responder changes, this will also need to change
+        return resp(serializer, response, serialized, unserialized);
+      });
+    // TODO: Potentially add specific serializer errors here
+    // .catch(function(e) {
+    //   return Promise.reject(e);
+    // });
+  },
 });

ui-v2/app/adapters/http.js

@@ -1,4 +1,5 @@
 import Adapter from 'ember-data/adapter';
+import { get } from '@ember/object';
 import {
   AbortError,
   TimeoutError,
@@ -10,12 +11,47 @@ import {
   InvalidError,
   AdapterError,
 } from 'ember-data/adapters/errors';
-import { get } from '@ember/object';
 
+// TODO: This is a little skeleton cb function
+// is to be replaced soon with something slightly more involved
+const responder = function(response) {
+  return response;
+};
+const read = function(adapter, serializer, client, type, query) {
+  return client
+    .request(function(request) {
+      return adapter[`requestFor${type}`](request, query);
+    })
+    .catch(function(e) {
+      return adapter.error(e);
+    })
+    .then(function(response) {
+      return serializer[`respondFor${type}`](responder(response), query);
+    });
+  // TODO: Potentially add specific serializer errors here
+  // .catch(function(e) {
+  //   return Promise.reject(e);
+  // });
+};
+const write = function(adapter, serializer, client, type, snapshot) {
+  const unserialized = snapshot.attributes();
+  const serialized = serializer.serialize(snapshot, {});
+  return client
+    .request(function(request) {
+      return adapter[`requestFor${type}`](request, serialized, unserialized);
+    })
+    .catch(function(e) {
+      return adapter.error(e);
+    })
+    .then(function(response) {
+      return serializer[`respondFor${type}`](responder(response), serialized, unserialized);
+    });
+  // TODO: Potentially add specific serializer errors here
+  // .catch(function(e) {
+  //   return Promise.reject(e);
+  // });
+};
 export default Adapter.extend({
-  snapshotToJSON: function(snapshot, type, options) {
-    return snapshot.attributes();
-  },
   error: function(err) {
     const errors = [
       {
@@ -62,51 +98,45 @@ export default Adapter.extend({
     throw error;
   },
   query: function(store, type, query) {
-    const serializer = store.serializerFor(type.modelName);
-    return get(this, 'client')
-      .request(request => this.requestForQuery(request, query))
-      .catch(e => this.error(e))
-      .then(respond => serializer.respondForQuery(respond, query, type));
+    return read(this, store.serializerFor(type.modelName), get(this, 'client'), 'Query', query);
   },
   queryRecord: function(store, type, query) {
-    const serializer = store.serializerFor(type.modelName);
-    return get(this, 'client')
-      .request(request => this.requestForQueryRecord(request, query))
-      .catch(e => this.error(e))
-      .then(respond => serializer.respondForQueryRecord(respond, query));
+    return read(
+      this,
+      store.serializerFor(type.modelName),
+      get(this, 'client'),
+      'QueryRecord',
+      query
+    );
   },
   findAll: function(store, type) {
-    const serializer = store.serializerFor(type.modelName);
-    return get(this, 'client')
-      .request(request => this.requestForFindAll(request))
-      .catch(e => this.error(e))
-      .then(respond => serializer.respondForFindAll(respond));
+    return read(this, store.serializerFor(type.modelName), get(this, 'client'), 'FindAll');
   },
   createRecord: function(store, type, snapshot) {
-    const serializer = store.serializerFor(type.modelName);
-    const unserialized = this.snapshotToJSON(snapshot, type);
-    const serialized = serializer.serialize(snapshot, {});
-    return get(this, 'client')
-      .request(request => this.requestForCreateRecord(request, unserialized), serialized)
-      .catch(e => this.error(e))
-      .then(respond => serializer.respondForCreateRecord(respond, unserialized, type));
+    return write(
+      this,
+      store.serializerFor(type.modelName),
+      get(this, 'client'),
+      'CreateRecord',
+      snapshot
+    );
   },
   updateRecord: function(store, type, snapshot) {
-    const serializer = store.serializerFor(type.modelName);
-    const unserialized = this.snapshotToJSON(snapshot, type);
-    const serialized = serializer.serialize(snapshot, {});
-    return get(this, 'client')
-      .request(request => this.requestForUpdateRecord(request, unserialized), serialized)
-      .catch(e => this.error(e))
-      .then(respond => serializer.respondForUpdateRecord(respond, unserialized, type));
+    return write(
+      this,
+      store.serializerFor(type.modelName),
+      get(this, 'client'),
+      'UpdateRecord',
+      snapshot
+    );
   },
   deleteRecord: function(store, type, snapshot) {
-    const serializer = store.serializerFor(type.modelName);
-    const unserialized = this.snapshotToJSON(snapshot, type);
-    const serialized = serializer.serialize(snapshot, {});
-    return get(this, 'client')
-      .request(request => this.requestForDeleteRecord(request, unserialized), serialized)
-      .catch(e => this.error(e))
-      .then(respond => serializer.respondForDeleteRecord(respond, unserialized, type));
+    return write(
+      this,
+      store.serializerFor(type.modelName),
+      get(this, 'client'),
+      'DeleteRecord',
+      snapshot
+    );
   },
 });

ui-v2/app/adapters/intention.js

@@ -19,18 +19,22 @@ export default Adapter.extend({
       ${{ index }}
     `;
   },
-  requestForCreateRecord: function(request, data) {
+  requestForCreateRecord: function(request, serialized, data) {
     // TODO: need to make sure we remove dc
     return request`
       POST /v1/connect/intentions?${{ [API_DATACENTER_KEY]: data[DATACENTER_KEY] }}
+
+      ${serialized}
     `;
   },
-  requestForUpdateRecord: function(request, data) {
+  requestForUpdateRecord: function(request, serialized, data) {
     return request`
       PUT /v1/connect/intentions/${data[SLUG_KEY]}?${{ [API_DATACENTER_KEY]: data[DATACENTER_KEY] }}
+
+      ${serialized}
     `;
   },
-  requestForDeleteRecord: function(request, data) {
+  requestForDeleteRecord: function(request, serialized, data) {
     return request`
       DELETE /v1/connect/intentions/${data[SLUG_KEY]}?${{
       [API_DATACENTER_KEY]: data[DATACENTER_KEY],

ui-v2/app/adapters/kv.js

@@ -28,19 +28,25 @@ export default Adapter.extend({
       ${{ index }}
     `;
   },
-  requestForCreateRecord: function(request, data) {
+  // TODO: Should we replace text/plain here with x-www-form-encoded?
+  // See https://github.com/hashicorp/consul/issues/3804
+  requestForCreateRecord: function(request, serialized, data) {
     return request`
       PUT /v1/kv/${keyToArray(data[SLUG_KEY])}?${{ [API_DATACENTER_KEY]: data[DATACENTER_KEY] }}
-      Content-Type: application/x-www-form-urlencoded
+      Content-Type: text/plain; charset=utf-8
+
+      ${serialized}
     `;
   },
-  requestForUpdateRecord: function(request, data) {
+  requestForUpdateRecord: function(request, serialized, data) {
     return request`
       PUT /v1/kv/${keyToArray(data[SLUG_KEY])}?${{ [API_DATACENTER_KEY]: data[DATACENTER_KEY] }}
-      Content-Type: application/x-www-form-urlencoded
+      Content-Type: text/plain; charset=utf-8
+
+      ${serialized}
     `;
   },
-  requestForDeleteRecord: function(request, data) {
+  requestForDeleteRecord: function(request, serialized, data) {
     let recurse;
     if (isFolder(data[SLUG_KEY])) {
       recurse = null;

ui-v2/app/adapters/policy.js

@@ -21,17 +21,21 @@ export default Adapter.extend({
       ${{ index }}
     `;
   },
-  requestForCreateRecord: function(request, data) {
+  requestForCreateRecord: function(request, serialized, data) {
     return request`
       PUT /v1/acl/policy?${{ [API_DATACENTER_KEY]: data[DATACENTER_KEY] }}
+
+      ${serialized}
     `;
   },
-  requestForUpdateRecord: function(request, data) {
+  requestForUpdateRecord: function(request, serialized, data) {
     return request`
       PUT /v1/acl/policy/${data[SLUG_KEY]}?${{ [API_DATACENTER_KEY]: data[DATACENTER_KEY] }}
+
+      ${serialized}
     `;
   },
-  requestForDeleteRecord: function(request, data) {
+  requestForDeleteRecord: function(request, serialized, data) {
     return request`
       DELETE /v1/acl/policy/${data[SLUG_KEY]}?${{ [API_DATACENTER_KEY]: data[DATACENTER_KEY] }}
     `;

ui-v2/app/adapters/role.js

@@ -21,17 +21,21 @@ export default Adapter.extend({
       ${{ index }}
     `;
   },
-  requestForCreateRecord: function(request, data) {
+  requestForCreateRecord: function(request, serialized, data) {
     return request`
       PUT /v1/acl/role?${{ [API_DATACENTER_KEY]: data[DATACENTER_KEY] }}
+
+      ${serialized}
     `;
   },
-  requestForUpdateRecord: function(request, data) {
+  requestForUpdateRecord: function(request, serialized, data) {
     return request`
       PUT /v1/acl/role/${data[SLUG_KEY]}?${{ [API_DATACENTER_KEY]: data[DATACENTER_KEY] }}
+
+      ${serialized}
     `;
   },
-  requestForDeleteRecord: function(request, data) {
+  requestForDeleteRecord: function(request, serialized, data) {
     return request`
       DELETE /v1/acl/role/${data[SLUG_KEY]}?${{ [API_DATACENTER_KEY]: data[DATACENTER_KEY] }}
     `;

ui-v2/app/adapters/session.js

@@ -23,7 +23,7 @@ export default Adapter.extend({
       ${{ index }}
     `;
   },
-  requestForDeleteRecord: function(request, data) {
+  requestForDeleteRecord: function(request, serialized, data) {
     return request`
       PUT /v1/session/destroy/${data[SLUG_KEY]}?${{ [API_DATACENTER_KEY]: data[DATACENTER_KEY] }}
     `;