Github-issue#1048 : s3 object index. #2586
Conversation
Signed-off-by: Deepak Sahu <deepak.sahu562@gmail.com>
deepaksahu562
requested review from
chenqi0805,
engechas,
graytaylor0,
dinujoh,
cmanning09,
asifsmohammed,
dlvenable and
oeyh
as code owners
| /** | ||
| * Class responsible for creation of s3 key pattern based on date time stamp | ||
| */ | ||
| public class S3ObjectIndex { |
There was a problem hiding this comment.
S3ObjectIndexUtility may be a more accurate name for this class, since it is performing validations and creation of the s3 key pattern
Signed-off-by: Deepak Sahu <deepak.sahu562@gmail.com>
| public static String getObjectNameWithDateTimeId(final String indexAlias) { | ||
| DateTimeFormatter dateFormatter = getDatePatternFormatter(indexAlias); | ||
| String suffix = (dateFormatter != null) ? dateFormatter.format(getCurrentUtcTime()) : ""; | ||
| return indexAlias.replaceAll(TIME_PATTERN_REGULAR_EXPRESSION, "") + suffix + "-" + getTimeNanos() + "-" |
There was a problem hiding this comment.
Nit: Can we simplify it to
indexAlias.replaceAll(TIME_PATTERN_REGULAR_EXPRESSION, suffix + "-" + getTimeNanos() + "-" + UUID.randomUUID())
| /** | ||
| * Validate the index with the regular expression pattern. Throws exception if validation fails | ||
| */ | ||
| public static DateTimeFormatter getDatePatternFormatter(final String indexAlias) { |
There was a problem hiding this comment.
We should have a more precise name for the method, e.g. validateAndGetDateTimeFormatter or validateAndCreateDateTimeFormatter.
There was a problem hiding this comment.
Changed the method name to validateAndGetDateTimeFormatter.
| } | ||
|
|
||
| @Test | ||
| void testgetObjectPathPrefix_not_equal() throws IllegalArgumentException { |
There was a problem hiding this comment.
Is not_equal correct in the test case name?
There was a problem hiding this comment.
Modified the method name based on the functionality.
| @Test | ||
| void testObjectTimePatterns_equal() throws IllegalArgumentException { | ||
|
|
||
| DateTimeFormatter expectedIndex = S3ObjectIndexUtility.getDatePatternFormatter("events-%{yyyy-MM-dd}"); |
There was a problem hiding this comment.
expectedIndex -> expectedDateTimeFormatter
| void testObjectTimePatterns_equal() throws IllegalArgumentException { | ||
|
|
||
| DateTimeFormatter expectedIndex = S3ObjectIndexUtility.getDatePatternFormatter("events-%{yyyy-MM-dd}"); | ||
| DateTimeFormatter actualIndex = S3ObjectIndexUtility.getDatePatternFormatter("events-%{yyyy-MM-dd}"); |
There was a problem hiding this comment.
actualIndex -> actualDateTimeFormatter
There was a problem hiding this comment.
Also it is recommended to use a slightly different indexAlias as long as the same datetime pattern is contained.
| @Test | ||
| void test_utc_current_time() throws IllegalArgumentException { | ||
|
|
||
| ZonedDateTime expectedIndex = S3ObjectIndexUtility.getCurrentUtcTime(); |
There was a problem hiding this comment.
Similar as above. Please change the name of the variable to be accurate.
| @Test | ||
| void testObjectAliasWithDatePrefix_equal() throws IllegalArgumentException { | ||
|
|
||
| String expectedIndex = S3ObjectIndexUtility.getObjectNameWithDateTimeId("events-%{yyyy-MM-dd}"); |
Signed-off-by: Deepak Sahu <deepak.sahu562@gmail.com>
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #2586 +/- ##
============================================
+ Coverage 93.40% 93.46% +0.06%
- Complexity 2168 2186 +18
============================================
Files 257 258 +1
Lines 6064 6107 +43
Branches 490 497 +7
============================================
+ Hits 5664 5708 +44
+ Misses 270 268 -2
- Partials 130 131 +1
|
Description
Issues Resolved
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.