paul-rogers commented on code in PR #13045:
URL: https://github.com/apache/druid/pull/13045#discussion_r965319656
##########
server/src/main/java/org/apache/druid/server/StatusResource.java:
##########
@@ -92,15 +96,15 @@ private Map<String, String> filterHiddenProperties(
Map<String, String> allProperties
)
{
- return Maps.filterEntries(
- allProperties,
- (entry) -> hiddenProperties
- .stream()
- .anyMatch(
- hiddenPropertyElement ->
-
!StringUtils.toLowerCase(entry.getKey()).contains(StringUtils.toLowerCase(hiddenPropertyElement))
- )
+ Map<String, String> propertyCopy = new HashMap<>(allProperties);
Review Comment:
When this code is nested inside a resource, it is hard to test. Should we
pull this out as a static method so it can be tested easily with various sets
of properties and hidden property lists?
##########
server/src/main/java/org/apache/druid/server/StatusResource.java:
##########
@@ -92,15 +96,15 @@ private Map<String, String> filterHiddenProperties(
Map<String, String> allProperties
)
{
- return Maps.filterEntries(
- allProperties,
- (entry) -> hiddenProperties
- .stream()
- .anyMatch(
- hiddenPropertyElement ->
-
!StringUtils.toLowerCase(entry.getKey()).contains(StringUtils.toLowerCase(hiddenPropertyElement))
- )
+ Map<String, String> propertyCopy = new HashMap<>(allProperties);
+ allProperties.keySet().forEach(
Review Comment:
The set of hidden properties is likely to be much smaller than the set of
all properties. Should we iterate over the hidden properties so we have a much
lower total cost? One could argue that this operation is used infrequently and
efficiency doesn't matter. But, since we're changing the code anyway, we might
as well make it simple.
##########
server/src/main/java/org/apache/druid/server/StatusResource.java:
##########
@@ -92,15 +96,15 @@ private Map<String, String> filterHiddenProperties(
Map<String, String> allProperties
)
{
- return Maps.filterEntries(
- allProperties,
- (entry) -> hiddenProperties
- .stream()
- .anyMatch(
- hiddenPropertyElement ->
-
!StringUtils.toLowerCase(entry.getKey()).contains(StringUtils.toLowerCase(hiddenPropertyElement))
- )
+ Map<String, String> propertyCopy = new HashMap<>(allProperties);
+ allProperties.keySet().forEach(
+ (key) -> {
+ if (hiddenProperties.stream().anyMatch((hiddenProperty) ->
StringUtils.toLowerCase(key).contains(StringUtils.toLowerCase(hiddenProperty))))
{
+ propertyCopy.remove(key);
Review Comment:
Should we convert the value to, say, "*****" so that at least the user knows
that the property was set to something?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
