Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Maximum Message Size accepted a long, but should accept an int #903

Merged
merged 4 commits into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions src/NATS.Client/Internals/Validator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -359,14 +359,25 @@ internal static long ValidateMaxBucketBytes(long max)
return ValidateGtZeroOrMinus1(max, "Max BucketBytes"); // max bucket bytes is a kv alias to max bytes
}

internal static long ValidateMaxMessageSize(long max, string label)
{
long l = ValidateGtZeroOrMinus1(max, label);
if (l > int.MaxValue)
{
throw new ArgumentException($"{label} cannot be larger than {int.MaxValue}");
}

return l;
}

internal static long ValidateMaxMessageSize(long max)
{
return ValidateGtZeroOrMinus1(max, "Max Message Size");
return ValidateMaxMessageSize(max, "Max Message Size");
}

internal static long ValidateMaxValueSize(long max)
{
return ValidateGtZeroOrMinus1(max, "Max Value Size"); // max value size is a kv alias to max message size
return ValidateMaxMessageSize(max, "Max Value Size"); // max value size is a kv alias to max message size
}

internal static int ValidateNumberOfReplicas(int replicas)
Expand Down
31 changes: 22 additions & 9 deletions src/NATS.Client/JetStream/StreamConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public sealed class StreamConfiguration : JsonSerializable
public long MaxMsgsPerSubject { get; }
public long MaxBytes { get; }
public Duration MaxAge { get; }
public long MaxMsgSize { get; }
public int MaximumMessageSize { get; }
public StorageType StorageType { get; }
public int Replicas { get; }
public bool NoAck { get; }
Expand All @@ -54,8 +54,10 @@ public sealed class StreamConfiguration : JsonSerializable
public IDictionary<string, string> Metadata { get; }
public ulong FirstSequence { get; private set; }

[Obsolete("MaxMsgSize was mistakenly renamed in a previous change.", false)]
public long MaxValueSize => MaxMsgSize;
[Obsolete("The server value is a 32-bit signed value. Use MaximumMessageSize.", false)]
public long MaxMsgSize => MaximumMessageSize;
[Obsolete("MaxMsgSize was mistakenly renamed in a previous change. Use MaximumMessageSize.", false)]
public long MaxValueSize => MaximumMessageSize;

internal StreamConfiguration(string json) : this(JSON.Parse(json)) { }

Expand All @@ -73,7 +75,7 @@ internal StreamConfiguration(JSONNode scNode)
MaxMsgsPerSubject = AsLongOrMinus1(scNode, ApiConstants.MaxMsgsPerSubject);
MaxBytes = AsLongOrMinus1(scNode, ApiConstants.MaxBytes);
MaxAge = AsDuration(scNode, ApiConstants.MaxAge, Duration.Zero);
MaxMsgSize = AsLongOrMinus1(scNode, ApiConstants.MaxMsgSize);
MaximumMessageSize = AsIntOrMinus1(scNode, ApiConstants.MaxMsgSize);
Replicas = scNode[ApiConstants.NumReplicas].AsInt;
NoAck = scNode[ApiConstants.NoAck].AsBool;
TemplateOwner = scNode[ApiConstants.TemplateOwner].Value;
Expand Down Expand Up @@ -107,7 +109,7 @@ private StreamConfiguration(StreamConfigurationBuilder builder)
MaxMsgsPerSubject = builder._maxMsgsPerSubject;
MaxBytes = builder._maxBytes;
MaxAge = builder._maxAge;
MaxMsgSize = builder._maxMsgSize;
MaximumMessageSize = builder._maxMsgSize;
StorageType = builder._storageType;
Replicas = builder._replicas;
NoAck = builder._noAck;
Expand Down Expand Up @@ -149,7 +151,7 @@ public override JSONNode ToJsonNode()
AddField(o, ApiConstants.MaxMsgsPerSubject, MaxMsgsPerSubject);
AddField(o, ApiConstants.MaxBytes, MaxBytes);
AddField(o, ApiConstants.MaxAge, MaxAge.Nanos);
AddField(o, ApiConstants.MaxMsgSize, MaxMsgSize);
AddField(o, ApiConstants.MaxMsgSize, MaximumMessageSize);
AddField(o, ApiConstants.NumReplicas, Replicas);
AddField(o, ApiConstants.NoAck, NoAck);
AddField(o, ApiConstants.TemplateOwner, TemplateOwner);
Expand Down Expand Up @@ -197,7 +199,7 @@ public sealed class StreamConfigurationBuilder
internal long _maxMsgsPerSubject = -1;
internal long _maxBytes = -1;
internal Duration _maxAge = Duration.Zero;
internal long _maxMsgSize = -1;
internal int _maxMsgSize = -1;
internal StorageType _storageType = StorageType.File;
internal int _replicas = 1;
internal bool _noAck;
Expand Down Expand Up @@ -236,7 +238,7 @@ public StreamConfigurationBuilder(StreamConfiguration sc)
_maxMsgsPerSubject = sc.MaxMsgsPerSubject;
_maxBytes = sc.MaxBytes;
_maxAge = sc.MaxAge;
_maxMsgSize = sc.MaxMsgSize;
_maxMsgSize = sc.MaximumMessageSize;
_storageType = sc.StorageType;
_replicas = sc.Replicas;
_noAck = sc.NoAck;
Expand Down Expand Up @@ -424,8 +426,19 @@ public StreamConfigurationBuilder WithMaxAge(long maxAgeMillis) {
/// </summary>
/// <param name="maxMsgSize">the maximum message size</param>
/// <returns>The StreamConfigurationBuilder</returns>
[Obsolete("The server value is a 32-bit signed value. Use WithMaximumMessageSize.", false)]
public StreamConfigurationBuilder WithMaxMsgSize(long maxMsgSize) {
_maxMsgSize = Validator.ValidateMaxMessageSize(maxMsgSize);
_maxMsgSize = (int)Validator.ValidateMaxMessageSize(maxMsgSize);
return this;
}

/// <summary>
/// Sets the maximum message size in the StreamConfiguration.
/// </summary>
/// <param name="maxMsgSize">the maximum message size</param>
/// <returns>The StreamConfigurationBuilder</returns>
public StreamConfigurationBuilder WithMaximumMessageSize(int maxMsgSize) {
_maxMsgSize = (int)Validator.ValidateMaxMessageSize(maxMsgSize);
return this;
}

Expand Down
23 changes: 20 additions & 3 deletions src/NATS.Client/KeyValue/KeyValueConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,13 @@ internal KeyValueConfiguration(StreamConfiguration sc)
/// <summary>
/// The maximum size for an individual value in the bucket
/// </summary>
public long MaxValueSize => BackingConfig.MaxMsgSize;
[Obsolete("The server value is a 32-bit signed value. Use MaximumValueSize.", false)]
public long MaxValueSize => BackingConfig.MaximumMessageSize;

/// <summary>
/// The maximum size for an individual value in the bucket
/// </summary>
public int MaximumValueSize => BackingConfig.MaximumMessageSize;

/// <summary>
/// The maximum age for a value in this bucket
Expand Down Expand Up @@ -215,8 +221,19 @@ public KeyValueConfigurationBuilder WithMaxBucketSize(long maxBucketSize) {
/// </summary>
/// <param name="maxValueSize">the maximum size for a value</param>
/// <returns></returns>
[Obsolete("The server value is a 32-bit signed value. Use WithMaximumValueSize.", false)]
public KeyValueConfigurationBuilder WithMaxValueSize(long maxValueSize) {
scBuilder.WithMaxMsgSize(Validator.ValidateMaxValueSize(maxValueSize));
scBuilder.WithMaximumMessageSize((int)Validator.ValidateMaxValueSize(maxValueSize));
return this;
}

/// <summary>
/// Sets the maximum size for an individual value in the KeyValueConfiguration.
/// </summary>
/// <param name="maxValueSize">the maximum size for a value</param>
/// <returns></returns>
public KeyValueConfigurationBuilder WithMaximumValueSize(int maxValueSize) {
scBuilder.WithMaximumMessageSize((int)Validator.ValidateMaxValueSize(maxValueSize));
return this;
}

Expand Down Expand Up @@ -428,7 +445,7 @@ public KeyValueConfiguration Build() {

public override string ToString()
{
return $"BucketName: {BucketName}, Description: {Description}, MaxHistoryPerKey: {MaxHistoryPerKey}, MaxBucketSize: {MaxBucketSize}, MaxValueSize: {MaxValueSize}, Ttl: {Ttl}, StorageType: {StorageType}, Replicas: {Replicas} Placement: {Placement}, Republish: {Republish}, IsCompressed: {IsCompressed}";
return $"BucketName: {BucketName}, Description: {Description}, MaxHistoryPerKey: {MaxHistoryPerKey}, MaxBucketSize: {MaxBucketSize}, MaximumValueSize: {MaximumValueSize}, Ttl: {Ttl}, StorageType: {StorageType}, Replicas: {Replicas} Placement: {Placement}, Republish: {Republish}, IsCompressed: {IsCompressed}";
}
}
}
11 changes: 9 additions & 2 deletions src/NATS.Client/KeyValue/KeyValueStatus.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

using System;
using System.Collections.Generic;
using NATS.Client.Internals;
using NATS.Client.JetStream;
Expand Down Expand Up @@ -67,7 +68,13 @@ public KeyValueStatus(StreamInfo si) {
/// <summary>
/// The maximum size for an individual value in the bucket
/// </summary>
public long MaxValueSize => Config.MaxValueSize;
[Obsolete("The server value is a 32-bit signed value. Use MaximumValueSize.", false)]
public long MaxValueSize => Config.MaximumValueSize;

/// <summary>
/// The maximum size for an individual value in the bucket
/// </summary>
public long MaximumValueSize => Config.MaximumValueSize;

/// <summary>
/// The maximum age for a value in this bucket
Expand Down Expand Up @@ -111,7 +118,7 @@ public KeyValueStatus(StreamInfo si) {

public override string ToString()
{
return $"BucketName: {BucketName}, Description: {Description}, EntryCount: {EntryCount}, MaxHistoryPerKey: {MaxHistoryPerKey}, MaxBucketSize: {MaxBucketSize}, MaxValueSize: {MaxValueSize}, Ttl: {Ttl}, StorageType: {StorageType}, Replicas: {Replicas}, BackingStore: {BackingStore}";
return $"BucketName: {BucketName}, Description: {Description}, EntryCount: {EntryCount}, MaxHistoryPerKey: {MaxHistoryPerKey}, MaxBucketSize: {MaxBucketSize}, MaximumValueSize: {MaximumValueSize}, Ttl: {Ttl}, StorageType: {StorageType}, Replicas: {Replicas}, BackingStore: {BackingStore}";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public static void Main(string[] args)
// .WithMaxConsumers(...)
// .WithMaxBytes(...)
// .WithMaxAge(...)
// .WithMaxMsgSize(...)
// .WithMaximumMessageSize(...)
.WithStorageType(StorageType.Memory)
// .WithReplicas(...)
// .WithNoAck(...)
Expand Down
2 changes: 1 addition & 1 deletion src/Tests/IntegrationTests/TestJetStream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1064,7 +1064,7 @@ public async Task TestMaxPayloadJs()
.WithName(streamName)
.WithStorageType(StorageType.Memory)
.WithSubjects(subject1, subject2)
.WithMaxMsgSize(1000)
.WithMaximumMessageSize(1000)
.Build()
);

Expand Down
10 changes: 5 additions & 5 deletions src/Tests/IntegrationTests/TestJetStreamManagement.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public void TestStreamCreate()
Assert.Equal(-1, sc.MaxConsumers);
Assert.Equal(-1, sc.MaxMsgs);
Assert.Equal(-1, sc.MaxBytes);
Assert.Equal(-1, sc.MaxMsgSize);
Assert.Equal(-1, sc.MaximumMessageSize);
Assert.Equal(1, sc.Replicas);
Assert.Equal(1U, sc.FirstSequence);

Expand Down Expand Up @@ -121,7 +121,7 @@ public void TestStreamCreateWithNoSubject() {
Assert.Equal(-1, sc.MaxConsumers);
Assert.Equal(-1, sc.MaxMsgs);
Assert.Equal(-1, sc.MaxBytes);
Assert.Equal(-1, sc.MaxMsgSize);
Assert.Equal(-1, sc.MaximumMessageSize);
Assert.Equal(1, sc.Replicas);

Assert.Equal(Duration.Zero, sc.MaxAge);
Expand Down Expand Up @@ -153,7 +153,7 @@ public void TestUpdateStream()
Assert.Equal(Subject(0), sc.Subjects[0]);
Assert.Equal(Subject(1), sc.Subjects[1]);
Assert.Equal(-1, sc.MaxBytes);
Assert.Equal(-1, sc.MaxMsgSize);
Assert.Equal(-1, sc.MaximumMessageSize);
Assert.Equal(Duration.Zero, sc.MaxAge);
Assert.Equal(StorageType.Memory, sc.StorageType);
Assert.Equal(DiscardPolicy.Old, sc.DiscardPolicy);
Expand All @@ -167,7 +167,7 @@ public void TestUpdateStream()
.WithStorageType(StorageType.Memory)
.WithSubjects(Subject(0), Subject(1), Subject(2))
.WithMaxBytes(43)
.WithMaxMsgSize(44)
.WithMaximumMessageSize(44)
.WithMaxAge(Duration.OfDays(100))
.WithDiscardPolicy(DiscardPolicy.New)
.WithNoAck(true)
Expand All @@ -185,7 +185,7 @@ public void TestUpdateStream()
Assert.Equal(Subject(1), sc.Subjects[1]);
Assert.Equal(Subject(2), sc.Subjects[2]);
Assert.Equal(43u, sc.MaxBytes);
Assert.Equal(44, sc.MaxMsgSize);
Assert.Equal(44, sc.MaximumMessageSize);
Assert.Equal(Duration.OfDays(100), sc.MaxAge);
Assert.Equal(StorageType.Memory, sc.StorageType);
Assert.Equal(DiscardPolicy.New, sc.DiscardPolicy);
Expand Down
10 changes: 5 additions & 5 deletions src/Tests/IntegrationTests/TestKeyValue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ public void TestWorkFlow()
Assert.Equal(3, kvc.MaxHistoryPerKey);
Assert.Equal(-1, status.MaxBucketSize);
Assert.Equal(-1, kvc.MaxBucketSize);
Assert.Equal(-1, status.MaxValueSize);
Assert.Equal(-1, kvc.MaxValueSize);
Assert.Equal(-1, status.MaximumValueSize);
Assert.Equal(-1, kvc.MaximumValueSize);
Assert.Equal(Duration.Zero, status.Ttl);
Assert.Equal(Duration.Zero, kvc.Ttl);
Assert.Equal(StorageType.Memory, status.StorageType);
Expand Down Expand Up @@ -557,7 +557,7 @@ public void TestCreateUpdate() {
Assert.Empty(kvs.Description);
Assert.Equal(1, kvs.MaxHistoryPerKey);
Assert.Equal(-1, kvs.MaxBucketSize);
Assert.Equal(-1, kvs.MaxValueSize);
Assert.Equal(-1, kvs.MaximumValueSize);
Assert.Equal(Duration.Zero, kvs.Ttl);
Assert.Equal(StorageType.Memory, kvs.StorageType);
Assert.Equal(1, kvs.Replicas);
Expand All @@ -580,7 +580,7 @@ public void TestCreateUpdate() {
.WithDescription(Plain)
.WithMaxHistoryPerKey(3)
.WithMaxBucketSize(10_000)
.WithMaxValueSize(100)
.WithMaximumValueSize(100)
.WithTtl(Duration.OfHours(1))
.Build();

Expand All @@ -590,7 +590,7 @@ public void TestCreateUpdate() {
Assert.Equal(Plain, kvs.Description);
Assert.Equal(3, kvs.MaxHistoryPerKey);
Assert.Equal(10_000, kvs.MaxBucketSize);
Assert.Equal(100, kvs.MaxValueSize);
Assert.Equal(100, kvs.MaximumValueSize);
Assert.Equal(Duration.OfHours(1), kvs.Ttl);
Assert.Equal(StorageType.Memory, kvs.StorageType);
Assert.Equal(1, kvs.Replicas);
Expand Down
8 changes: 4 additions & 4 deletions src/Tests/UnitTests/JetStream/TestKeyValueConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public void TestConstructionWorks() {
.WithName("bucketName")
.WithMaxHistoryPerKey(44)
.WithMaxBucketSize(555)
.WithMaxValueSize(666)
.WithMaximumValueSize(666)
.WithTtl(Duration.OfMillis(777))
.WithStorageType(StorageType.Memory)
.WithReplicas(2)
Expand All @@ -59,7 +59,7 @@ private void Validate(KeyValueConfiguration kvc) {
Assert.Equal("bucketName", kvc.BucketName);
Assert.Equal(44, kvc.MaxHistoryPerKey);
Assert.Equal(555, kvc.MaxBucketSize);
Assert.Equal(666, kvc.MaxValueSize);
Assert.Equal(666, kvc.MaximumValueSize);
Assert.Equal(Duration.OfMillis(777), kvc.Ttl);
Assert.Equal(StorageType.Memory, kvc.StorageType);
Assert.True(kvc.AllowDirect);
Expand Down Expand Up @@ -171,8 +171,8 @@ public void TestConstructionInvalidsCoverage() {
Assert.Throws<ArgumentException>(() => KeyValueConfiguration.Builder().WithMaxHistoryPerKey(65));
Assert.Throws<ArgumentException>(() => KeyValueConfiguration.Builder().WithMaxBucketSize(0));
Assert.Throws<ArgumentException>(() => KeyValueConfiguration.Builder().WithMaxBucketSize(-2));
Assert.Throws<ArgumentException>(() => KeyValueConfiguration.Builder().WithMaxValueSize(0));
Assert.Throws<ArgumentException>(() => KeyValueConfiguration.Builder().WithMaxValueSize(-2));
Assert.Throws<ArgumentException>(() => KeyValueConfiguration.Builder().WithMaximumValueSize(0));
Assert.Throws<ArgumentException>(() => KeyValueConfiguration.Builder().WithMaximumValueSize(-2));
Assert.Throws<ArgumentException>(() => KeyValueConfiguration.Builder().WithTtl(Duration.OfNanos(-1)));
Assert.Throws<ArgumentException>(() => KeyValueConfiguration.Builder().WithReplicas(0));
Assert.Throws<ArgumentException>(() => KeyValueConfiguration.Builder().WithReplicas(6));
Expand Down
8 changes: 4 additions & 4 deletions src/Tests/UnitTests/JetStream/TestStreamConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public void TestConstruction()
.WithMaxMessagesPerSubject(testSc.MaxMsgsPerSubject)
.WithMaxBytes(testSc.MaxBytes)
.WithMaxAge(testSc.MaxAge)
.WithMaxMsgSize(testSc.MaxMsgSize)
.WithMaximumMessageSize(testSc.MaximumMessageSize)
.WithStorageType(testSc.StorageType)
.WithReplicas(testSc.Replicas)
.WithNoAck(testSc.NoAck)
Expand Down Expand Up @@ -108,8 +108,8 @@ public void TestConstructionInvalidsCoverage()
Assert.Throws<ArgumentException>(() => StreamConfiguration.Builder().WithMaxBytes(-2));
Assert.Throws<ArgumentException>(() => StreamConfiguration.Builder().WithMaxAge(Duration.OfNanos(-1)));
Assert.Throws<ArgumentException>(() => StreamConfiguration.Builder().WithMaxAge(-1));
Assert.Throws<ArgumentException>(() => StreamConfiguration.Builder().WithMaxMsgSize(0));
Assert.Throws<ArgumentException>(() => StreamConfiguration.Builder().WithMaxMsgSize(-2));
Assert.Throws<ArgumentException>(() => StreamConfiguration.Builder().WithMaximumMessageSize(0));
Assert.Throws<ArgumentException>(() => StreamConfiguration.Builder().WithMaximumMessageSize(-2));
Assert.Throws<ArgumentException>(() => StreamConfiguration.Builder().WithReplicas(0));
Assert.Throws<ArgumentException>(() => StreamConfiguration.Builder().WithReplicas(6));
Assert.Throws<ArgumentException>(() => StreamConfiguration.Builder().WithDuplicateWindow(Duration.OfNanos(-1)));
Expand Down Expand Up @@ -282,7 +282,7 @@ private void Validate(StreamConfiguration sc, bool serverTest)
Assert.Equal(732, sc.MaxBytes);
Assert.Equal(Duration.OfNanos(43000000000L), sc.MaxAge);
Assert.Equal(Duration.OfNanos(42000000000L), sc.DuplicateWindow);
Assert.Equal(734, sc.MaxMsgSize);
Assert.Equal(734, sc.MaximumMessageSize);
Assert.Equal(StorageType.Memory, sc.StorageType);
Assert.Equal(DiscardPolicy.New, sc.DiscardPolicy);

Expand Down
2 changes: 1 addition & 1 deletion src/Tests/UnitTests/JetStream/TestStreamInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public void JsonIsReadProperly()
Assert.Equal(3u, si.Config.MaxBytes);
Assert.Equal(DiscardPolicy.Old, si.Config.DiscardPolicy);
Assert.Equal(100000000000, si.Config.MaxAge.Nanos);
Assert.Equal(4, si.Config.MaxMsgSize);
Assert.Equal(4, si.Config.MaximumMessageSize);
Assert.Equal(StorageType.Memory, si.Config.StorageType);
Assert.Equal(5, si.Config.Replicas);
Assert.Equal(120000000000, si.Config.DuplicateWindow.Nanos);
Expand Down
Loading