fix: copy constructors should use spread operator

In order to prevent bugs in our config copy constructors, we should
use the spread operator over `this`.

Previously in the copy constuctors we built a copy of the
corresponding properties by hand. Because some properties are
optional, and because we add new properties over time, we introduced
drift between the new properties and various copy constructors.

That is, when we added new properties, some copy constructors were out
of date and hence falling back to defaults instead of copying those
new properties over.

We can fix this across the board by:
1. aligning the field names in each config class with the corresponding
property name, and
2. building the props copy using a spread over `this` followed by the
new value.

packages/client-sdk-nodejs/src/config/configuration.ts

@@ -183,13 +183,8 @@ export class CacheConfiguration implements Configuration {
 
   withRetryStrategy(retryStrategy: RetryStrategy): Configuration {
     return new CacheConfiguration({
-      loggerFactory: this.loggerFactory,
-      retryStrategy: retryStrategy,
-      transportStrategy: this.transportStrategy,
-      middlewares: this.middlewares,
-      throwOnErrors: this.throwOnErrors,
-      readConcern: this.readConcern,
-      compression: this.compression,
+      ...this,
+      retryStrategy,
     });
   }
 
@@ -199,13 +194,8 @@ export class CacheConfiguration implements Configuration {
 
   withTransportStrategy(transportStrategy: TransportStrategy): Configuration {
     return new CacheConfiguration({
-      loggerFactory: this.loggerFactory,
-      retryStrategy: this.retryStrategy,
-      transportStrategy: transportStrategy,
-      middlewares: this.middlewares,
-      throwOnErrors: this.throwOnErrors,
-      readConcern: this.readConcern,
-      compression: this.compression,
+      ...this,
+      transportStrategy,
     });
   }
 
@@ -225,38 +215,23 @@ export class CacheConfiguration implements Configuration {
 
   withMiddlewares(middlewares: Middleware[]): Configuration {
     return new CacheConfiguration({
-      loggerFactory: this.loggerFactory,
-      retryStrategy: this.retryStrategy,
-      transportStrategy: this.transportStrategy,
-      middlewares: middlewares,
-      throwOnErrors: this.throwOnErrors,
-      readConcern: this.readConcern,
-      compression: this.compression,
+      ...this,
+      middlewares,
     });
   }
 
   addMiddleware(middleware: Middleware): Configuration {
     return new CacheConfiguration({
-      loggerFactory: this.loggerFactory,
-      retryStrategy: this.retryStrategy,
-      transportStrategy: this.transportStrategy,
+      ...this,
       middlewares: [middleware, ...this.middlewares],
-      throwOnErrors: this.throwOnErrors,
-      readConcern: this.readConcern,
-      compression: this.compression,
     });
   }
 
   withClientTimeoutMillis(clientTimeout: number): Configuration {
     return new CacheConfiguration({
-      loggerFactory: this.loggerFactory,
-      retryStrategy: this.retryStrategy,
+      ...this,
       transportStrategy:
         this.transportStrategy.withClientTimeoutMillis(clientTimeout),
-      middlewares: this.middlewares,
-      throwOnErrors: this.throwOnErrors,
-      readConcern: this.readConcern,
-      compression: this.compression,
     });
   }
 
@@ -266,13 +241,8 @@ export class CacheConfiguration implements Configuration {
 
   withThrowOnErrors(throwOnErrors: boolean): Configuration {
     return new CacheConfiguration({
-      loggerFactory: this.loggerFactory,
-      retryStrategy: this.retryStrategy,
-      transportStrategy: this.transportStrategy,
-      middlewares: this.middlewares,
-      throwOnErrors: throwOnErrors,
-      readConcern: this.readConcern,
-      compression: this.compression,
+      ...this,
+      throwOnErrors,
     });
   }
 
@@ -282,13 +252,8 @@ export class CacheConfiguration implements Configuration {
 
   withReadConcern(readConcern: ReadConcern): Configuration {
     return new CacheConfiguration({
-      loggerFactory: this.loggerFactory,
-      retryStrategy: this.retryStrategy,
-      transportStrategy: this.transportStrategy,
-      middlewares: this.middlewares,
-      throwOnErrors: this.throwOnErrors,
-      readConcern: readConcern,
-      compression: this.compression,
+      ...this,
+      readConcern,
     });
   }
 
@@ -300,12 +265,7 @@ export class CacheConfiguration implements Configuration {
     compressionStrategy: CompressionStrategy
   ): Configuration {
     return new CacheConfiguration({
-      loggerFactory: this.loggerFactory,
-      retryStrategy: this.retryStrategy,
-      transportStrategy: this.transportStrategy,
-      middlewares: this.middlewares,
-      throwOnErrors: this.throwOnErrors,
-      readConcern: this.readConcern,
+      ...this,
       compression: compressionStrategy,
     });
   }

packages/client-sdk-nodejs/src/config/leaderboard-configuration.ts

@@ -120,22 +120,18 @@ export class LeaderboardClientConfiguration
     clientTimeoutMillis: number
   ): LeaderboardConfiguration {
     return new LeaderboardClientConfiguration({
-      loggerFactory: this.loggerFactory,
+      ...this,
       transportStrategy:
         this.transportStrategy.withClientTimeoutMillis(clientTimeoutMillis),
-      throwOnErrors: this.throwOnErrors,
-      middlewares: this.middlewares,
     });
   }
 
   withTransportStrategy(
     transportStrategy: TransportStrategy
   ): LeaderboardConfiguration {
     return new LeaderboardClientConfiguration({
-      loggerFactory: this.loggerFactory,
-      transportStrategy: transportStrategy,
-      throwOnErrors: this.throwOnErrors,
-      middlewares: this.middlewares,
+      ...this,
+      transportStrategy,
     });
   }
 
@@ -145,10 +141,8 @@ export class LeaderboardClientConfiguration
 
   withThrowOnErrors(throwOnErrors: boolean): LeaderboardConfiguration {
     return new LeaderboardClientConfiguration({
-      loggerFactory: this.loggerFactory,
-      transportStrategy: this.transportStrategy,
-      throwOnErrors: throwOnErrors,
-      middlewares: this.middlewares,
+      ...this,
+      throwOnErrors,
     });
   }
 
@@ -158,19 +152,15 @@ export class LeaderboardClientConfiguration
 
   withMiddlewares(middlewares: Middleware[]): LeaderboardConfiguration {
     return new LeaderboardClientConfiguration({
-      loggerFactory: this.loggerFactory,
-      transportStrategy: this.transportStrategy,
-      middlewares: middlewares,
-      throwOnErrors: this.throwOnErrors,
+      ...this,
+      middlewares,
     });
   }
 
   addMiddleware(middleware: Middleware): LeaderboardConfiguration {
     return new LeaderboardClientConfiguration({
-      loggerFactory: this.loggerFactory,
-      transportStrategy: this.transportStrategy,
+      ...this,
       middlewares: [middleware, ...this.middlewares],
-      throwOnErrors: this.throwOnErrors,
     });
   }
 }

packages/client-sdk-nodejs/src/config/storage-configuration.ts

@@ -87,28 +87,25 @@ export class StorageClientConfiguration implements StorageConfiguration {
 
   withClientTimeoutMillis(clientTimeoutMillis: number): StorageConfiguration {
     return new StorageClientConfiguration({
-      loggerFactory: this.loggerFactory,
+      ...this,
       transportStrategy:
         this.transportStrategy.withClientTimeoutMillis(clientTimeoutMillis),
-      retryStrategy: this.retryStrategy,
     });
   }
 
   withTransportStrategy(
     transportStrategy: StorageTransportStrategy
   ): StorageConfiguration {
     return new StorageClientConfiguration({
-      loggerFactory: this.loggerFactory,
-      transportStrategy: transportStrategy,
-      retryStrategy: this.retryStrategy,
+      ...this,
+      transportStrategy,
     });
   }
 
   withRetryStrategy(retryStrategy: RetryStrategy): StorageConfiguration {
     return new StorageClientConfiguration({
-      loggerFactory: this.loggerFactory,
-      transportStrategy: this.transportStrategy,
-      retryStrategy: retryStrategy,
+      ...this,
+      retryStrategy,
     });
   }
 }

packages/client-sdk-nodejs/src/config/topic-configuration.ts

@@ -86,9 +86,8 @@ export class TopicClientConfiguration implements TopicConfiguration {
     transportStrategy: TopicTransportStrategy
   ): TopicConfiguration {
     return new TopicClientConfiguration({
-      loggerFactory: this.loggerFactory,
-      transportStrategy: transportStrategy,
-      throwOnErrors: this.throwOnErrors,
+      ...this,
+      transportStrategy,
     });
   }
 
@@ -108,9 +107,8 @@ export class TopicClientConfiguration implements TopicConfiguration {
 
   withThrowOnErrors(throwOnErrors: boolean): TopicConfiguration {
     return new TopicClientConfiguration({
-      loggerFactory: this.loggerFactory,
-      transportStrategy: this.transportStrategy,
-      throwOnErrors: throwOnErrors,
+      ...this,
+      throwOnErrors,
     });
   }
 }

packages/client-sdk-nodejs/src/config/transport/cache/transport-strategy.ts

@@ -137,17 +137,15 @@ export class StaticGrpcConfiguration implements GrpcConfiguration {
 
   withDeadlineMillis(deadlineMillis: number): StaticGrpcConfiguration {
     return new StaticGrpcConfiguration({
-      deadlineMillis: deadlineMillis,
-      maxSessionMemoryMb: this.maxSessionMemoryMb,
-      numClients: this.numClients,
+      ...this,
+      deadlineMillis,
     });
   }
 
   withMaxSessionMemoryMb(maxSessionMemoryMb: number): StaticGrpcConfiguration {
     return new StaticGrpcConfiguration({
-      deadlineMillis: this.deadlineMillis,
-      maxSessionMemoryMb: maxSessionMemoryMb,
-      numClients: this.numClients,
+      ...this,
+      maxSessionMemoryMb,
     });
   }
 
@@ -165,9 +163,8 @@ export class StaticGrpcConfiguration implements GrpcConfiguration {
 
   withNumClients(numClients: number): GrpcConfiguration {
     return new StaticGrpcConfiguration({
-      deadlineMillis: this.deadlineMillis,
-      maxSessionMemoryMb: this.maxSessionMemoryMb,
-      numClients: numClients,
+      ...this,
+      numClients,
     });
   }
 
@@ -177,38 +174,37 @@ export class StaticGrpcConfiguration implements GrpcConfiguration {
 
   withMaxConcurrentRequests(maxConcurrentRequests: number): GrpcConfiguration {
     return new StaticGrpcConfiguration({
-      deadlineMillis: this.deadlineMillis,
-      maxSessionMemoryMb: this.maxSessionMemoryMb,
-      numClients: this.numClients,
-      maxConcurrentRequests: maxConcurrentRequests,
+      ...this,
+      maxConcurrentRequests,
     });
   }
 }
 
 export class StaticTransportStrategy implements TransportStrategy {
-  private readonly grpcConfig: GrpcConfiguration;
+  private readonly grpcConfiguration: GrpcConfiguration;
   private readonly maxIdleMillis: number;
   private readonly maxClientAgeMillis?: number;
 
   constructor(props: TransportStrategyProps) {
-    this.grpcConfig = props.grpcConfiguration;
+    this.grpcConfiguration = props.grpcConfiguration;
     this.maxIdleMillis = props.maxIdleMillis;
     this.maxClientAgeMillis = props.maxClientAgeMillis;
   }
 
   getGrpcConfig(): GrpcConfiguration {
-    return this.grpcConfig;
+    return this.grpcConfiguration;
   }
 
   getMaxClientAgeMillis(): number | undefined {
     return this.maxClientAgeMillis;
   }
 
-  withGrpcConfig(grpcConfig: GrpcConfiguration): StaticTransportStrategy {
+  withGrpcConfig(
+    grpcConfiguration: GrpcConfiguration
+  ): StaticTransportStrategy {
     return new StaticTransportStrategy({
-      grpcConfiguration: grpcConfig,
-      maxIdleMillis: this.maxIdleMillis,
-      maxClientAgeMillis: this.maxClientAgeMillis,
+      ...this,
+      grpcConfiguration,
     });
   }
 
@@ -218,39 +214,38 @@ export class StaticTransportStrategy implements TransportStrategy {
 
   withMaxIdleMillis(maxIdleMillis: number): TransportStrategy {
     return new StaticTransportStrategy({
-      grpcConfiguration: this.grpcConfig,
-      maxIdleMillis: maxIdleMillis,
-      maxClientAgeMillis: this.maxClientAgeMillis,
+      ...this,
+      maxIdleMillis,
     });
   }
 
   withMaxClientAgeMillis(maxClientAgeMillis: number): TransportStrategy {
     return new StaticTransportStrategy({
-      grpcConfiguration: this.grpcConfig,
-      maxIdleMillis: this.maxIdleMillis,
-      maxClientAgeMillis: maxClientAgeMillis,
+      ...this,
+      maxClientAgeMillis,
     });
   }
 
   withClientTimeoutMillis(clientTimeout: number): StaticTransportStrategy {
     return new StaticTransportStrategy({
-      grpcConfiguration: this.grpcConfig.withDeadlineMillis(clientTimeout),
-      maxIdleMillis: this.maxIdleMillis,
+      ...this,
+      grpcConfiguration:
+        this.grpcConfiguration.withDeadlineMillis(clientTimeout),
     });
   }
 
   getMaxConcurrentRequests(): number | undefined {
-    return this.grpcConfig.getMaxConcurrentRequests();
+    return this.grpcConfiguration.getMaxConcurrentRequests();
   }
 
   withMaxConcurrentRequests(
     maxConcurrentRequests: number
   ): StaticTransportStrategy {
     return new StaticTransportStrategy({
-      grpcConfiguration: this.grpcConfig.withMaxConcurrentRequests(
+      ...this,
+      grpcConfiguration: this.grpcConfiguration.withMaxConcurrentRequests(
         maxConcurrentRequests
       ),
-      maxIdleMillis: this.maxIdleMillis,
     });
   }
 }