mehbey commented on code in PR #14135: URL: https://github.com/apache/kafka/pull/14135#discussion_r1289574103
########## core/src/test/scala/unit/kafka/log/LogValidatorTest.scala: ########## @@ -1398,6 +1437,73 @@ class LogValidatorTest { assertEquals(6, e.recordErrors.size) } + @Test + def testRecordWithPastTimestampIsRejected(): Unit = { + val timestampBeforeMaxConfig = 24 * 60 * 60 * 1000L //24 hrs + val timestampAfterMaxConfig = 1 * 60 * 60 * 1000L //1 hr + val now = System.currentTimeMillis() + val fiveMinutesBeforeThreshold = now - timestampBeforeMaxConfig - (5 * 60 * 1000L) + val records = createRecords(magicValue = RecordBatch.MAGIC_VALUE_V2, timestamp = fiveMinutesBeforeThreshold, + codec = CompressionType.GZIP) + val e = assertThrows(classOf[RecordValidationException], + () => new LogValidator( + records, + topicPartition, + time, + CompressionType.GZIP, + CompressionType.GZIP, + false, + RecordBatch.MAGIC_VALUE_V1, + TimestampType.CREATE_TIME, + timestampBeforeMaxConfig, + timestampAfterMaxConfig, + RecordBatch.NO_PARTITION_LEADER_EPOCH, + AppendOrigin.CLIENT, + MetadataVersion.latest + ).validateMessagesAndAssignOffsets( + PrimitiveRef.ofLong(0L), metricsRecorder, RequestLocal.withThreadConfinedCaching.bufferSupplier + ) + ) + + assertTrue(e.invalidException.isInstanceOf[InvalidTimestampException]) + assertFalse(e.recordErrors.isEmpty) + assertEquals(e.recordErrors.size, 3) + } + + + @Test + def testRecordWithFutureTimestampIsRejected(): Unit = { + val timestampBeforeMaxConfig = 24 * 60 * 60 * 1000L //24 hrs + val timestampAfterMaxConfig = 1 * 60 * 60 * 1000L //1 hr + val now = System.currentTimeMillis() Review Comment: that will make the test a little bit involved. The reason is we are using a producer from the common test until `TestUtils.createProducer` which spins up a test producer using the System's current timestamp. We can potentially refactor that code so that it takes a mock time. But without that the timestamp validation will use the system's current timestamp hence the tests follow the same pattern as well. ########## core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala: ########## @@ -761,6 +761,7 @@ class KafkaConfigTest { } @Test + @nowarn("cat=deprecation") Review Comment: Updated everywhere. Thank you. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org