You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Patrick Rhomberg <pr...@pivotal.io> on 2017/05/16 22:29:06 UTC
Re: Review Request 59323: GEODE-1994: Overhaul of
internal.lang.StringUtils
to extend and heavily use commons.lang.StringUtils.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59323/
-----------------------------------------------------------
(Updated May 16, 2017, 10:29 p.m.)
Review request for geode, Jinmei Liao, Jared Stewart, Ken Howe, Kirk Lund, and Swapnil Bawaskar.
Summary (updated)
-----------------
GEODE-1994: Overhaul of internal.lang.StringUtils to extend and heavily use commons.lang.StringUtils.
Repository: geode
Description
-------
* Renamed:
* -- EMPTY_STRING -> EMPTY (inherited)
* -- toUpperCase -> upperCase (inherited)
* -- toLowerCase -> lowerCase (inherited)
* -- padEnding -> rightPad (inherited)
*
* Removed:
* -- EMPTY_STRING_ARRAY; usage replaced with commons.lang.ArrayUtils.EMPTY_STRING_ARRAY
* -- SPACES
* -- UTF_8; rare usage replaced with raw string
* -- concat; usage replaced with commons.lang.join, refactoring as necessary.
* -- getLettersOnly
* -- getSpaces
* -- truncate
* -- valueOf; usage refactored to use defaultString
*
* Refactored
* -- defaultIfBlank: previously relied on varargs and could return null. Usage refactored to allow inheritance from commons.
* -- defaultString(s, EMPTY) refactored to use standard signature defaultString(s) for consistency throughout codebase.
* -- isBlank: usage refactored to resolve discrepancies with commons.lang.isBlank, which is now inherited.
* -- isEmpty: usage refactored to resolve discrepancies with commons.lang.isEmpty, which is now inherited.
*
* Code Cleanup:
* -- Many uses of !isBlank -> isNotBlank
* -- Changes suggested by Inspections on most touched files.
* -- Explicit <T> -> <> when type is inferable
* -- while loops operating on iterators converted to for each loops
* -- for loops operating on array indices converted to for each loops
* -- Various string typos corrected.
* -- isEmpty(s.trim()) -> isBlank(s)
* -- s.trim().isEmpty() -> isEmpty(s)
* -- Removed some instances of 'dead' code
*
* Qualitative Changes:
* -- The following functions now throw an error when called with a null string input:
* -- * LocatorLauncher.Builder.setMemberName
* -- * ServerLauncher.Builder.setMemberName
* -- * ServerLauncher.Builder.setHostnameForClients
*
* Notes:
* -- StringUtils.wraps may be inherited from Apache Commons when the dependency is updated.
* -- AbstractLauncher.getMember has the documented behavior of returning null when both MemberName and ID are blank. Is this the best behavior for this method?
======
I'm going through the diff myself now and noticing a lot of refactorings that happen in dead code. I'm going back to cull the dead code entirely, but wanted to go ahead and get this diff posted.
Diffs
-----
geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsDUnitTest.java 5277e57ae4d5d3901fddb627edfb11e4145a13f8
geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsTest.java 0554f69adbea10f30b5dac1580e7d87ec786d228
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/controllers/RestAPITestBase.java f2e90a44a3d08070aa1869df543f5b647cb18b22
geode-core/src/main/java/org/apache/geode/cache/AttributesFactory.java 69f108714e42a9bf970a10f157491e36fa2b5dd3
geode-core/src/main/java/org/apache/geode/cache/client/internal/locator/LocatorStatusResponse.java d531cc15529a9389cd1ebbcffd769e9d0b6f3e94
geode-core/src/main/java/org/apache/geode/cache/query/internal/AttributeDescriptor.java f40ab3e5ecffc0bbd3c0f92e84a512cc5b34f496
geode-core/src/main/java/org/apache/geode/cache/query/internal/CumulativeNonDistinctResults.java 2565dd3db4f27df205c4f141d728c7ee441850d3
geode-core/src/main/java/org/apache/geode/cache/query/internal/NWayMergeResults.java 4e52a670924b437d464d23f45bf8541c86d847e9
geode-core/src/main/java/org/apache/geode/cache/query/internal/QRegion.java 5f0d6e580576926283aa47b0005a34c15c03a20b
geode-core/src/main/java/org/apache/geode/cache/query/internal/types/TypeUtils.java 6c2399a63ec365abdc4513f6cf44a626d14963ad
geode-core/src/main/java/org/apache/geode/distributed/AbstractLauncher.java feba8937b216d374acd8357775d5d1806568b355
geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java 641e009d5e9fe646684fe65d91110805fe589b91
geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java b2d01511e4868b7055521901d378aa5efaefe203
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java ad5e04d4f11085cb3477da6a13aec0167761ed35
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java 4bf010b42fc6d4d20392bba883cb802846ee1bcf
geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java 78b294475a49e568b267f69c4c6bdecd4338f4cb
geode-core/src/main/java/org/apache/geode/internal/ObjIdConcurrentMap.java 17894ad51428112c4c635b307a25047a8420621a
geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java af3cb5d4f60b68ca19fe6dfdf52d0cdcbcc2c44d
geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java 55e354262e35f69e6e2182b5c28b573cf8fa282d
geode-core/src/main/java/org/apache/geode/internal/lang/StringUtils.java 499d546c828ce8ffa1d56f7d3cba8325d3486702
geode-core/src/main/java/org/apache/geode/internal/lang/SystemUtils.java 3485ede5955d2a3acad9b5c54ee5003596bd07f3
geode-core/src/main/java/org/apache/geode/internal/process/FileProcessController.java 629740c4fafa89c2a332d5a27293d42b7834ee34
geode-core/src/main/java/org/apache/geode/internal/process/signal/Signal.java faab4edbfca3461fa6a6182ea3f44577743e8dd4
geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java 8366930a3bf6a244d077c745fc810d77c4699c38
geode-core/src/main/java/org/apache/geode/internal/util/ArrayUtils.java 6f1c7cc80b22f3e30c6900df6b28c86ef0421fd5
geode-core/src/main/java/org/apache/geode/internal/util/IOUtils.java c1a1952621c00476163fc8aef397830889f1225d
geode-core/src/main/java/org/apache/geode/management/internal/AgentUtil.java ba155086183696bedcbb2feaa1546ba4a245a66d
geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java 7c262973907a1f1ce8650a332704534653b48856
geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java 62310e88aa9158b22b7d3fd977461813acbe038a
geode-core/src/main/java/org/apache/geode/management/internal/RestAgent.java 837e81555e9ba1588d3546dfd4ec4343e813ac55
geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemBridge.java ef643ac02a7d422060c56fd5b1d0b76724334d79
geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemMBean.java a87b3668cde7688df5fe818477450ded0d56458c
geode-core/src/main/java/org/apache/geode/management/internal/beans/ManagementAdapter.java 7dce60213ea12ce6a6e36e926ef9a4a00d32bac3
geode-core/src/main/java/org/apache/geode/management/internal/beans/QueryDataFunction.java f701d2979b175e8a02a241cf3a4761d6c1b4d096
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigCommands.java 5dfc1b83b56884bd5a265280d5a6116fc2dbddff
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java 5b0651e91650c9ae56e0094036891ac6c684a477
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/IndexCommands.java 9110a1a3c501127cf2d0a11d2b5d8474a7167990
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java 101bae4fb7599cdb01e34593d494d3e205502ae8
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java c9e79b0c258f02eedbb05d19c8747a409b7ef323
geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/GatewaySenderIdConverter.java 59851daf45cabfec107c56bb1327a27557c19fe2
geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/DataCommandResult.java 423d781248458a450c2e98cd8a0e609c8656edb0
geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/IndexDetails.java 64bb5128372f90e8a85d5556847303d8b25e49b1
geode-core/src/main/java/org/apache/geode/management/internal/cli/help/HelpBlock.java 4383044959de629b9caa8f26527b00f6e2c2b0b3
geode-core/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java 9dbf59c140394ce74fe950d57ec247206ce7c765
geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ResultBuilder.java 9bd2bf98c24e0a21e530f0dfbe6726163c689592
geode-core/src/main/java/org/apache/geode/management/internal/cli/result/TableBuilder.java da3c4aea56ca0134d4c9f3824ca4ab3f617c21cf
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategy.java 7c80e0d270eea429adf04972a9c07d6f75d2dcde
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java e9d183e4731d3d50935f57653c5054b8403cfdbc
geode-core/src/main/java/org/apache/geode/management/internal/cli/util/CommandStringBuilder.java 4410fea0be74d438d107f9def74e34a5fc838903
geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/XmlEntity.java be74e84289c218b533443931db270c22ba4f06e1
geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationRequest.java 837d99e52b914f5fbd01927a68ff6ce39517babf
geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationResponse.java 3248c98b05f7e47ffb331cab3418221a3af33e6b
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/AbstractCommandsController.java 0b64a4477bb00821798f91f15d43848ee5587ccb
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ConfigCommandsController.java f468c65c072f9684e2b16c8b84832273df9421ce
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DataCommandsController.java 3a8ed82eab68204dab75b557fa1aaf32ed409e04
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DeployCommandsController.java fe5be621a020fd9dde303e45deeb09da12f3972b
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DiskStoreCommandsController.java 661340c91d95f319be777ebb58659bbc7dc7c668
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DurableClientCommandsController.java 1d718b4c986c96fec2c261820f17e5765532d1a7
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java 604bdee9f30bea1fff42007cf87dbe5f2f2e26b0
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/FunctionCommandsController.java 77cfb402a309a43108b752674755b1bc6f24be61
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/MiscellaneousCommandsController.java d19aee13e8c88cbf139a1b344e3e01df242a2a06
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/PdxCommandsController.java c68ee3592c86931abb5197e633da0b4e02518483
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/QueueCommandsController.java 396726e0b7a7e54030309db2f0c0c6b9e550cefe
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/RegionCommandsController.java e503e561d951c3fb58259009e22f2e164a9c5630
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ShellCommandsController.java 0ecb77f9c413aaff1e945011f4e16cf6efb13fc2
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/WanCommandsController.java fa5aa57a829118b49b76d2a791822aa03dcaf1de
geode-core/src/main/java/org/apache/geode/management/internal/web/domain/Link.java cef8cabf109e958937a17b46e8d9935980de9511
geode-core/src/main/java/org/apache/geode/management/internal/web/domain/LinkIndex.java 7d5bb379d1df719fea672e2bec10197121f60e62
geode-core/src/main/java/org/apache/geode/management/internal/web/http/HttpHeader.java ee7e932bcbc09952700cf423cd6a9287d46f7835
geode-core/src/main/java/org/apache/geode/management/internal/web/shell/RestHttpOperationInvoker.java eeedf4033e832b2422a81ce42a2f48b3575fda51
geode-core/src/main/java/org/apache/geode/management/internal/web/util/UriUtils.java b83064e8d146028166b51159266813f1b87d1a96
geode-core/src/test/java/org/apache/geode/cache/client/internal/LocatorTestBase.java c3b349a5f889b393552be7196b501e2cc5da5c66
geode-core/src/test/java/org/apache/geode/cache30/MultiVMRegionTestCase.java ff996a279958131ba612d2b76d3e53a165916d9d
geode-core/src/test/java/org/apache/geode/distributed/AbstractLauncherTest.java 62d4bdd443d02aef910074e8b20d3963d7d1d301
geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherIntegrationTest.java 5b53c6a33f2a269880232f726642ad2bebcc5976
geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherTest.java 06d6054f524a4fd31fc8b8189b24aa3def10babd
geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherIntegrationTest.java 7bd7462e1e8394f5f8235d7a9c7fa6954735d6a7
geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherTest.java f5d6271c3a3d598973b28c7423f609961dbc8272
geode-core/src/test/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java ec4e179709efe21bec09a0c8fa2bfc8153915ea8
geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionAPIDUnitTest.java 0b18ebce48fa4c6fa3ce648cfd6040cfaecf8206
geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionSingleNodeOperationsJUnitTest.java e69aa956f6a02f7a47d3c2622d937d13e98154cc
geode-core/src/test/java/org/apache/geode/internal/cache/execute/Bug51193DUnitTest.java 0dfbe6cc1580d890104d907e8d9efb388b892c24
geode-core/src/test/java/org/apache/geode/internal/lang/ClassUtilsJUnitTest.java 3ec3b067bfc978268a16c68fb497e418daab0b1a
geode-core/src/test/java/org/apache/geode/internal/lang/StringUtilsJUnitTest.java f3e0abed3241b95cfd473649e5b3d94d908104a7
geode-core/src/test/java/org/apache/geode/internal/util/CollectionUtilsJUnitTest.java 36be8b8e99549ea250491edbe9242afa075b7798
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AbstractCommandsSupportJUnitTest.java 91a59f87947baac724a2a9863d16ab16bdf8aa2b
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListIndexCommandDUnitTest.java 8bf1c4318190f42d83ef9c4c9ee1ff6fdfecdea0
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java fc920c43d3dbe88f93227194eb23e69a513c3d05
geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommands.java b2be12afb53df95acd6832032922efaa769e672c
geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommandsJUnitTest.java d4dca4adefe2070cca3d91abbb36723c615bbd34
geode-pulse/src/test/java/org/apache/geode/tools/pulse/testbed/PropMockDataUpdater.java 6ee5b53744f31df90ff442e0997efeff3634c032
geode-spark-connector/geode-functions/src/main/java/org/apache/geode/spark/connector/internal/geodefunctions/RetrieveRegionFunction.java 096e4d5cb71c1043c7c0754a02252b05b52ba9f3
geode-web/src/test/java/org/apache/geode/management/internal/web/AbstractWebTestCase.java eac0b8d4f577bf5e80e75ed583fdd01297540e14
geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/ShellCommandsControllerJUnitTest.java 37ec508b12280939f5be3f0310c47481b5e81195
geode-web/src/test/java/org/apache/geode/management/internal/web/shell/RestHttpOperationInvokerJUnitTest.java c69013badbdee87a06a01938ba5be2918c1961b4
Diff: https://reviews.apache.org/r/59323/diff/1/
Testing
-------
Precheckin running. Tests and LegacyTests already returned green.
Thanks,
Patrick Rhomberg
Re: Review Request 59323: GEODE-1994: Overhaul of
internal.lang.StringUtils
to extend and heavily use commons.lang.StringUtils.
Posted by Patrick Rhomberg <pr...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59323/#review175167
-----------------------------------------------------------
There are a few files that I've cleaned up a little, but it appears that any changes I made w.r.t. StringUtils were reverted. Should I keep those cleanups in place, or should I prefer to touch fewer files?
geode-core/src/main/java/org/apache/geode/cache/AttributesFactory.java
Lines 1514-1518 (original), 1514-1518 (patched)
<https://reviews.apache.org/r/59323/#comment248558>
Remove dead code.
geode-core/src/main/java/org/apache/geode/cache/query/internal/CumulativeNonDistinctResults.java
Lines 198-200 (original), 198-201 (patched)
<https://reviews.apache.org/r/59323/#comment248559>
Remove dead code.
geode-core/src/main/java/org/apache/geode/cache/query/internal/NWayMergeResults.java
Lines 214-216 (original), 214-217 (patched)
<https://reviews.apache.org/r/59323/#comment248560>
Remove dead code.
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
Line 325 (original), 303-309 (patched)
<https://reviews.apache.org/r/59323/#comment248563>
hostnameForClients can potentially be null.
A null memberName gets reassigned and so that block can be removed.
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
Line 325 (original), 303-309 (patched)
<https://reviews.apache.org/r/59323/#comment248564>
hostnameForClients can potentially be null, which would raise an error in the set.
A null memberName gets reassigned and so that block can be removed.
- Patrick Rhomberg
On May 16, 2017, 10:29 p.m., Patrick Rhomberg wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59323/
> -----------------------------------------------------------
>
> (Updated May 16, 2017, 10:29 p.m.)
>
>
> Review request for geode, Jinmei Liao, Jared Stewart, Ken Howe, Kirk Lund, and Swapnil Bawaskar.
>
>
> Repository: geode
>
>
> Description
> -------
>
> * Renamed:
> * -- EMPTY_STRING -> EMPTY (inherited)
> * -- toUpperCase -> upperCase (inherited)
> * -- toLowerCase -> lowerCase (inherited)
> * -- padEnding -> rightPad (inherited)
> *
> * Removed:
> * -- EMPTY_STRING_ARRAY; usage replaced with commons.lang.ArrayUtils.EMPTY_STRING_ARRAY
> * -- SPACES
> * -- UTF_8; rare usage replaced with raw string
> * -- concat; usage replaced with commons.lang.join, refactoring as necessary.
> * -- getLettersOnly
> * -- getSpaces
> * -- truncate
> * -- valueOf; usage refactored to use defaultString
> *
> * Refactored
> * -- defaultIfBlank: previously relied on varargs and could return null. Usage refactored to allow inheritance from commons.
> * -- defaultString(s, EMPTY) refactored to use standard signature defaultString(s) for consistency throughout codebase.
> * -- isBlank: usage refactored to resolve discrepancies with commons.lang.isBlank, which is now inherited.
> * -- isEmpty: usage refactored to resolve discrepancies with commons.lang.isEmpty, which is now inherited.
> *
> * Code Cleanup:
> * -- Many uses of !isBlank -> isNotBlank
> * -- Changes suggested by Inspections on most touched files.
> * -- Explicit <T> -> <> when type is inferable
> * -- while loops operating on iterators converted to for each loops
> * -- for loops operating on array indices converted to for each loops
> * -- Various string typos corrected.
> * -- isEmpty(s.trim()) -> isBlank(s)
> * -- s.trim().isEmpty() -> isEmpty(s)
> * -- Removed some instances of 'dead' code
> *
> * Qualitative Changes:
> * -- The following functions now throw an error when called with a null string input:
> * -- * LocatorLauncher.Builder.setMemberName
> * -- * ServerLauncher.Builder.setMemberName
> * -- * ServerLauncher.Builder.setHostnameForClients
> *
> * Notes:
> * -- StringUtils.wraps may be inherited from Apache Commons when the dependency is updated.
> * -- AbstractLauncher.getMember has the documented behavior of returning null when both MemberName and ID are blank. Is this the best behavior for this method?
>
> ======
>
> I'm going through the diff myself now and noticing a lot of refactorings that happen in dead code. I'm going back to cull the dead code entirely, but wanted to go ahead and get this diff posted.
>
>
> Diffs
> -----
>
> geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsDUnitTest.java 5277e57ae4d5d3901fddb627edfb11e4145a13f8
> geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsTest.java 0554f69adbea10f30b5dac1580e7d87ec786d228
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/controllers/RestAPITestBase.java f2e90a44a3d08070aa1869df543f5b647cb18b22
> geode-core/src/main/java/org/apache/geode/cache/AttributesFactory.java 69f108714e42a9bf970a10f157491e36fa2b5dd3
> geode-core/src/main/java/org/apache/geode/cache/client/internal/locator/LocatorStatusResponse.java d531cc15529a9389cd1ebbcffd769e9d0b6f3e94
> geode-core/src/main/java/org/apache/geode/cache/query/internal/AttributeDescriptor.java f40ab3e5ecffc0bbd3c0f92e84a512cc5b34f496
> geode-core/src/main/java/org/apache/geode/cache/query/internal/CumulativeNonDistinctResults.java 2565dd3db4f27df205c4f141d728c7ee441850d3
> geode-core/src/main/java/org/apache/geode/cache/query/internal/NWayMergeResults.java 4e52a670924b437d464d23f45bf8541c86d847e9
> geode-core/src/main/java/org/apache/geode/cache/query/internal/QRegion.java 5f0d6e580576926283aa47b0005a34c15c03a20b
> geode-core/src/main/java/org/apache/geode/cache/query/internal/types/TypeUtils.java 6c2399a63ec365abdc4513f6cf44a626d14963ad
> geode-core/src/main/java/org/apache/geode/distributed/AbstractLauncher.java feba8937b216d374acd8357775d5d1806568b355
> geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java 641e009d5e9fe646684fe65d91110805fe589b91
> geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java b2d01511e4868b7055521901d378aa5efaefe203
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java ad5e04d4f11085cb3477da6a13aec0167761ed35
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java 4bf010b42fc6d4d20392bba883cb802846ee1bcf
> geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java 78b294475a49e568b267f69c4c6bdecd4338f4cb
> geode-core/src/main/java/org/apache/geode/internal/ObjIdConcurrentMap.java 17894ad51428112c4c635b307a25047a8420621a
> geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java af3cb5d4f60b68ca19fe6dfdf52d0cdcbcc2c44d
> geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java 55e354262e35f69e6e2182b5c28b573cf8fa282d
> geode-core/src/main/java/org/apache/geode/internal/lang/StringUtils.java 499d546c828ce8ffa1d56f7d3cba8325d3486702
> geode-core/src/main/java/org/apache/geode/internal/lang/SystemUtils.java 3485ede5955d2a3acad9b5c54ee5003596bd07f3
> geode-core/src/main/java/org/apache/geode/internal/process/FileProcessController.java 629740c4fafa89c2a332d5a27293d42b7834ee34
> geode-core/src/main/java/org/apache/geode/internal/process/signal/Signal.java faab4edbfca3461fa6a6182ea3f44577743e8dd4
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java 8366930a3bf6a244d077c745fc810d77c4699c38
> geode-core/src/main/java/org/apache/geode/internal/util/ArrayUtils.java 6f1c7cc80b22f3e30c6900df6b28c86ef0421fd5
> geode-core/src/main/java/org/apache/geode/internal/util/IOUtils.java c1a1952621c00476163fc8aef397830889f1225d
> geode-core/src/main/java/org/apache/geode/management/internal/AgentUtil.java ba155086183696bedcbb2feaa1546ba4a245a66d
> geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java 7c262973907a1f1ce8650a332704534653b48856
> geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java 62310e88aa9158b22b7d3fd977461813acbe038a
> geode-core/src/main/java/org/apache/geode/management/internal/RestAgent.java 837e81555e9ba1588d3546dfd4ec4343e813ac55
> geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemBridge.java ef643ac02a7d422060c56fd5b1d0b76724334d79
> geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemMBean.java a87b3668cde7688df5fe818477450ded0d56458c
> geode-core/src/main/java/org/apache/geode/management/internal/beans/ManagementAdapter.java 7dce60213ea12ce6a6e36e926ef9a4a00d32bac3
> geode-core/src/main/java/org/apache/geode/management/internal/beans/QueryDataFunction.java f701d2979b175e8a02a241cf3a4761d6c1b4d096
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigCommands.java 5dfc1b83b56884bd5a265280d5a6116fc2dbddff
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java 5b0651e91650c9ae56e0094036891ac6c684a477
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/IndexCommands.java 9110a1a3c501127cf2d0a11d2b5d8474a7167990
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java 101bae4fb7599cdb01e34593d494d3e205502ae8
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java c9e79b0c258f02eedbb05d19c8747a409b7ef323
> geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/GatewaySenderIdConverter.java 59851daf45cabfec107c56bb1327a27557c19fe2
> geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/DataCommandResult.java 423d781248458a450c2e98cd8a0e609c8656edb0
> geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/IndexDetails.java 64bb5128372f90e8a85d5556847303d8b25e49b1
> geode-core/src/main/java/org/apache/geode/management/internal/cli/help/HelpBlock.java 4383044959de629b9caa8f26527b00f6e2c2b0b3
> geode-core/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java 9dbf59c140394ce74fe950d57ec247206ce7c765
> geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ResultBuilder.java 9bd2bf98c24e0a21e530f0dfbe6726163c689592
> geode-core/src/main/java/org/apache/geode/management/internal/cli/result/TableBuilder.java da3c4aea56ca0134d4c9f3824ca4ab3f617c21cf
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategy.java 7c80e0d270eea429adf04972a9c07d6f75d2dcde
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java e9d183e4731d3d50935f57653c5054b8403cfdbc
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/CommandStringBuilder.java 4410fea0be74d438d107f9def74e34a5fc838903
> geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/XmlEntity.java be74e84289c218b533443931db270c22ba4f06e1
> geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationRequest.java 837d99e52b914f5fbd01927a68ff6ce39517babf
> geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationResponse.java 3248c98b05f7e47ffb331cab3418221a3af33e6b
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/AbstractCommandsController.java 0b64a4477bb00821798f91f15d43848ee5587ccb
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ConfigCommandsController.java f468c65c072f9684e2b16c8b84832273df9421ce
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DataCommandsController.java 3a8ed82eab68204dab75b557fa1aaf32ed409e04
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DeployCommandsController.java fe5be621a020fd9dde303e45deeb09da12f3972b
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DiskStoreCommandsController.java 661340c91d95f319be777ebb58659bbc7dc7c668
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DurableClientCommandsController.java 1d718b4c986c96fec2c261820f17e5765532d1a7
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java 604bdee9f30bea1fff42007cf87dbe5f2f2e26b0
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/FunctionCommandsController.java 77cfb402a309a43108b752674755b1bc6f24be61
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/MiscellaneousCommandsController.java d19aee13e8c88cbf139a1b344e3e01df242a2a06
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/PdxCommandsController.java c68ee3592c86931abb5197e633da0b4e02518483
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/QueueCommandsController.java 396726e0b7a7e54030309db2f0c0c6b9e550cefe
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/RegionCommandsController.java e503e561d951c3fb58259009e22f2e164a9c5630
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ShellCommandsController.java 0ecb77f9c413aaff1e945011f4e16cf6efb13fc2
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/WanCommandsController.java fa5aa57a829118b49b76d2a791822aa03dcaf1de
> geode-core/src/main/java/org/apache/geode/management/internal/web/domain/Link.java cef8cabf109e958937a17b46e8d9935980de9511
> geode-core/src/main/java/org/apache/geode/management/internal/web/domain/LinkIndex.java 7d5bb379d1df719fea672e2bec10197121f60e62
> geode-core/src/main/java/org/apache/geode/management/internal/web/http/HttpHeader.java ee7e932bcbc09952700cf423cd6a9287d46f7835
> geode-core/src/main/java/org/apache/geode/management/internal/web/shell/RestHttpOperationInvoker.java eeedf4033e832b2422a81ce42a2f48b3575fda51
> geode-core/src/main/java/org/apache/geode/management/internal/web/util/UriUtils.java b83064e8d146028166b51159266813f1b87d1a96
> geode-core/src/test/java/org/apache/geode/cache/client/internal/LocatorTestBase.java c3b349a5f889b393552be7196b501e2cc5da5c66
> geode-core/src/test/java/org/apache/geode/cache30/MultiVMRegionTestCase.java ff996a279958131ba612d2b76d3e53a165916d9d
> geode-core/src/test/java/org/apache/geode/distributed/AbstractLauncherTest.java 62d4bdd443d02aef910074e8b20d3963d7d1d301
> geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherIntegrationTest.java 5b53c6a33f2a269880232f726642ad2bebcc5976
> geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherTest.java 06d6054f524a4fd31fc8b8189b24aa3def10babd
> geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherIntegrationTest.java 7bd7462e1e8394f5f8235d7a9c7fa6954735d6a7
> geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherTest.java f5d6271c3a3d598973b28c7423f609961dbc8272
> geode-core/src/test/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java ec4e179709efe21bec09a0c8fa2bfc8153915ea8
> geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionAPIDUnitTest.java 0b18ebce48fa4c6fa3ce648cfd6040cfaecf8206
> geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionSingleNodeOperationsJUnitTest.java e69aa956f6a02f7a47d3c2622d937d13e98154cc
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/Bug51193DUnitTest.java 0dfbe6cc1580d890104d907e8d9efb388b892c24
> geode-core/src/test/java/org/apache/geode/internal/lang/ClassUtilsJUnitTest.java 3ec3b067bfc978268a16c68fb497e418daab0b1a
> geode-core/src/test/java/org/apache/geode/internal/lang/StringUtilsJUnitTest.java f3e0abed3241b95cfd473649e5b3d94d908104a7
> geode-core/src/test/java/org/apache/geode/internal/util/CollectionUtilsJUnitTest.java 36be8b8e99549ea250491edbe9242afa075b7798
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AbstractCommandsSupportJUnitTest.java 91a59f87947baac724a2a9863d16ab16bdf8aa2b
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListIndexCommandDUnitTest.java 8bf1c4318190f42d83ef9c4c9ee1ff6fdfecdea0
> geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java fc920c43d3dbe88f93227194eb23e69a513c3d05
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommands.java b2be12afb53df95acd6832032922efaa769e672c
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommandsJUnitTest.java d4dca4adefe2070cca3d91abbb36723c615bbd34
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/testbed/PropMockDataUpdater.java 6ee5b53744f31df90ff442e0997efeff3634c032
> geode-spark-connector/geode-functions/src/main/java/org/apache/geode/spark/connector/internal/geodefunctions/RetrieveRegionFunction.java 096e4d5cb71c1043c7c0754a02252b05b52ba9f3
> geode-web/src/test/java/org/apache/geode/management/internal/web/AbstractWebTestCase.java eac0b8d4f577bf5e80e75ed583fdd01297540e14
> geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/ShellCommandsControllerJUnitTest.java 37ec508b12280939f5be3f0310c47481b5e81195
> geode-web/src/test/java/org/apache/geode/management/internal/web/shell/RestHttpOperationInvokerJUnitTest.java c69013badbdee87a06a01938ba5be2918c1961b4
>
>
> Diff: https://reviews.apache.org/r/59323/diff/1/
>
>
> Testing
> -------
>
> Precheckin running. Tests and LegacyTests already returned green.
>
>
> Thanks,
>
> Patrick Rhomberg
>
>
Re: Review Request 59323: GEODE-1994: Overhaul of
internal.lang.StringUtils
to extend and heavily use commons.lang.StringUtils.
Posted by Jared Stewart <js...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59323/#review175280
-----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/cache/query/internal/AttributeDescriptor.java
Line 224 (original), 224 (patched)
<https://reviews.apache.org/r/59323/#comment248725>
Looks like this comment may have been changed accidentally. Actually, since we're already looking around in this class, I think all of these "Not yet used" method comments should be deleted.
geode-core/src/main/java/org/apache/geode/cache/query/internal/NWayMergeResults.java
Line 214 (original), 214 (patched)
<https://reviews.apache.org/r/59323/#comment248726>
Delete this comment
geode-core/src/main/java/org/apache/geode/distributed/AbstractLauncher.java
Line 504 (original), 507 (patched)
<https://reviews.apache.org/r/59323/#comment248728>
There is no need to pass the empty string into this constructor, `new StringBuilder();` will suffice.
geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherTest.java
Line 157 (original)
<https://reviews.apache.org/r/59323/#comment248745>
Did you intended to remove these assertions?
geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherTest.java
Line 194 (original)
<https://reviews.apache.org/r/59323/#comment248746>
Did you intended to remove these assertions?
geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherTest.java
Line 165 (original)
<https://reviews.apache.org/r/59323/#comment248747>
Did you intended to remove these assertions?
geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherTest.java
Line 280 (original)
<https://reviews.apache.org/r/59323/#comment248748>
Did you intended to remove these assertions?
- Jared Stewart
On May 16, 2017, 11:25 p.m., Patrick Rhomberg wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59323/
> -----------------------------------------------------------
>
> (Updated May 16, 2017, 11:25 p.m.)
>
>
> Review request for geode, Jinmei Liao, Jared Stewart, Ken Howe, Kirk Lund, and Swapnil Bawaskar.
>
>
> Repository: geode
>
>
> Description
> -------
>
> * Renamed:
> * -- EMPTY_STRING -> EMPTY (inherited)
> * -- toUpperCase -> upperCase (inherited)
> * -- toLowerCase -> lowerCase (inherited)
> * -- padEnding -> rightPad (inherited)
> *
> * Removed:
> * -- EMPTY_STRING_ARRAY; usage replaced with commons.lang.ArrayUtils.EMPTY_STRING_ARRAY
> * -- SPACES
> * -- UTF_8; rare usage replaced with raw string
> * -- concat; usage replaced with commons.lang.join, refactoring as necessary.
> * -- getLettersOnly
> * -- getSpaces
> * -- truncate
> * -- valueOf; usage refactored to use defaultString
> *
> * Refactored
> * -- defaultIfBlank: previously relied on varargs and could return null. Usage refactored to allow inheritance from commons.
> * -- defaultString(s, EMPTY) refactored to use standard signature defaultString(s) for consistency throughout codebase.
> * -- isBlank: usage refactored to resolve discrepancies with commons.lang.isBlank, which is now inherited.
> * -- isEmpty: usage refactored to resolve discrepancies with commons.lang.isEmpty, which is now inherited.
> *
> * Code Cleanup:
> * -- Many uses of !isBlank -> isNotBlank
> * -- Changes suggested by Inspections on most touched files.
> * -- Explicit <T> -> <> when type is inferable
> * -- while loops operating on iterators converted to for each loops
> * -- for loops operating on array indices converted to for each loops
> * -- Various string typos corrected.
> * -- isEmpty(s.trim()) -> isBlank(s)
> * -- s.trim().isEmpty() -> isEmpty(s)
> * -- Removed some instances of 'dead' code
> *
> * Qualitative Changes:
> * -- The following functions now throw an error when called with a null string input:
> * -- * LocatorLauncher.Builder.setMemberName
> * -- * ServerLauncher.Builder.setMemberName
> * -- * ServerLauncher.Builder.setHostnameForClients
> *
> * Notes:
> * -- StringUtils.wraps may be inherited from Apache Commons when the dependency is updated.
> * -- AbstractLauncher.getMember has the documented behavior of returning null when both MemberName and ID are blank. Is this the best behavior for this method?
>
> ======
>
> I'm going through the diff myself now and noticing a lot of refactorings that happen in dead code. I'm going back to cull the dead code entirely, but wanted to go ahead and get this diff posted.
>
>
> Diffs
> -----
>
> geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsDUnitTest.java 5277e57
> geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsTest.java 0554f69
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/controllers/RestAPITestBase.java f2e90a4
> geode-core/src/main/java/org/apache/geode/cache/client/internal/locator/LocatorStatusResponse.java d531cc1
> geode-core/src/main/java/org/apache/geode/cache/query/internal/AttributeDescriptor.java f40ab3e
> geode-core/src/main/java/org/apache/geode/cache/query/internal/NWayMergeResults.java 4e52a67
> geode-core/src/main/java/org/apache/geode/distributed/AbstractLauncher.java feba893
> geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java 641e009
> geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java b2d0151
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java ad5e04d
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java 4bf010b
> geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java 78b2944
> geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java af3cb5d
> geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java 55e3542
> geode-core/src/main/java/org/apache/geode/internal/lang/StringUtils.java 499d546
> geode-core/src/main/java/org/apache/geode/internal/lang/SystemUtils.java 3485ede
> geode-core/src/main/java/org/apache/geode/internal/process/FileProcessController.java 629740c
> geode-core/src/main/java/org/apache/geode/internal/process/signal/Signal.java faab4ed
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java 8366930
> geode-core/src/main/java/org/apache/geode/internal/util/ArrayUtils.java 6f1c7cc
> geode-core/src/main/java/org/apache/geode/internal/util/IOUtils.java c1a1952
> geode-core/src/main/java/org/apache/geode/management/internal/AgentUtil.java ba15508
> geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java 7c26297
> geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java 62310e8
> geode-core/src/main/java/org/apache/geode/management/internal/RestAgent.java 837e815
> geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemBridge.java ef643ac
> geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemMBean.java a87b366
> geode-core/src/main/java/org/apache/geode/management/internal/beans/ManagementAdapter.java 7dce602
> geode-core/src/main/java/org/apache/geode/management/internal/beans/QueryDataFunction.java f701d29
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigCommands.java 5dfc1b8
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java 5b0651e
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/IndexCommands.java 9110a1a
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java 101bae4
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java c9e79b0
> geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/GatewaySenderIdConverter.java 59851da
> geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/IndexDetails.java 64bb512
> geode-core/src/main/java/org/apache/geode/management/internal/cli/help/HelpBlock.java 4383044
> geode-core/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java 9dbf59c
> geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ResultBuilder.java 9bd2bf9
> geode-core/src/main/java/org/apache/geode/management/internal/cli/result/TableBuilder.java da3c4ae
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategy.java 7c80e0d
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java e9d183e
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/CommandStringBuilder.java 4410fea
> geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/XmlEntity.java be74e84
> geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationRequest.java 837d99e
> geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationResponse.java 3248c98
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/AbstractCommandsController.java 0b64a44
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ConfigCommandsController.java f468c65
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DataCommandsController.java 3a8ed82
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DeployCommandsController.java fe5be62
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DiskStoreCommandsController.java 661340c
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DurableClientCommandsController.java 1d718b4
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java 604bdee
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/FunctionCommandsController.java 77cfb40
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/MiscellaneousCommandsController.java d19aee1
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/PdxCommandsController.java c68ee35
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/QueueCommandsController.java 396726e
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/RegionCommandsController.java e503e56
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ShellCommandsController.java 0ecb77f
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/WanCommandsController.java fa5aa57
> geode-core/src/main/java/org/apache/geode/management/internal/web/domain/Link.java cef8cab
> geode-core/src/main/java/org/apache/geode/management/internal/web/domain/LinkIndex.java 7d5bb37
> geode-core/src/main/java/org/apache/geode/management/internal/web/http/HttpHeader.java ee7e932
> geode-core/src/main/java/org/apache/geode/management/internal/web/shell/RestHttpOperationInvoker.java eeedf40
> geode-core/src/main/java/org/apache/geode/management/internal/web/util/UriUtils.java b83064e
> geode-core/src/test/java/org/apache/geode/cache/client/internal/LocatorTestBase.java c3b349a
> geode-core/src/test/java/org/apache/geode/distributed/AbstractLauncherTest.java 62d4bdd
> geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherIntegrationTest.java 5b53c6a
> geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherTest.java 06d6054
> geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherIntegrationTest.java 7bd7462
> geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherTest.java f5d6271
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/Bug51193DUnitTest.java 0dfbe6c
> geode-core/src/test/java/org/apache/geode/internal/lang/ClassUtilsJUnitTest.java 3ec3b06
> geode-core/src/test/java/org/apache/geode/internal/lang/StringUtilsJUnitTest.java f3e0abe
> geode-core/src/test/java/org/apache/geode/internal/util/CollectionUtilsJUnitTest.java 36be8b8
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AbstractCommandsSupportJUnitTest.java 91a59f8
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListIndexCommandDUnitTest.java 8bf1c43
> geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java fc920c4
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommands.java b2be12a
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommandsJUnitTest.java d4dca4a
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/testbed/PropMockDataUpdater.java 6ee5b53
> geode-spark-connector/geode-functions/src/main/java/org/apache/geode/spark/connector/internal/geodefunctions/RetrieveRegionFunction.java 096e4d5
> geode-web/src/test/java/org/apache/geode/management/internal/web/AbstractWebTestCase.java eac0b8d
> geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/ShellCommandsControllerJUnitTest.java 37ec508
> geode-web/src/test/java/org/apache/geode/management/internal/web/shell/RestHttpOperationInvokerJUnitTest.java c69013b
>
>
> Diff: https://reviews.apache.org/r/59323/diff/2/
>
>
> Testing
> -------
>
> Precheckin running. Tests and LegacyTests already returned green.
>
>
> Thanks,
>
> Patrick Rhomberg
>
>
Re: Review Request 59323: GEODE-1994: Overhaul of
internal.lang.StringUtils
to extend and heavily use commons.lang.StringUtils.
Posted by Kirk Lund <ki...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59323/#review175428
-----------------------------------------------------------
Ship it!
Ship It!
- Kirk Lund
On May 18, 2017, midnight, Patrick Rhomberg wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59323/
> -----------------------------------------------------------
>
> (Updated May 18, 2017, midnight)
>
>
> Review request for geode, Jinmei Liao, Jared Stewart, Ken Howe, Kirk Lund, and Swapnil Bawaskar.
>
>
> Repository: geode
>
>
> Description
> -------
>
> * geode.internal.lang.StringUtils has been deprecated. In the interim, it has been heavily refactored and extends commons.lang.StringUtils.
> *
> * Renamed:
> * -- EMPTY_STRING -> EMPTY (inherited)
> * -- toUpperCase -> upperCase (inherited)
> * -- toLowerCase -> lowerCase (inherited)
> * -- padEnding -> rightPad (inherited)
> *
> * Removed:
> * -- EMPTY_STRING_ARRAY; usage replaced with commons.lang.ArrayUtils.EMPTY_STRING_ARRAY
> * -- SPACES
> * -- UTF_8; rare usage replaced with raw string
> * -- concat; usage replaced with commons.lang.join, refactoring as necessary.
> * -- getLettersOnly
> * -- getSpaces
> * -- truncate
> * -- valueOf; usage refactored to use defaultString
> *
> * Refactored
> * -- defaultIfBlank: previously relied on varargs and could return null. Usage refactored to allow inheritance from commons.
> * -- defaultString(s, EMPTY) refactored to use standard signature defaultString(s) for consistency throughout codebase.
> * -- isBlank: usage refactored to resolve discrepancies with commons.lang.isBlank, which is now inherited.
> * -- isEmpty: usage refactored to resolve discrepancies with commons.lang.isEmpty, which is now inherited.
> *
> * Code Cleanup:
> * -- Many uses of !isBlank -> isNotBlank
> * -- Changes suggested by Inspections on most touched files.
> * -- Explicit <T> -> <> when type is inferable
> * -- while loops operating on iterators converted to for each loops
> * -- for loops operating on array indices converted to for each loops
> * -- Various string typos corrected.
> * -- isEmpty(s.trim()) -> isBlank(s)
> * -- s.trim().isEmpty() -> isEmpty(s)
> * -- Removed some instances of 'dead' code
> *
> * Qualitative Changes:
> * -- The following functions now throw an error when called with a null string input:
> * -- * LocatorLauncher.Builder.setMemberName
> * -- * ServerLauncher.Builder.setMemberName
> * -- * ServerLauncher.Builder.setHostnameForClients
> * -- (Unit tests added to capture these changes)
> *
> * Notes:
> * -- StringUtils.wraps may be inherited from Apache Commons when the dependency is updated.
> * -- AbstractLauncher.getMember has the documented behavior of returning null when both MemberName and ID are blank. Is this the best behavior for this method?
>
> ======
>
> I'm going through the diff myself now and noticing a lot of refactorings that happen in dead code. I'm going back to cull the dead code entirely, but wanted to go ahead and get this diff posted.
>
>
> Diffs
> -----
>
> geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsDUnitTest.java 5277e57
> geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsTest.java 0554f69
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/controllers/RestAPITestBase.java f2e90a4
> geode-core/src/main/java/org/apache/geode/cache/client/internal/locator/LocatorStatusResponse.java d531cc1
> geode-core/src/main/java/org/apache/geode/cache/query/internal/AttributeDescriptor.java f40ab3e
> geode-core/src/main/java/org/apache/geode/cache/query/internal/NWayMergeResults.java 4e52a67
> geode-core/src/main/java/org/apache/geode/distributed/AbstractLauncher.java feba893
> geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java 641e009
> geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java b2d0151
> geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterConfigurationService.java 10623b4
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java ad5e04d
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java 4bf010b
> geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java 78b2944
> geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java af3cb5d
> geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java 55e3542
> geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java 185fde7
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java 0b11bf1
> geode-core/src/main/java/org/apache/geode/internal/lang/StringUtils.java 499d546
> geode-core/src/main/java/org/apache/geode/internal/lang/SystemUtils.java 2e47556
> geode-core/src/main/java/org/apache/geode/internal/process/FileProcessController.java 629740c
> geode-core/src/main/java/org/apache/geode/internal/process/signal/Signal.java faab4ed
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java 8366930
> geode-core/src/main/java/org/apache/geode/internal/util/ArrayUtils.java 6f1c7cc
> geode-core/src/main/java/org/apache/geode/internal/util/IOUtils.java c1a1952
> geode-core/src/main/java/org/apache/geode/management/internal/AgentUtil.java ba15508
> geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java 7c26297
> geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java 62310e8
> geode-core/src/main/java/org/apache/geode/management/internal/RestAgent.java 837e815
> geode-core/src/main/java/org/apache/geode/management/internal/SSLUtil.java 1b39b73
> geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemBridge.java ef643ac
> geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemMBean.java a87b366
> geode-core/src/main/java/org/apache/geode/management/internal/beans/ManagementAdapter.java 7dce602
> geode-core/src/main/java/org/apache/geode/management/internal/beans/QueryDataFunction.java f701d29
> geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java bd6d810
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/AbstractCommandsSupport.java ae44e24
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigCommands.java 5dfc1b8
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java 5b0651e
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportImportClusterConfigurationCommands.java dfd20a9
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/IndexCommands.java 9110a1a
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java 101bae4
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java c9e79b0
> geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/FilePathStringConverter.java 3fadd96
> geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/GatewaySenderIdConverter.java 59851da
> geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/DiskStoreDetails.java b5d1b0c
> geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/IndexDetails.java 64bb512
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DescribeDiskStoreFunction.java 4c241d0
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/FetchSharedConfigurationStatusFunction.java 57d209b
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java c99f84a
> geode-core/src/main/java/org/apache/geode/management/internal/cli/help/HelpBlock.java 4383044
> geode-core/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java 9dbf59c
> geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ResultBuilder.java 9bd2bf9
> geode-core/src/main/java/org/apache/geode/management/internal/cli/result/TableBuilder.java da3c4ae
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/GfshConfig.java c35f420
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategy.java 7c80e0d
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java e9d183e
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/CommandStringBuilder.java 4410fea
> geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/XmlEntity.java 0dbe7e5
> geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationRequest.java 837d99e
> geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationResponse.java 3248c98
> geode-core/src/main/java/org/apache/geode/management/internal/configuration/utils/XmlUtils.java 218762c
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/AbstractCommandsController.java 0b64a44
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ConfigCommandsController.java f468c65
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DataCommandsController.java 3a8ed82
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DeployCommandsController.java fe5be62
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DiskStoreCommandsController.java 661340c
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DurableClientCommandsController.java 1d718b4
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java 604bdee
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/FunctionCommandsController.java 77cfb40
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/MiscellaneousCommandsController.java d19aee1
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/PdxCommandsController.java c68ee35
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/QueueCommandsController.java 396726e
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/RegionCommandsController.java e503e56
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ShellCommandsController.java 0ecb77f
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/WanCommandsController.java fa5aa57
> geode-core/src/main/java/org/apache/geode/management/internal/web/domain/Link.java cef8cab
> geode-core/src/main/java/org/apache/geode/management/internal/web/domain/LinkIndex.java 7d5bb37
> geode-core/src/main/java/org/apache/geode/management/internal/web/http/HttpHeader.java ee7e932
> geode-core/src/main/java/org/apache/geode/management/internal/web/shell/RestHttpOperationInvoker.java eeedf40
> geode-core/src/main/java/org/apache/geode/management/internal/web/util/ConvertUtils.java f3b092d
> geode-core/src/main/java/org/apache/geode/management/internal/web/util/UriUtils.java b83064e
> geode-core/src/test/java/org/apache/geode/cache/client/internal/LocatorTestBase.java c3b349a
> geode-core/src/test/java/org/apache/geode/distributed/AbstractLauncherIntegrationTestCase.java 09fa09e
> geode-core/src/test/java/org/apache/geode/distributed/AbstractLauncherTest.java 62d4bdd
> geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherIntegrationTest.java 5b53c6a
> geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherTest.java 06d6054
> geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherIntegrationTest.java 7bd7462
> geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherTest.java f5d6271
> geode-core/src/test/java/org/apache/geode/internal/lang/ClassUtilsJUnitTest.java 3ec3b06
> geode-core/src/test/java/org/apache/geode/internal/lang/StringUtilsJUnitTest.java f3e0abe
> geode-core/src/test/java/org/apache/geode/internal/util/CollectionUtilsJUnitTest.java 36be8b8
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AbstractCommandsSupportJUnitTest.java 91a59f8
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListIndexCommandDUnitTest.java 8bf1c43
> geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java fc920c4
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java 19a1662
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommands.java b2be12a
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/functions/LuceneCreateIndexFunction.java f7edd8f
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommandsJUnitTest.java d4dca4a
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/testbed/PropMockDataUpdater.java 6ee5b53
> geode-spark-connector/geode-functions/src/main/java/org/apache/geode/spark/connector/internal/geodefunctions/RetrieveRegionFunction.java 096e4d5
> geode-web/src/test/java/org/apache/geode/management/internal/web/AbstractWebTestCase.java eac0b8d
> geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/ShellCommandsControllerJUnitTest.java 37ec508
> geode-web/src/test/java/org/apache/geode/management/internal/web/shell/RestHttpOperationInvokerJUnitTest.java c69013b
>
>
> Diff: https://reviews.apache.org/r/59323/diff/3/
>
>
> Testing
> -------
>
> Precheckin running. Tests and LegacyTests already returned green.
>
>
> Thanks,
>
> Patrick Rhomberg
>
>
Re: Review Request 59323: GEODE-1994: Overhaul of
internal.lang.StringUtils
to extend and heavily use commons.lang.StringUtils.
Posted by Patrick Rhomberg <pr...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59323/
-----------------------------------------------------------
(Updated May 18, 2017, midnight)
Review request for geode, Jinmei Liao, Jared Stewart, Ken Howe, Kirk Lund, and Swapnil Bawaskar.
Changes
-------
Updated w.r.t. requested changes. Optimized imports in all touched files.
Repository: geode
Description
-------
* geode.internal.lang.StringUtils has been deprecated. In the interim, it has been heavily refactored and extends commons.lang.StringUtils.
*
* Renamed:
* -- EMPTY_STRING -> EMPTY (inherited)
* -- toUpperCase -> upperCase (inherited)
* -- toLowerCase -> lowerCase (inherited)
* -- padEnding -> rightPad (inherited)
*
* Removed:
* -- EMPTY_STRING_ARRAY; usage replaced with commons.lang.ArrayUtils.EMPTY_STRING_ARRAY
* -- SPACES
* -- UTF_8; rare usage replaced with raw string
* -- concat; usage replaced with commons.lang.join, refactoring as necessary.
* -- getLettersOnly
* -- getSpaces
* -- truncate
* -- valueOf; usage refactored to use defaultString
*
* Refactored
* -- defaultIfBlank: previously relied on varargs and could return null. Usage refactored to allow inheritance from commons.
* -- defaultString(s, EMPTY) refactored to use standard signature defaultString(s) for consistency throughout codebase.
* -- isBlank: usage refactored to resolve discrepancies with commons.lang.isBlank, which is now inherited.
* -- isEmpty: usage refactored to resolve discrepancies with commons.lang.isEmpty, which is now inherited.
*
* Code Cleanup:
* -- Many uses of !isBlank -> isNotBlank
* -- Changes suggested by Inspections on most touched files.
* -- Explicit <T> -> <> when type is inferable
* -- while loops operating on iterators converted to for each loops
* -- for loops operating on array indices converted to for each loops
* -- Various string typos corrected.
* -- isEmpty(s.trim()) -> isBlank(s)
* -- s.trim().isEmpty() -> isEmpty(s)
* -- Removed some instances of 'dead' code
*
* Qualitative Changes:
* -- The following functions now throw an error when called with a null string input:
* -- * LocatorLauncher.Builder.setMemberName
* -- * ServerLauncher.Builder.setMemberName
* -- * ServerLauncher.Builder.setHostnameForClients
* -- (Unit tests added to capture these changes)
*
* Notes:
* -- StringUtils.wraps may be inherited from Apache Commons when the dependency is updated.
* -- AbstractLauncher.getMember has the documented behavior of returning null when both MemberName and ID are blank. Is this the best behavior for this method?
======
I'm going through the diff myself now and noticing a lot of refactorings that happen in dead code. I'm going back to cull the dead code entirely, but wanted to go ahead and get this diff posted.
Diffs (updated)
-----
geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsDUnitTest.java 5277e57
geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsTest.java 0554f69
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/controllers/RestAPITestBase.java f2e90a4
geode-core/src/main/java/org/apache/geode/cache/client/internal/locator/LocatorStatusResponse.java d531cc1
geode-core/src/main/java/org/apache/geode/cache/query/internal/AttributeDescriptor.java f40ab3e
geode-core/src/main/java/org/apache/geode/cache/query/internal/NWayMergeResults.java 4e52a67
geode-core/src/main/java/org/apache/geode/distributed/AbstractLauncher.java feba893
geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java 641e009
geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java b2d0151
geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterConfigurationService.java 10623b4
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java ad5e04d
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java 4bf010b
geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java 78b2944
geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java af3cb5d
geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java 55e3542
geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java 185fde7
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java 0b11bf1
geode-core/src/main/java/org/apache/geode/internal/lang/StringUtils.java 499d546
geode-core/src/main/java/org/apache/geode/internal/lang/SystemUtils.java 2e47556
geode-core/src/main/java/org/apache/geode/internal/process/FileProcessController.java 629740c
geode-core/src/main/java/org/apache/geode/internal/process/signal/Signal.java faab4ed
geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java 8366930
geode-core/src/main/java/org/apache/geode/internal/util/ArrayUtils.java 6f1c7cc
geode-core/src/main/java/org/apache/geode/internal/util/IOUtils.java c1a1952
geode-core/src/main/java/org/apache/geode/management/internal/AgentUtil.java ba15508
geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java 7c26297
geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java 62310e8
geode-core/src/main/java/org/apache/geode/management/internal/RestAgent.java 837e815
geode-core/src/main/java/org/apache/geode/management/internal/SSLUtil.java 1b39b73
geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemBridge.java ef643ac
geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemMBean.java a87b366
geode-core/src/main/java/org/apache/geode/management/internal/beans/ManagementAdapter.java 7dce602
geode-core/src/main/java/org/apache/geode/management/internal/beans/QueryDataFunction.java f701d29
geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java bd6d810
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/AbstractCommandsSupport.java ae44e24
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigCommands.java 5dfc1b8
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java 5b0651e
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportImportClusterConfigurationCommands.java dfd20a9
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/IndexCommands.java 9110a1a
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java 101bae4
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java c9e79b0
geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/FilePathStringConverter.java 3fadd96
geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/GatewaySenderIdConverter.java 59851da
geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/DiskStoreDetails.java b5d1b0c
geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/IndexDetails.java 64bb512
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DescribeDiskStoreFunction.java 4c241d0
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/FetchSharedConfigurationStatusFunction.java 57d209b
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java c99f84a
geode-core/src/main/java/org/apache/geode/management/internal/cli/help/HelpBlock.java 4383044
geode-core/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java 9dbf59c
geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ResultBuilder.java 9bd2bf9
geode-core/src/main/java/org/apache/geode/management/internal/cli/result/TableBuilder.java da3c4ae
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/GfshConfig.java c35f420
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategy.java 7c80e0d
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java e9d183e
geode-core/src/main/java/org/apache/geode/management/internal/cli/util/CommandStringBuilder.java 4410fea
geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/XmlEntity.java 0dbe7e5
geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationRequest.java 837d99e
geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationResponse.java 3248c98
geode-core/src/main/java/org/apache/geode/management/internal/configuration/utils/XmlUtils.java 218762c
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/AbstractCommandsController.java 0b64a44
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ConfigCommandsController.java f468c65
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DataCommandsController.java 3a8ed82
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DeployCommandsController.java fe5be62
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DiskStoreCommandsController.java 661340c
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DurableClientCommandsController.java 1d718b4
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java 604bdee
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/FunctionCommandsController.java 77cfb40
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/MiscellaneousCommandsController.java d19aee1
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/PdxCommandsController.java c68ee35
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/QueueCommandsController.java 396726e
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/RegionCommandsController.java e503e56
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ShellCommandsController.java 0ecb77f
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/WanCommandsController.java fa5aa57
geode-core/src/main/java/org/apache/geode/management/internal/web/domain/Link.java cef8cab
geode-core/src/main/java/org/apache/geode/management/internal/web/domain/LinkIndex.java 7d5bb37
geode-core/src/main/java/org/apache/geode/management/internal/web/http/HttpHeader.java ee7e932
geode-core/src/main/java/org/apache/geode/management/internal/web/shell/RestHttpOperationInvoker.java eeedf40
geode-core/src/main/java/org/apache/geode/management/internal/web/util/ConvertUtils.java f3b092d
geode-core/src/main/java/org/apache/geode/management/internal/web/util/UriUtils.java b83064e
geode-core/src/test/java/org/apache/geode/cache/client/internal/LocatorTestBase.java c3b349a
geode-core/src/test/java/org/apache/geode/distributed/AbstractLauncherIntegrationTestCase.java 09fa09e
geode-core/src/test/java/org/apache/geode/distributed/AbstractLauncherTest.java 62d4bdd
geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherIntegrationTest.java 5b53c6a
geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherTest.java 06d6054
geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherIntegrationTest.java 7bd7462
geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherTest.java f5d6271
geode-core/src/test/java/org/apache/geode/internal/lang/ClassUtilsJUnitTest.java 3ec3b06
geode-core/src/test/java/org/apache/geode/internal/lang/StringUtilsJUnitTest.java f3e0abe
geode-core/src/test/java/org/apache/geode/internal/util/CollectionUtilsJUnitTest.java 36be8b8
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AbstractCommandsSupportJUnitTest.java 91a59f8
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListIndexCommandDUnitTest.java 8bf1c43
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java fc920c4
geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java 19a1662
geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommands.java b2be12a
geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/functions/LuceneCreateIndexFunction.java f7edd8f
geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommandsJUnitTest.java d4dca4a
geode-pulse/src/test/java/org/apache/geode/tools/pulse/testbed/PropMockDataUpdater.java 6ee5b53
geode-spark-connector/geode-functions/src/main/java/org/apache/geode/spark/connector/internal/geodefunctions/RetrieveRegionFunction.java 096e4d5
geode-web/src/test/java/org/apache/geode/management/internal/web/AbstractWebTestCase.java eac0b8d
geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/ShellCommandsControllerJUnitTest.java 37ec508
geode-web/src/test/java/org/apache/geode/management/internal/web/shell/RestHttpOperationInvokerJUnitTest.java c69013b
Diff: https://reviews.apache.org/r/59323/diff/3/
Changes: https://reviews.apache.org/r/59323/diff/2-3/
Testing
-------
Precheckin running. Tests and LegacyTests already returned green.
Thanks,
Patrick Rhomberg
Re: Review Request 59323: GEODE-1994: Overhaul of
internal.lang.StringUtils
to extend and heavily use commons.lang.StringUtils.
Posted by Jinmei Liao <ji...@pivotal.io>.
> On May 17, 2017, 5:32 p.m., Jinmei Liao wrote:
> > geode-core/src/main/java/org/apache/geode/internal/lang/StringUtils.java
> > Line 29 (original), 29 (patched)
> > <https://reviews.apache.org/r/59323/diff/2/?file=1721981#file1721981line29>
> >
> > I would not make this class extends apache common's StringUtils. We should always encourage devleopers to use the apache's String utils, and only for very specific cases, we would want to use our own.
You can make this change after you commited this. I would imagine this would touch more files.
- Jinmei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59323/#review175278
-----------------------------------------------------------
On May 16, 2017, 11:25 p.m., Patrick Rhomberg wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59323/
> -----------------------------------------------------------
>
> (Updated May 16, 2017, 11:25 p.m.)
>
>
> Review request for geode, Jinmei Liao, Jared Stewart, Ken Howe, Kirk Lund, and Swapnil Bawaskar.
>
>
> Repository: geode
>
>
> Description
> -------
>
> * Renamed:
> * -- EMPTY_STRING -> EMPTY (inherited)
> * -- toUpperCase -> upperCase (inherited)
> * -- toLowerCase -> lowerCase (inherited)
> * -- padEnding -> rightPad (inherited)
> *
> * Removed:
> * -- EMPTY_STRING_ARRAY; usage replaced with commons.lang.ArrayUtils.EMPTY_STRING_ARRAY
> * -- SPACES
> * -- UTF_8; rare usage replaced with raw string
> * -- concat; usage replaced with commons.lang.join, refactoring as necessary.
> * -- getLettersOnly
> * -- getSpaces
> * -- truncate
> * -- valueOf; usage refactored to use defaultString
> *
> * Refactored
> * -- defaultIfBlank: previously relied on varargs and could return null. Usage refactored to allow inheritance from commons.
> * -- defaultString(s, EMPTY) refactored to use standard signature defaultString(s) for consistency throughout codebase.
> * -- isBlank: usage refactored to resolve discrepancies with commons.lang.isBlank, which is now inherited.
> * -- isEmpty: usage refactored to resolve discrepancies with commons.lang.isEmpty, which is now inherited.
> *
> * Code Cleanup:
> * -- Many uses of !isBlank -> isNotBlank
> * -- Changes suggested by Inspections on most touched files.
> * -- Explicit <T> -> <> when type is inferable
> * -- while loops operating on iterators converted to for each loops
> * -- for loops operating on array indices converted to for each loops
> * -- Various string typos corrected.
> * -- isEmpty(s.trim()) -> isBlank(s)
> * -- s.trim().isEmpty() -> isEmpty(s)
> * -- Removed some instances of 'dead' code
> *
> * Qualitative Changes:
> * -- The following functions now throw an error when called with a null string input:
> * -- * LocatorLauncher.Builder.setMemberName
> * -- * ServerLauncher.Builder.setMemberName
> * -- * ServerLauncher.Builder.setHostnameForClients
> *
> * Notes:
> * -- StringUtils.wraps may be inherited from Apache Commons when the dependency is updated.
> * -- AbstractLauncher.getMember has the documented behavior of returning null when both MemberName and ID are blank. Is this the best behavior for this method?
>
> ======
>
> I'm going through the diff myself now and noticing a lot of refactorings that happen in dead code. I'm going back to cull the dead code entirely, but wanted to go ahead and get this diff posted.
>
>
> Diffs
> -----
>
> geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsDUnitTest.java 5277e57
> geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsTest.java 0554f69
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/controllers/RestAPITestBase.java f2e90a4
> geode-core/src/main/java/org/apache/geode/cache/client/internal/locator/LocatorStatusResponse.java d531cc1
> geode-core/src/main/java/org/apache/geode/cache/query/internal/AttributeDescriptor.java f40ab3e
> geode-core/src/main/java/org/apache/geode/cache/query/internal/NWayMergeResults.java 4e52a67
> geode-core/src/main/java/org/apache/geode/distributed/AbstractLauncher.java feba893
> geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java 641e009
> geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java b2d0151
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java ad5e04d
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java 4bf010b
> geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java 78b2944
> geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java af3cb5d
> geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java 55e3542
> geode-core/src/main/java/org/apache/geode/internal/lang/StringUtils.java 499d546
> geode-core/src/main/java/org/apache/geode/internal/lang/SystemUtils.java 3485ede
> geode-core/src/main/java/org/apache/geode/internal/process/FileProcessController.java 629740c
> geode-core/src/main/java/org/apache/geode/internal/process/signal/Signal.java faab4ed
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java 8366930
> geode-core/src/main/java/org/apache/geode/internal/util/ArrayUtils.java 6f1c7cc
> geode-core/src/main/java/org/apache/geode/internal/util/IOUtils.java c1a1952
> geode-core/src/main/java/org/apache/geode/management/internal/AgentUtil.java ba15508
> geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java 7c26297
> geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java 62310e8
> geode-core/src/main/java/org/apache/geode/management/internal/RestAgent.java 837e815
> geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemBridge.java ef643ac
> geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemMBean.java a87b366
> geode-core/src/main/java/org/apache/geode/management/internal/beans/ManagementAdapter.java 7dce602
> geode-core/src/main/java/org/apache/geode/management/internal/beans/QueryDataFunction.java f701d29
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigCommands.java 5dfc1b8
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java 5b0651e
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/IndexCommands.java 9110a1a
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java 101bae4
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java c9e79b0
> geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/GatewaySenderIdConverter.java 59851da
> geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/IndexDetails.java 64bb512
> geode-core/src/main/java/org/apache/geode/management/internal/cli/help/HelpBlock.java 4383044
> geode-core/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java 9dbf59c
> geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ResultBuilder.java 9bd2bf9
> geode-core/src/main/java/org/apache/geode/management/internal/cli/result/TableBuilder.java da3c4ae
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategy.java 7c80e0d
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java e9d183e
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/CommandStringBuilder.java 4410fea
> geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/XmlEntity.java be74e84
> geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationRequest.java 837d99e
> geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationResponse.java 3248c98
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/AbstractCommandsController.java 0b64a44
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ConfigCommandsController.java f468c65
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DataCommandsController.java 3a8ed82
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DeployCommandsController.java fe5be62
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DiskStoreCommandsController.java 661340c
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DurableClientCommandsController.java 1d718b4
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java 604bdee
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/FunctionCommandsController.java 77cfb40
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/MiscellaneousCommandsController.java d19aee1
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/PdxCommandsController.java c68ee35
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/QueueCommandsController.java 396726e
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/RegionCommandsController.java e503e56
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ShellCommandsController.java 0ecb77f
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/WanCommandsController.java fa5aa57
> geode-core/src/main/java/org/apache/geode/management/internal/web/domain/Link.java cef8cab
> geode-core/src/main/java/org/apache/geode/management/internal/web/domain/LinkIndex.java 7d5bb37
> geode-core/src/main/java/org/apache/geode/management/internal/web/http/HttpHeader.java ee7e932
> geode-core/src/main/java/org/apache/geode/management/internal/web/shell/RestHttpOperationInvoker.java eeedf40
> geode-core/src/main/java/org/apache/geode/management/internal/web/util/UriUtils.java b83064e
> geode-core/src/test/java/org/apache/geode/cache/client/internal/LocatorTestBase.java c3b349a
> geode-core/src/test/java/org/apache/geode/distributed/AbstractLauncherTest.java 62d4bdd
> geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherIntegrationTest.java 5b53c6a
> geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherTest.java 06d6054
> geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherIntegrationTest.java 7bd7462
> geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherTest.java f5d6271
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/Bug51193DUnitTest.java 0dfbe6c
> geode-core/src/test/java/org/apache/geode/internal/lang/ClassUtilsJUnitTest.java 3ec3b06
> geode-core/src/test/java/org/apache/geode/internal/lang/StringUtilsJUnitTest.java f3e0abe
> geode-core/src/test/java/org/apache/geode/internal/util/CollectionUtilsJUnitTest.java 36be8b8
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AbstractCommandsSupportJUnitTest.java 91a59f8
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListIndexCommandDUnitTest.java 8bf1c43
> geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java fc920c4
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommands.java b2be12a
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommandsJUnitTest.java d4dca4a
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/testbed/PropMockDataUpdater.java 6ee5b53
> geode-spark-connector/geode-functions/src/main/java/org/apache/geode/spark/connector/internal/geodefunctions/RetrieveRegionFunction.java 096e4d5
> geode-web/src/test/java/org/apache/geode/management/internal/web/AbstractWebTestCase.java eac0b8d
> geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/ShellCommandsControllerJUnitTest.java 37ec508
> geode-web/src/test/java/org/apache/geode/management/internal/web/shell/RestHttpOperationInvokerJUnitTest.java c69013b
>
>
> Diff: https://reviews.apache.org/r/59323/diff/2/
>
>
> Testing
> -------
>
> Precheckin running. Tests and LegacyTests already returned green.
>
>
> Thanks,
>
> Patrick Rhomberg
>
>
Re: Review Request 59323: GEODE-1994: Overhaul of
internal.lang.StringUtils
to extend and heavily use commons.lang.StringUtils.
Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59323/#review175278
-----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/internal/lang/StringUtils.java
Line 29 (original), 29 (patched)
<https://reviews.apache.org/r/59323/#comment248722>
I would not make this class extends apache common's StringUtils. We should always encourage devleopers to use the apache's String utils, and only for very specific cases, we would want to use our own.
- Jinmei Liao
On May 16, 2017, 11:25 p.m., Patrick Rhomberg wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59323/
> -----------------------------------------------------------
>
> (Updated May 16, 2017, 11:25 p.m.)
>
>
> Review request for geode, Jinmei Liao, Jared Stewart, Ken Howe, Kirk Lund, and Swapnil Bawaskar.
>
>
> Repository: geode
>
>
> Description
> -------
>
> * Renamed:
> * -- EMPTY_STRING -> EMPTY (inherited)
> * -- toUpperCase -> upperCase (inherited)
> * -- toLowerCase -> lowerCase (inherited)
> * -- padEnding -> rightPad (inherited)
> *
> * Removed:
> * -- EMPTY_STRING_ARRAY; usage replaced with commons.lang.ArrayUtils.EMPTY_STRING_ARRAY
> * -- SPACES
> * -- UTF_8; rare usage replaced with raw string
> * -- concat; usage replaced with commons.lang.join, refactoring as necessary.
> * -- getLettersOnly
> * -- getSpaces
> * -- truncate
> * -- valueOf; usage refactored to use defaultString
> *
> * Refactored
> * -- defaultIfBlank: previously relied on varargs and could return null. Usage refactored to allow inheritance from commons.
> * -- defaultString(s, EMPTY) refactored to use standard signature defaultString(s) for consistency throughout codebase.
> * -- isBlank: usage refactored to resolve discrepancies with commons.lang.isBlank, which is now inherited.
> * -- isEmpty: usage refactored to resolve discrepancies with commons.lang.isEmpty, which is now inherited.
> *
> * Code Cleanup:
> * -- Many uses of !isBlank -> isNotBlank
> * -- Changes suggested by Inspections on most touched files.
> * -- Explicit <T> -> <> when type is inferable
> * -- while loops operating on iterators converted to for each loops
> * -- for loops operating on array indices converted to for each loops
> * -- Various string typos corrected.
> * -- isEmpty(s.trim()) -> isBlank(s)
> * -- s.trim().isEmpty() -> isEmpty(s)
> * -- Removed some instances of 'dead' code
> *
> * Qualitative Changes:
> * -- The following functions now throw an error when called with a null string input:
> * -- * LocatorLauncher.Builder.setMemberName
> * -- * ServerLauncher.Builder.setMemberName
> * -- * ServerLauncher.Builder.setHostnameForClients
> *
> * Notes:
> * -- StringUtils.wraps may be inherited from Apache Commons when the dependency is updated.
> * -- AbstractLauncher.getMember has the documented behavior of returning null when both MemberName and ID are blank. Is this the best behavior for this method?
>
> ======
>
> I'm going through the diff myself now and noticing a lot of refactorings that happen in dead code. I'm going back to cull the dead code entirely, but wanted to go ahead and get this diff posted.
>
>
> Diffs
> -----
>
> geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsDUnitTest.java 5277e57
> geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsTest.java 0554f69
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/controllers/RestAPITestBase.java f2e90a4
> geode-core/src/main/java/org/apache/geode/cache/client/internal/locator/LocatorStatusResponse.java d531cc1
> geode-core/src/main/java/org/apache/geode/cache/query/internal/AttributeDescriptor.java f40ab3e
> geode-core/src/main/java/org/apache/geode/cache/query/internal/NWayMergeResults.java 4e52a67
> geode-core/src/main/java/org/apache/geode/distributed/AbstractLauncher.java feba893
> geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java 641e009
> geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java b2d0151
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java ad5e04d
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java 4bf010b
> geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java 78b2944
> geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java af3cb5d
> geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java 55e3542
> geode-core/src/main/java/org/apache/geode/internal/lang/StringUtils.java 499d546
> geode-core/src/main/java/org/apache/geode/internal/lang/SystemUtils.java 3485ede
> geode-core/src/main/java/org/apache/geode/internal/process/FileProcessController.java 629740c
> geode-core/src/main/java/org/apache/geode/internal/process/signal/Signal.java faab4ed
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java 8366930
> geode-core/src/main/java/org/apache/geode/internal/util/ArrayUtils.java 6f1c7cc
> geode-core/src/main/java/org/apache/geode/internal/util/IOUtils.java c1a1952
> geode-core/src/main/java/org/apache/geode/management/internal/AgentUtil.java ba15508
> geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java 7c26297
> geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java 62310e8
> geode-core/src/main/java/org/apache/geode/management/internal/RestAgent.java 837e815
> geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemBridge.java ef643ac
> geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemMBean.java a87b366
> geode-core/src/main/java/org/apache/geode/management/internal/beans/ManagementAdapter.java 7dce602
> geode-core/src/main/java/org/apache/geode/management/internal/beans/QueryDataFunction.java f701d29
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigCommands.java 5dfc1b8
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java 5b0651e
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/IndexCommands.java 9110a1a
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java 101bae4
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java c9e79b0
> geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/GatewaySenderIdConverter.java 59851da
> geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/IndexDetails.java 64bb512
> geode-core/src/main/java/org/apache/geode/management/internal/cli/help/HelpBlock.java 4383044
> geode-core/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java 9dbf59c
> geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ResultBuilder.java 9bd2bf9
> geode-core/src/main/java/org/apache/geode/management/internal/cli/result/TableBuilder.java da3c4ae
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategy.java 7c80e0d
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java e9d183e
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/CommandStringBuilder.java 4410fea
> geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/XmlEntity.java be74e84
> geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationRequest.java 837d99e
> geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationResponse.java 3248c98
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/AbstractCommandsController.java 0b64a44
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ConfigCommandsController.java f468c65
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DataCommandsController.java 3a8ed82
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DeployCommandsController.java fe5be62
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DiskStoreCommandsController.java 661340c
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DurableClientCommandsController.java 1d718b4
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java 604bdee
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/FunctionCommandsController.java 77cfb40
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/MiscellaneousCommandsController.java d19aee1
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/PdxCommandsController.java c68ee35
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/QueueCommandsController.java 396726e
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/RegionCommandsController.java e503e56
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ShellCommandsController.java 0ecb77f
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/WanCommandsController.java fa5aa57
> geode-core/src/main/java/org/apache/geode/management/internal/web/domain/Link.java cef8cab
> geode-core/src/main/java/org/apache/geode/management/internal/web/domain/LinkIndex.java 7d5bb37
> geode-core/src/main/java/org/apache/geode/management/internal/web/http/HttpHeader.java ee7e932
> geode-core/src/main/java/org/apache/geode/management/internal/web/shell/RestHttpOperationInvoker.java eeedf40
> geode-core/src/main/java/org/apache/geode/management/internal/web/util/UriUtils.java b83064e
> geode-core/src/test/java/org/apache/geode/cache/client/internal/LocatorTestBase.java c3b349a
> geode-core/src/test/java/org/apache/geode/distributed/AbstractLauncherTest.java 62d4bdd
> geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherIntegrationTest.java 5b53c6a
> geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherTest.java 06d6054
> geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherIntegrationTest.java 7bd7462
> geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherTest.java f5d6271
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/Bug51193DUnitTest.java 0dfbe6c
> geode-core/src/test/java/org/apache/geode/internal/lang/ClassUtilsJUnitTest.java 3ec3b06
> geode-core/src/test/java/org/apache/geode/internal/lang/StringUtilsJUnitTest.java f3e0abe
> geode-core/src/test/java/org/apache/geode/internal/util/CollectionUtilsJUnitTest.java 36be8b8
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AbstractCommandsSupportJUnitTest.java 91a59f8
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListIndexCommandDUnitTest.java 8bf1c43
> geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java fc920c4
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommands.java b2be12a
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommandsJUnitTest.java d4dca4a
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/testbed/PropMockDataUpdater.java 6ee5b53
> geode-spark-connector/geode-functions/src/main/java/org/apache/geode/spark/connector/internal/geodefunctions/RetrieveRegionFunction.java 096e4d5
> geode-web/src/test/java/org/apache/geode/management/internal/web/AbstractWebTestCase.java eac0b8d
> geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/ShellCommandsControllerJUnitTest.java 37ec508
> geode-web/src/test/java/org/apache/geode/management/internal/web/shell/RestHttpOperationInvokerJUnitTest.java c69013b
>
>
> Diff: https://reviews.apache.org/r/59323/diff/2/
>
>
> Testing
> -------
>
> Precheckin running. Tests and LegacyTests already returned green.
>
>
> Thanks,
>
> Patrick Rhomberg
>
>
Re: Review Request 59323: GEODE-1994: Overhaul of
internal.lang.StringUtils
to extend and heavily use commons.lang.StringUtils.
Posted by Patrick Rhomberg <pr...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59323/
-----------------------------------------------------------
(Updated May 16, 2017, 11:25 p.m.)
Review request for geode, Jinmei Liao, Jared Stewart, Ken Howe, Kirk Lund, and Swapnil Bawaskar.
Changes
-------
Reverted a few files that were touched during aggressive name refactoring. Purged dead code from internal StringUtils.
Repository: geode
Description
-------
* Renamed:
* -- EMPTY_STRING -> EMPTY (inherited)
* -- toUpperCase -> upperCase (inherited)
* -- toLowerCase -> lowerCase (inherited)
* -- padEnding -> rightPad (inherited)
*
* Removed:
* -- EMPTY_STRING_ARRAY; usage replaced with commons.lang.ArrayUtils.EMPTY_STRING_ARRAY
* -- SPACES
* -- UTF_8; rare usage replaced with raw string
* -- concat; usage replaced with commons.lang.join, refactoring as necessary.
* -- getLettersOnly
* -- getSpaces
* -- truncate
* -- valueOf; usage refactored to use defaultString
*
* Refactored
* -- defaultIfBlank: previously relied on varargs and could return null. Usage refactored to allow inheritance from commons.
* -- defaultString(s, EMPTY) refactored to use standard signature defaultString(s) for consistency throughout codebase.
* -- isBlank: usage refactored to resolve discrepancies with commons.lang.isBlank, which is now inherited.
* -- isEmpty: usage refactored to resolve discrepancies with commons.lang.isEmpty, which is now inherited.
*
* Code Cleanup:
* -- Many uses of !isBlank -> isNotBlank
* -- Changes suggested by Inspections on most touched files.
* -- Explicit <T> -> <> when type is inferable
* -- while loops operating on iterators converted to for each loops
* -- for loops operating on array indices converted to for each loops
* -- Various string typos corrected.
* -- isEmpty(s.trim()) -> isBlank(s)
* -- s.trim().isEmpty() -> isEmpty(s)
* -- Removed some instances of 'dead' code
*
* Qualitative Changes:
* -- The following functions now throw an error when called with a null string input:
* -- * LocatorLauncher.Builder.setMemberName
* -- * ServerLauncher.Builder.setMemberName
* -- * ServerLauncher.Builder.setHostnameForClients
*
* Notes:
* -- StringUtils.wraps may be inherited from Apache Commons when the dependency is updated.
* -- AbstractLauncher.getMember has the documented behavior of returning null when both MemberName and ID are blank. Is this the best behavior for this method?
======
I'm going through the diff myself now and noticing a lot of refactorings that happen in dead code. I'm going back to cull the dead code entirely, but wanted to go ahead and get this diff posted.
Diffs (updated)
-----
geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsDUnitTest.java 5277e57
geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsTest.java 0554f69
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/controllers/RestAPITestBase.java f2e90a4
geode-core/src/main/java/org/apache/geode/cache/client/internal/locator/LocatorStatusResponse.java d531cc1
geode-core/src/main/java/org/apache/geode/cache/query/internal/AttributeDescriptor.java f40ab3e
geode-core/src/main/java/org/apache/geode/cache/query/internal/NWayMergeResults.java 4e52a67
geode-core/src/main/java/org/apache/geode/distributed/AbstractLauncher.java feba893
geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java 641e009
geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java b2d0151
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java ad5e04d
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java 4bf010b
geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java 78b2944
geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java af3cb5d
geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java 55e3542
geode-core/src/main/java/org/apache/geode/internal/lang/StringUtils.java 499d546
geode-core/src/main/java/org/apache/geode/internal/lang/SystemUtils.java 3485ede
geode-core/src/main/java/org/apache/geode/internal/process/FileProcessController.java 629740c
geode-core/src/main/java/org/apache/geode/internal/process/signal/Signal.java faab4ed
geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java 8366930
geode-core/src/main/java/org/apache/geode/internal/util/ArrayUtils.java 6f1c7cc
geode-core/src/main/java/org/apache/geode/internal/util/IOUtils.java c1a1952
geode-core/src/main/java/org/apache/geode/management/internal/AgentUtil.java ba15508
geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java 7c26297
geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java 62310e8
geode-core/src/main/java/org/apache/geode/management/internal/RestAgent.java 837e815
geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemBridge.java ef643ac
geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemMBean.java a87b366
geode-core/src/main/java/org/apache/geode/management/internal/beans/ManagementAdapter.java 7dce602
geode-core/src/main/java/org/apache/geode/management/internal/beans/QueryDataFunction.java f701d29
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigCommands.java 5dfc1b8
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java 5b0651e
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/IndexCommands.java 9110a1a
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java 101bae4
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java c9e79b0
geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/GatewaySenderIdConverter.java 59851da
geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/IndexDetails.java 64bb512
geode-core/src/main/java/org/apache/geode/management/internal/cli/help/HelpBlock.java 4383044
geode-core/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java 9dbf59c
geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ResultBuilder.java 9bd2bf9
geode-core/src/main/java/org/apache/geode/management/internal/cli/result/TableBuilder.java da3c4ae
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategy.java 7c80e0d
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java e9d183e
geode-core/src/main/java/org/apache/geode/management/internal/cli/util/CommandStringBuilder.java 4410fea
geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/XmlEntity.java be74e84
geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationRequest.java 837d99e
geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationResponse.java 3248c98
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/AbstractCommandsController.java 0b64a44
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ConfigCommandsController.java f468c65
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DataCommandsController.java 3a8ed82
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DeployCommandsController.java fe5be62
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DiskStoreCommandsController.java 661340c
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DurableClientCommandsController.java 1d718b4
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java 604bdee
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/FunctionCommandsController.java 77cfb40
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/MiscellaneousCommandsController.java d19aee1
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/PdxCommandsController.java c68ee35
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/QueueCommandsController.java 396726e
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/RegionCommandsController.java e503e56
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ShellCommandsController.java 0ecb77f
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/WanCommandsController.java fa5aa57
geode-core/src/main/java/org/apache/geode/management/internal/web/domain/Link.java cef8cab
geode-core/src/main/java/org/apache/geode/management/internal/web/domain/LinkIndex.java 7d5bb37
geode-core/src/main/java/org/apache/geode/management/internal/web/http/HttpHeader.java ee7e932
geode-core/src/main/java/org/apache/geode/management/internal/web/shell/RestHttpOperationInvoker.java eeedf40
geode-core/src/main/java/org/apache/geode/management/internal/web/util/UriUtils.java b83064e
geode-core/src/test/java/org/apache/geode/cache/client/internal/LocatorTestBase.java c3b349a
geode-core/src/test/java/org/apache/geode/distributed/AbstractLauncherTest.java 62d4bdd
geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherIntegrationTest.java 5b53c6a
geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherTest.java 06d6054
geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherIntegrationTest.java 7bd7462
geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherTest.java f5d6271
geode-core/src/test/java/org/apache/geode/internal/cache/execute/Bug51193DUnitTest.java 0dfbe6c
geode-core/src/test/java/org/apache/geode/internal/lang/ClassUtilsJUnitTest.java 3ec3b06
geode-core/src/test/java/org/apache/geode/internal/lang/StringUtilsJUnitTest.java f3e0abe
geode-core/src/test/java/org/apache/geode/internal/util/CollectionUtilsJUnitTest.java 36be8b8
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AbstractCommandsSupportJUnitTest.java 91a59f8
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListIndexCommandDUnitTest.java 8bf1c43
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java fc920c4
geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommands.java b2be12a
geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommandsJUnitTest.java d4dca4a
geode-pulse/src/test/java/org/apache/geode/tools/pulse/testbed/PropMockDataUpdater.java 6ee5b53
geode-spark-connector/geode-functions/src/main/java/org/apache/geode/spark/connector/internal/geodefunctions/RetrieveRegionFunction.java 096e4d5
geode-web/src/test/java/org/apache/geode/management/internal/web/AbstractWebTestCase.java eac0b8d
geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/ShellCommandsControllerJUnitTest.java 37ec508
geode-web/src/test/java/org/apache/geode/management/internal/web/shell/RestHttpOperationInvokerJUnitTest.java c69013b
Diff: https://reviews.apache.org/r/59323/diff/2/
Changes: https://reviews.apache.org/r/59323/diff/1-2/
Testing
-------
Precheckin running. Tests and LegacyTests already returned green.
Thanks,
Patrick Rhomberg