fix: fixed bug with rollout when starting with 100 instead of 0 (#1004)

DevCycleHQ · Dec 3, 2024 · 0df1269 · 0df1269
1 parent f2f5eb4
commit 0df1269
Show file tree

Hide file tree

Showing 7 changed files with 186 additions and 11 deletions.
diff --git a/dev-apps/nextjs/app-router/app/devcycle.tsx b/dev-apps/nextjs/app-router/app/devcycle.tsx
@@ -9,15 +9,16 @@ const getUserIdentity = async () => {
     }
 }
 
-const { getVariableValue, getClientContext } = setupDevCycle(
+const { getVariableValue, getClientContext } = setupDevCycle({
+    serverSDKKey: process.env.NEXT_PUBLIC_DEVCYCLE_SERVER_SDK_KEY ?? '',
     // SDK Key. This will be public and sent to the client, so you MUST use the client SDK key.
-    process.env.NEXT_PUBLIC_DEVCYCLE_CLIENT_SDK_KEY ?? '',
+    clientSDKKey: process.env.NEXT_PUBLIC_DEVCYCLE_CLIENT_SDK_KEY ?? '',
     // pass your method for getting the user identity
-    getUserIdentity,
+    userGetter: getUserIdentity,
     // pass any options you want to use for the DevCycle SDK
-    {
+    options: {
         enableStreaming: process.env.NEXT_PUBLIC_ENABLE_STREAMING === '1',
     },
-)
+})
 
 export { getVariableValue, getClientContext, getUserIdentity }
diff --git a/lib/shared/bucketing/__tests__/bucketing.test.ts b/lib/shared/bucketing/__tests__/bucketing.test.ts
@@ -1,5 +1,5 @@
 /* eslint-disable max-len */
-import { Audience, FeatureType, Rollout } from '@devcycle/types'
+import { Audience, FeatureType, PublicRollout, Rollout } from '@devcycle/types'
 import {
     generateBoundedHashes,
     decideTargetVariation,
@@ -1415,6 +1415,173 @@ describe('Rollout Logic', () => {
                 doesUserPassRollout({ rollout, boundedHash: 0.9 }),
             ).toBeFalsy()
         })
+
+        it('should handle stepped rollout with 0% start and 100% later stage', () => {
+            const rollout: PublicRollout = {
+                type: 'stepped',
+                startDate: new Date(
+                    new Date().getTime() - 1000 * 60 * 60 * 24 * 7,
+                ),
+                startPercentage: 0,
+                stages: [
+                    {
+                        type: 'discrete',
+                        date: new Date(
+                            new Date().getTime() + 1000 * 60 * 60 * 24 * 7,
+                        ),
+                        percentage: 1,
+                    },
+                ],
+            }
+
+            // Before next stage - should be 0%
+            jest.useFakeTimers().setSystemTime(new Date())
+            for (let i = 0; i < 100; i++) {
+                expect(
+                    doesUserPassRollout({ rollout, boundedHash: i / 100 }),
+                ).toBeFalsy()
+            }
+
+            // After 100% stage - should pass all users
+            jest.useFakeTimers().setSystemTime(
+                new Date(new Date().getTime() + 1000 * 60 * 60 * 24 * 8),
+            )
+            for (let i = 0; i < 100; i++) {
+                expect(
+                    doesUserPassRollout({ rollout, boundedHash: i / 100 }),
+                ).toBeTruthy()
+            }
+
+            jest.useRealTimers()
+        })
+
+        it('should handle stepped rollout with 50% start and 100% later stage', () => {
+            const rollout: PublicRollout = {
+                type: 'stepped',
+                startDate: new Date(
+                    new Date().getTime() - 1000 * 60 * 60 * 24 * 7,
+                ),
+                startPercentage: 0.5,
+                stages: [
+                    {
+                        type: 'discrete',
+                        date: new Date(
+                            new Date().getTime() + 1000 * 60 * 60 * 24 * 7,
+                        ),
+                        percentage: 1,
+                    },
+                ],
+            }
+
+            // Before next stage - should be 50%
+            jest.useFakeTimers().setSystemTime(new Date())
+            expect(
+                doesUserPassRollout({ rollout, boundedHash: 0.49 }),
+            ).toBeTruthy()
+            expect(
+                doesUserPassRollout({ rollout, boundedHash: 0.51 }),
+            ).toBeFalsy()
+
+            // After 100% stage - should pass all users
+            jest.useFakeTimers().setSystemTime(
+                new Date(new Date().getTime() + 1000 * 60 * 60 * 24 * 8),
+            )
+            for (let i = 0; i < 100; i++) {
+                expect(
+                    doesUserPassRollout({ rollout, boundedHash: i / 100 }),
+                ).toBeTruthy()
+            }
+
+            jest.useRealTimers()
+        })
+
+        it('should handle stepped rollout with 100% start and 0% later stage', () => {
+            const rollout: PublicRollout = {
+                type: 'stepped',
+                startDate: new Date(
+                    new Date().getTime() - 1000 * 60 * 60 * 24 * 7,
+                ),
+                startPercentage: 1,
+                stages: [
+                    {
+                        type: 'discrete',
+                        date: new Date(
+                            new Date().getTime() + 1000 * 60 * 60 * 24 * 7,
+                        ),
+                        percentage: 0,
+                    },
+                ],
+            }
+
+            // Before next stage - should be 100%
+            jest.useFakeTimers().setSystemTime(new Date())
+            for (let i = 0; i < 100; i++) {
+                expect(
+                    doesUserPassRollout({ rollout, boundedHash: i / 100 }),
+                ).toBeTruthy()
+            }
+
+            // After 0% stage - should fail all users
+            jest.useFakeTimers().setSystemTime(
+                new Date(new Date().getTime() + 1000 * 60 * 60 * 24 * 8),
+            )
+            for (let i = 0; i < 100; i++) {
+                expect(
+                    doesUserPassRollout({ rollout, boundedHash: i / 100 }),
+                ).toBeFalsy()
+            }
+
+            jest.useRealTimers()
+        })
+
+        it('should handle stepped rollout with a future start date', () => {
+            const rollout: PublicRollout = {
+                type: 'stepped',
+                startDate: new Date(
+                    new Date().getTime() + 1000 * 60 * 60 * 24 * 7,
+                ),
+                startPercentage: 1,
+                stages: [
+                    {
+                        type: 'discrete',
+                        date: new Date(
+                            new Date().getTime() + 1000 * 60 * 60 * 24 * 14,
+                        ),
+                        percentage: 0,
+                    },
+                ],
+            }
+
+            // Before next stage - should be no one
+            jest.useFakeTimers().setSystemTime(new Date())
+            for (let i = 0; i < 100; i++) {
+                expect(
+                    doesUserPassRollout({ rollout, boundedHash: i / 100 }),
+                ).toBeFalsy()
+            }
+
+            // After start date - should pass all users
+            jest.useFakeTimers().setSystemTime(
+                new Date(new Date().getTime() + 1000 * 60 * 60 * 24 * 8),
+            )
+            for (let i = 0; i < 100; i++) {
+                expect(
+                    doesUserPassRollout({ rollout, boundedHash: i / 100 }),
+                ).toBeTruthy()
+            }
+
+            // After next stage - should be no one
+            jest.useFakeTimers().setSystemTime(
+                new Date(new Date().getTime() + 1000 * 60 * 60 * 24 * 15),
+            )
+            for (let i = 0; i < 100; i++) {
+                expect(
+                    doesUserPassRollout({ rollout, boundedHash: i / 100 }),
+                ).toBeFalsy()
+            }
+
+            jest.useRealTimers()
+        })
     })
 
     it('throws when given an empty rollout object', () => {

diff --git a/sdk/nextjs/package.json b/sdk/nextjs/package.json
@@ -22,6 +22,7 @@
         "@devcycle/js-client-sdk": "^1.32.0",
         "@devcycle/react-client-sdk": "^1.30.0",
         "@devcycle/types": "^1.19.0",
+        "class-transformer": "^0.5.1",
         "hoist-non-react-statics": "^3.3.2",
         "server-only": "^0.0.1"
     },

diff --git a/sdk/nextjs/src/pages/bucketing.ts b/sdk/nextjs/src/pages/bucketing.ts
@@ -2,6 +2,7 @@ import { DevCycleUser, DVCPopulatedUser } from '@devcycle/js-client-sdk'
 import { generateBucketedConfig } from '@devcycle/bucketing'
 import { BucketedUserConfig, ConfigBody, ConfigSource } from '@devcycle/types'
 import { fetchCDNConfig, sdkConfigAPI } from './requests.js'
+import { plainToInstance } from 'class-transformer'
 
 class CDNConfigSource extends ConfigSource {
     async getConfig(
@@ -18,7 +19,7 @@ class CDNConfigSource extends ConfigSource {
             throw new Error('Could not fetch config')
         }
         return {
-            config: await configResponse.json(),
+            config: plainToInstance(ConfigBody, await configResponse.json()),
             lastModified: configResponse.headers.get('last-modified'),
             metaData: {},
         }

diff --git a/sdk/nextjs/src/server/bucketing.ts b/sdk/nextjs/src/server/bucketing.ts
@@ -48,7 +48,10 @@ const generateBucketedConfigCached = cache(
 
         return {
             bucketedConfig: {
-                ...generateBucketedConfig({ user: populatedUser, config }),
+                ...generateBucketedConfig({
+                    user: populatedUser,
+                    config,
+                }),
                 clientSDKKey,
                 sse: {
                     url: config.sse
@@ -73,7 +76,7 @@ class CDNConfigSource extends ConfigSource {
             obfuscated,
         )
         return {
-            config: config,
+            config,
             lastModified: headers.get('last-modified'),
             metaData: {},
         }

diff --git a/sdk/nextjs/src/server/requests.ts b/sdk/nextjs/src/server/requests.ts
@@ -2,6 +2,7 @@ import { DVCPopulatedUser } from '@devcycle/js-client-sdk'
 import { serializeUserSearchParams } from '../common/serializeUser'
 import { cache } from 'react'
 import { BucketedUserConfig, ConfigBody } from '@devcycle/types'
+import { plainToInstance } from 'class-transformer'
 
 const getFetchUrl = (sdkKey: string, obfuscated: boolean) =>
     `https://config-cdn.devcycle.com/config/v2/server/bootstrap/${
@@ -30,7 +31,7 @@ export const fetchCDNConfig = cache(
             throw new Error('Could not fetch config: ' + responseText)
         }
         return {
-            config: (await response.json()) as ConfigBody,
+            config: plainToInstance(ConfigBody, await response.json()),
             headers: response.headers,
         }
     },

diff --git a/yarn.lock b/yarn.lock
@@ -4708,6 +4708,7 @@ __metadata:
     "@devcycle/js-client-sdk": ^1.32.0
     "@devcycle/react-client-sdk": ^1.30.0
     "@devcycle/types": ^1.19.0
+    class-transformer: ^0.5.1
     hoist-non-react-statics: ^3.3.2
     server-only: ^0.0.1
   languageName: unknown
@@ -13469,7 +13470,7 @@ __metadata:
   languageName: node
   linkType: hard
 
-"class-transformer@npm:0.5.1":
+"class-transformer@npm:0.5.1, class-transformer@npm:^0.5.1":
   version: 0.5.1
   resolution: "class-transformer@npm:0.5.1"
   checksum: f191c8b4cc4239990f5efdd790cabdd852c243ed66248e39f6616a349c910c6eed2d9fd1fbf7ee6ea89f69b4f1d0b493b27347fe0cd0ae26b47c3745a805b6d3