Repository navigation
feat: add UUID string converter (#1017) - #1088
zhanghuang090 wants to merge 9 commits into
Conversation
| import org.junit.jupiter.params.ParameterizedTest; | ||
| import org.junit.jupiter.params.provider.ValueSource; | ||
|
|
||
| class UuidConverterTest extends AbstractExcelTest { |
There was a problem hiding this comment.
Please add a @Tag(Tags.ROUND_TRIP)
| UUID value = converter.convertToJavaData(new ReadCellData<>(input), null, configuration); | ||
| WriteCellData<?> cell = converter.convertToExcelData(value, null, configuration); | ||
| Assertions.assertEquals(CellDataTypeEnum.STRING, cell.getType()); | ||
| Assertions.assertEquals(input.toLowerCase(java.util.Locale.ROOT), cell.getStringValue()); |
There was a problem hiding this comment.
Please replace the full java.util.Locale.ROOT in both places. Use import java.util.Locale;.
|
|
Nit: #1032 keeps its registration checks in |
|
Thanks for the contribution. Please resolve the merge conflicts and update the PR when you have a chance. |
| } | ||
|
|
||
| @Data | ||
| public static class UuidData { |
There was a problem hiding this comment.
Please rename to UUIDData to stay consistent with the rest of the naming
Thank you for your contribution!
| } | ||
|
|
||
| @Test | ||
| void loadConvertersRegistersUuidFamily() { |
There was a problem hiding this comment.
Same here --> loadConvertersRegistersUUIDFamily
| .get(ConverterKeyBuild.buildKey(UUID.class, CellDataTypeEnum.STRING))); | ||
| Assertions.assertInstanceOf( | ||
| UUIDStringConverter.class, | ||
| DefaultConverterLoader.loadDefaultWriteConverter().get(ConverterKeyBuild.buildKey(UUID.class))); |
There was a problem hiding this comment.
Should we maybe extract DefaultConverterLoader.loadDefaultWriteConverter() into a variable like in loadConvertersRegistersLocalTimeFamily just to avoid calling it twice for consitency:
Map<ConverterKey, Converter<?>> writeConverter = DefaultConverterLoader.loadDefaultWriteConverter();
Assertions.assertInstanceOf(
UUIDStringConverter.class, writeConverter.get(ConverterKeyBuild.buildKey(UUID.class)));
Assertions.assertInstanceOf(
UUIDStringConverter.class,
writeConverter.get(ConverterKeyBuild.buildKey(UUID.class, CellDataTypeEnum.STRING)));
Purpose of the pull request
Related: #1017
Fields declared as
java.util.UUIDcurrently have no built-in converter. This adds UUID ↔ STRING conversion so identifiers can be imported and exported without a custom converter.Proposal: #1017 (comment)
What's changed?
UuidStringConverter, usingUUID.fromString()for non-empty input andUUID.toString()for canonical lowercase output. Empty strings are treated as missing values, consistently with blank cells.DefaultConverterLoader.ExcelDataConvertExceptionwith the parsing error as the cause.Validation
ClassUtilsFieldOverrideTestcleanup error (DirectoryNotEmptyExceptionwhile deleting a JUnit temporary directory on Windows). A focused rerun of that class plus UUID and loader tests passed all 27 cases without code changes.mvn -B -ntp -pl fesod-sheet -am -Dmaven.test.skip=false -Dtest=ClassUtilsFieldOverrideTest,UuidConverterTest,DefaultConverterLoaderTest -Dsurefire.failIfNoSpecifiedTests=false package spotless:checkpassed.javac --release 8against the built project classes.git diff --cached --checkpassed.Checklist