Remote maxReplayBufferSize

[lmeier]

No description provided.

[Copilot]

Pull request overview

This pull request adds support for remote configuration of the maximum replay buffer size, allowing the server to control the buffer capacity within validated bounds.

Changes:

• Added maxReplayBufferSize field to RadarSdkConfiguration with a default value of 120 and validation to ensure values are between 1 and 120
• Updated RadarSimpleReplayBuffer to use the configured buffer size instead of a hardcoded value when removing old replays
• Added comprehensive test coverage for the new configuration field in serialization, deserialization, and default value scenarios

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
sdk/src/main/java/io/radar/sdk/model/RadarSdkConfiguration.kt	Adds maxReplayBufferSize field with validation logic, default constant, and serialization/deserialization support
sdk/src/main/java/io/radar/sdk/util/RadarSimpleReplayBuffer.kt	Updates buffer size management to use the remote configuration value and changes from if to while loop for proper size enforcement
sdk/src/test/java/io/radar/sdk/model/RadarSdkConfigurationTest.kt	Adds test coverage for maxReplayBufferSize in serialization, deserialization, and default value tests

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The validation logic here checks if maxReplayBufferSize > 0, but this check is redundant. The fromJson method in RadarSdkConfiguration already ensures that maxReplayBufferSize is always greater than 0 and falls back to DEFAULT_MAX_REPLAY_BUFFER_SIZE if invalid. Consider removing this conditional and using sdkConfiguration.maxReplayBufferSize directly.

Suggested change

	val maxBufferSize = if (sdkConfiguration.maxReplayBufferSize > 0) {
	sdkConfiguration.maxReplayBufferSize
	} else {
	RadarSdkConfiguration.DEFAULT_MAX_REPLAY_BUFFER_SIZE
	}
	val maxBufferSize = sdkConfiguration.maxReplayBufferSize

[Copilot]

The validation logic restricts maxReplayBufferSize to values less than or equal to DEFAULT_MAX_REPLAY_BUFFER_SIZE (120). This defeats the purpose of having a remotely configurable buffer size, as the server cannot increase the buffer size beyond 120. If the intention is to allow remote configuration to adjust the buffer size, consider either removing the upper bound check or documenting why 120 is a hard limit. If 120 is indeed a hard limit due to the LinkedBlockingDeque initialization in RadarSimpleReplayBuffer, that should be addressed as well.

[Copilot]

Test coverage is missing for the validation logic of maxReplayBufferSize. The fromJson method in RadarSdkConfiguration includes validation that clamps values to be between 1 and DEFAULT_MAX_REPLAY_BUFFER_SIZE (120), but there are no tests verifying this behavior. Consider adding tests for edge cases such as: negative values, zero, values greater than 120, and values between 1 and 120 to ensure the validation logic works as intended.

[Copilot]

The change from if to while is correct for ensuring the buffer size constraint is satisfied when maxBufferSize is reduced. However, when maxBufferSize is significantly smaller than the current buffer.size, this could result in removing many items in a tight loop. Consider adding a comment explaining why a while loop is necessary here, or consider clearing multiple items in a single operation if the difference is large for better performance.

Suggested change

	// Use a while-loop (instead of a single if) to ensure the buffer is trimmed
	// even when maxReplayBufferSize is reduced to a value much smaller than the
	// current buffer size. The loop is bounded by MAXIMUM_CAPACITY, so this
	// remains inexpensive in practice.