address review comments

oppia · seanlip · Jul 2, 2019 · Jun 16, 2019 · Jun 17, 2019 · Jun 17, 2019
commit 997257bff5c669800b4eba71c086652a55ce27f1
diff --git a/core/templates/dev/head/pages/Base.ts b/core/templates/dev/head/pages/Base.ts
@@ -53,7 +53,7 @@ oppia.controller('Base', [
 
     $scope.isBackgroundMaskActive = BackgroundMaskService.isMaskActive;
 
-    CsrfTokenService.getToken();
+    CsrfTokenService.initializeToken();
 
     // Listener function to catch the change in language preference.
     $rootScope.$on('$translateChangeSuccess', function(evt, response) {

diff --git a/core/templates/dev/head/pages/admin-page/admin-page.directive.ts b/core/templates/dev/head/pages/admin-page/admin-page.directive.ts
@@ -105,7 +105,7 @@ oppia.directive('adminPage', ['UrlInterpolationService',
           ctrl.isRolesTabOpen = AdminRouterService.isRolesTabOpen;
           ctrl.isMiscTabOpen = AdminRouterService.isMiscTabOpen;
 
-          CsrfTokenService.getToken();
+          CsrfTokenService.initializeToken();
 
           ctrl.setStatusMessage = function(statusMessage) {
             ctrl.statusMessage = statusMessage;

diff --git a/core/templates/dev/head/services/CsrfTokenService.ts b/core/templates/dev/head/services/CsrfTokenService.ts
@@ -18,33 +18,39 @@
 
 oppia.factory('CsrfTokenService', [function() {
   var token = null;
+  var tokenPromise = null;
+
   return {
-    setToken: function(csrfToken) {
-      token = csrfToken;
+    initializeToken: function() {
+      if (token !== null) {
+        throw Error('Token already set');
+      }
+      tokenPromise = new Promise(function(resolve, reject) {
+        // We use jQuery here instead of Angular's $http, since the latter
+        // creates a circular dependency.
+        $.ajax({
+          url: '/csrfhandler',
+          type: 'GET',
+          dataType: 'text',
+          dataFilter: function(data) {
+            // Remove the XSSI prefix.
+            var transformedData = data.substring(5);
+            return JSON.parse(transformedData);
+          },
+        }).done(function(response) {
+          token = response.token;
+          resolve(response.token);
+        });
+      });
     },
+
     getToken: function() {
-      // We use jQuery here instead of Angular's $http, since the latter
-      // creates a circular dependency.
-      return new Promise(function(resolve, reject) {
-        if (token) {
+      if (token) {
+        return new Promise(function(resolve, reject) {
           resolve(token);
-        } else {
-          // Fetch token if it isn't already available.
-          $.ajax({
-            url: '/csrfhandler',
-            type: 'GET',
-            dataType: 'text',
-            dataFilter: function(data) {
-              // Remove the XSSI prefix.
-              var transformedData = data.substring(5);
-              return JSON.parse(transformedData);
-            },
-          }).done(function(response) {
-            token = response.token;
-            resolve(response.token);
-          });
-        }
-      });
+        });
+      }
+      return tokenPromise;
     }
   };
 }]);
@@ -22,15 +22,31 @@ describe('Csrf Token Service', function() {
   var CsrfTokenService, $httpBackend;
 
   beforeEach(angular.mock.module('oppia'));
-  beforeEach(angular.mock.inject(function($injector) {
+  beforeEach(angular.mock.inject(function($injector, $q) {
     CsrfTokenService = $injector.get('CsrfTokenService');
     $httpBackend = $injector.get('$httpBackend');
+
+    spyOn($, 'ajax').and.callFake(function() {
+      var deferred = $q.defer();
+      deferred.resolve('sample-csrf-token');
+      return deferred.promise;
+    });
   }));
 
   it('should correctly set the csrf token', function() {
-    CsrfTokenService.setToken('sample csrf token');
+    CsrfTokenService.initializeToken();
+
     CsrfTokenService.getToken().then(function(token) {
-      expect(token).toBe('sample csrf token');
+      expect(token).toBe('sample-csrf-token');
     });
   });
+
+  it('should error if initialize is called more than once',
+    function() {
+      CsrfTokenService.initializeToken();
+
+      expect(CsrfTokenService.initializeToken()).toThrowError(
+        'Token already set');
+    }
+  );
 });