-
Notifications
You must be signed in to change notification settings - Fork 41.9k
Use LinkedHashMap and LinkedHashSet in configuration properties #50367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
2d16316
b880f12
2e3b145
23ed792
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| /* | ||
| * Copyright 2012-present the original author or authors. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * https://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.springframework.boot.build.architecture.configurationproperties.hashmap; | ||
|
|
||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
|
|
||
| import org.springframework.boot.build.architecture.annotations.TestConfigurationProperties; | ||
|
|
||
| /** | ||
| * Test {@link TestConfigurationProperties} using {@link HashMap}. | ||
| * | ||
| * @author Venkata Naga Sai Srikanth Gollapudi | ||
| */ | ||
| @TestConfigurationProperties("testing") | ||
| public class ConfigurationPropertiesWithHashMap { | ||
|
|
||
| private Map<String, String> properties = new HashMap<>(); | ||
|
|
||
| public Map<String, String> getProperties() { | ||
| return this.properties; | ||
| } | ||
|
|
||
| public void setProperties(Map<String, String> properties) { | ||
| this.properties = properties; | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| /* | ||
| * Copyright 2012-present the original author or authors. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * https://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.springframework.boot.build.architecture.configurationproperties.hashset; | ||
|
|
||
| import java.util.HashSet; | ||
| import java.util.Set; | ||
|
|
||
| import org.springframework.boot.build.architecture.annotations.TestConfigurationProperties; | ||
|
|
||
| /** | ||
| * Test {@link TestConfigurationProperties} using {@link HashSet}. | ||
| * | ||
| * @author Venkata Naga Sai Srikanth Gollapudi | ||
| */ | ||
| @TestConfigurationProperties("testing") | ||
| public class ConfigurationPropertiesWithHashSet { | ||
|
|
||
| private Set<String> items = new HashSet<>(); | ||
|
|
||
| public Set<String> getItems() { | ||
| return this.items; | ||
| } | ||
|
|
||
| public void setItems(Set<String> items) { | ||
| this.items = items; | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| /* | ||
| * Copyright 2012-present the original author or authors. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * https://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.springframework.boot.build.architecture.configurationproperties.linkedhashmap; | ||
|
|
||
| import java.util.LinkedHashMap; | ||
| import java.util.Map; | ||
|
|
||
| import org.springframework.boot.build.architecture.annotations.TestConfigurationProperties; | ||
|
|
||
| /** | ||
| * Test {@link TestConfigurationProperties} using {@link LinkedHashMap}. | ||
| * | ||
| * @author Venkata Naga Sai Srikanth Gollapudi | ||
| */ | ||
| @TestConfigurationProperties("testing") | ||
| public class ConfigurationPropertiesWithLinkedHashMap { | ||
|
|
||
| private Map<String, String> properties = new LinkedHashMap<>(); | ||
|
|
||
| public Map<String, String> getProperties() { | ||
| return this.properties; | ||
| } | ||
|
|
||
| public void setProperties(Map<String, String> properties) { | ||
| this.properties = properties; | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| /* | ||
| * Copyright 2012-present the original author or authors. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * https://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.springframework.boot.build.architecture.configurationproperties.linkedhashset; | ||
|
|
||
| import java.util.LinkedHashSet; | ||
| import java.util.Set; | ||
|
|
||
| import org.springframework.boot.build.architecture.annotations.TestConfigurationProperties; | ||
|
|
||
| /** | ||
| * Test {@link TestConfigurationProperties} using {@link LinkedHashSet}. | ||
| * | ||
| * @author Venkata Naga Sai Srikanth Gollapudi | ||
| */ | ||
| @TestConfigurationProperties("testing") | ||
| public class ConfigurationPropertiesWithLinkedHashSet { | ||
|
|
||
| private Set<String> items = new LinkedHashSet<>(); | ||
|
|
||
| public Set<String> getItems() { | ||
| return this.items; | ||
| } | ||
|
|
||
| public void setItems(Set<String> items) { | ||
| this.items = items; | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,7 @@ | |
|
|
||
| package org.springframework.boot.actuate.autoconfigure.context.properties; | ||
|
|
||
| import java.util.HashSet; | ||
| import java.util.LinkedHashSet; | ||
| import java.util.Set; | ||
|
|
||
| import org.springframework.boot.actuate.context.properties.ConfigurationPropertiesReportEndpoint; | ||
|
|
@@ -28,6 +28,7 @@ | |
| * | ||
| * @author Stephane Nicoll | ||
| * @author Madhura Bhave | ||
| * @author Venkata Naga Sai Srikanth Gollapudi | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please revert the author tag for every configurationproperties you've touched that only changes the type. This isn't considered as a significant change.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @snicoll , I'll revert the author tag changes and keep the authorship limited to the new rule implementation and any substantial additions. |
||
| * @since 2.0.0 | ||
| */ | ||
| @ConfigurationProperties("management.endpoint.configprops") | ||
|
|
@@ -42,7 +43,7 @@ public class ConfigurationPropertiesReportEndpointProperties { | |
| * Roles used to determine whether a user is authorized to be shown unsanitized | ||
| * values. When empty, all authenticated users are authorized. | ||
| */ | ||
| private final Set<String> roles = new HashSet<>(); | ||
| private final Set<String> roles = new LinkedHashSet<>(); | ||
|
|
||
| public Show getShowValues() { | ||
| return this.showValues; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This explicitly check for the presence of those two and would miss all other cases. It'd be better if the rule checks that field of type
MaporSet, if initialized, usesLinkedHashMaporLinkedHashSetrespectively. I also don't think we need to link those two together. We could have a "is property of type X initialized, make sure it is Y. With that we can have three rules for List, Set and Map that call the same logic with different arguments.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback.
That makes sense. I'll rework the rule to validate the expected implementation based on the field type rather than checking specifically for HashMap and HashSet. I'll also look at extracting the common logic so that List, Set and Map can share the same validation mechanism with the expected implementation passed as an argument.