ai_user cookie not present after reenabling the cookie #1585 (#1632)

* ai_user cookie not present after reenabling the cookie #1585
- Add optimization to avoid always updating the cookie value

* Add missing result setting

extensions/applicationinsights-properties-js/src/Context/User.ts

@@ -56,8 +56,16 @@ export class User implements IUserContext {
      */
     public accountAcquisitionDate: string;
 
+    /**
+     * A flag indicating whether this represents a new user
+     */
     public isNewUser = false;
 
+    /**
+     * A flag indicating whether the user cookie has been set
+     */
+    public isUserCookieSet = false;
+
     constructor(config: ITelemetryConfig, core: IAppInsightsCore) {
         let _logger = safeGetLogger(core);
         let _cookieManager: ICookieMgr = safeGetCookieMgr(core);
@@ -75,6 +83,8 @@ export class User implements IUserContext {
                 const params = cookie.split(User.cookieSeparator);
                 if (params.length > 0) {
                     _self.id = params[0];
+                    // we already have a cookie
+                    _self.isUserCookieSet = !!_self.id;
                 }
             }
 
@@ -98,7 +108,7 @@ export class User implements IUserContext {
                 // set it to 365 days from now
                 // 365 * 24 * 60 * 60 = 31536000 
                 const oneYear = 31536000;
-                _cookieManager.set(_storageNamePrefix(), cookie, oneYear);
+                _self.isUserCookieSet = _cookieManager.set(_storageNamePrefix(), cookie, oneYear);
             }
 
             if (!_self.id) {
@@ -172,9 +182,12 @@ export class User implements IUserContext {
             };
 
             _self.update = (userId?: string) => {
-                let user_id = userId ? userId : _generateNewId();
-                let user_cookie = _generateNewCookie(user_id);
-                _setUserCookie(user_cookie.join(User.cookieSeparator));
+                // Optimizations to avoid setting and processing the cookie when not needed
+                if (_self.id !== userId || !_self.isUserCookieSet) {
+                    let user_id = userId ? userId : _generateNewId();
+                    let user_cookie = _generateNewCookie(user_id);
+                    _setUserCookie(user_cookie.join(User.cookieSeparator));
+                }
             };
         });
     }
@@ -197,6 +210,11 @@ export class User implements IUserContext {
         // @DynamicProtoStub -- DO NOT add any code as this will be removed during packaging
     }
 
+    /**
+     * Update or create the user cookie if cookies where previously disabled or the new userId does not match the existing value.
+     * If you pass nothing a new random user id will be created.
+     * @param userId - Specific either the current (via appInsights.context.user.id) or new id that you want to set
+     */
     public update(userId?: string) {
         // @DynamicProtoStub -- DO NOT add any code as this will be removed during packaging
     }

extensions/applicationinsights-properties-js/src/PropertiesPlugin.ts

@@ -92,14 +92,15 @@ export default class PropertiesPlugin extends BaseTelemetryPlugin implements IPr
                         }
                     }
 
-                    if (theContext.user) {
-                        theContext.user.update(theContext.user.id);
+                    let userCtx = theContext.user;
+                    if (userCtx && !userCtx.isUserCookieSet) {
+                        userCtx.update(theContext.user.id);
                     }
 
                     _processTelemetryInternal(event, itemCtx);
 
-                    if (theContext.user && theContext.user.isNewUser) {
-                        theContext.user.isNewUser = false;
+                    if (userCtx && userCtx.isNewUser) {
+                        userCtx.isNewUser = false;
                         const message = new _InternalLogMessage(_InternalMessageId.SendBrowserInfoOnUserInit, ((getNavigator()||{} as any).userAgent||""));
                         itemCtx.diagLog().logInternalMessage(LoggingSeverity.CRITICAL, message);
                     }

shared/AppInsightsCommon/src/Interfaces/Context/IUser.ts

@@ -36,6 +36,11 @@ export interface IUser {
      * A flag indicating whether this represents a new user
      */
     isNewUser?: boolean;
+
+    /**
+     * A flag indicating whether the user cookie has been set
+     */
+    isUserCookieSet?: boolean;
 }
 
 export interface IUserContext extends IUser {

shared/AppInsightsCore/src/JavaScriptSDK.Interfaces/ICookieMgr.ts

@@ -20,8 +20,9 @@ export interface ICookieMgr {
      * @param maxAgeSec - [optional] The maximum number of SECONDS that this cookie should survive
      * @param domain - [optional] The domain to set for the cookie
      * @param path - [optional] Path to set for the cookie, if not supplied will default to "/"
+     * @returns - True if the cookie was set otherwise false (Because cookie usage is not enabled or available)
      */
-    set(name: string, value: string, maxAgeSec?: number, domain?: string, path?: string): void;
+    set(name: string, value: string, maxAgeSec?: number, domain?: string, path?: string): boolean;
 
     /**
      * Get the value of the named cookie
@@ -34,17 +35,19 @@ export interface ICookieMgr {
      * Note: Not using "delete" as the name because it's a reserved word which would cause issues on older browsers
      * @param name - The name of the cookie
      * @param path - [optional] Path to set for the cookie, if not supplied will default to "/"
+     * @returns - True if the cookie was marked for deletion otherwise false (Because cookie usage is not enabled or available)
      */
-    del(name: string, path?: string): void;
+    del(name: string, path?: string): boolean;
 
     /**
      * Purge the cookie from the system if cookie support is available, this function ignores the enabled setting of the manager
      * so any cookie will be removed.
      * Note: Not using "delete" as the name because it's a reserved word which would cause issues on older browsers
      * @param name - The name of the cookie
      * @param path - [optional] Path to set for the cookie, if not supplied will default to "/"
+     * @returns - True if the cookie was marked for deletion otherwise false (Because cookie usage is not available)
      */
-    purge(name: string, path?: string): void;
+    purge(name: string, path?: string): boolean;
 }
 
 /**

shared/AppInsightsCore/src/JavaScriptSDK/CookieMgr.ts

@@ -136,6 +136,7 @@ export function createCookieMgr(rootConfig?: IConfiguration, logger?: IDiagnosti
             _enabled = value !== false;
         },
         set: (name: string, value: string, maxAgeSec?: number, domain?: string, path?: string) => {
+            let result = false;
             if (_isMgrEnabled(cookieMgr)) {
                 let values: any = {};
                 let theValue = strTrim(value || strEmpty);
@@ -189,7 +190,10 @@ export function createCookieMgr(rootConfig?: IConfiguration, logger?: IDiagnosti
 
                 let setCookieFn = cookieMgrConfig.setCookie || _setCookieValue;
                 setCookieFn(name, _formatCookieValue(theValue, values));
+                result = true;
             }
+
+            return result;
         },
         get: (name: string): string => {
             let value = strEmpty
@@ -200,12 +204,16 @@ export function createCookieMgr(rootConfig?: IConfiguration, logger?: IDiagnosti
             return value;
         },
         del: (name: string, path?: string) => {
+            let result = false;
             if (_isMgrEnabled(cookieMgr)) {
                 // Only remove the cookie if the manager and cookie support has not been disabled
-                cookieMgr.purge(name, path);
+                result = cookieMgr.purge(name, path);
             }
+
+            return result;
         },
         purge: (name: string, path?: string) => {
+            let result = false;
             if (areCookiesSupported(logger)) {
                 // Setting the expiration date in the past immediately removes the cookie
                 let values = {
@@ -220,7 +228,10 @@ export function createCookieMgr(rootConfig?: IConfiguration, logger?: IDiagnosti
 
                 let delCookie = cookieMgrConfig.delCookie || _setCookieValue;
                 delCookie(name, _formatCookieValue(strEmpty, values));
+                result = true;
             }
+
+            return result;
         }
     };