alex-rufous commented on a change in pull request #103:
URL: https://github.com/apache/qpid-broker-j/pull/103#discussion_r683282742
##########
File path:
broker-core/src/main/java/org/apache/qpid/server/store/JsonFileConfigStore.java
##########
@@ -156,49 +156,52 @@ protected boolean load(final ConfiguredObjectRecord...
initialRecords)
boolean updated = false;
Collection<ConfiguredObjectRecord> records =
Collections.emptyList();
- ConfiguredObjectRecordConverter configuredObjectRecordConverter =
+ final ConfiguredObjectRecordConverter
configuredObjectRecordConverter =
new ConfiguredObjectRecordConverter(_parent.getModel());
- records = configuredObjectRecordConverter.readFromJson(_rootClass,
_parent, new FileReader(configFile));
-
- if(_rootClass == null)
+ try (FileReader configFileReader = new FileReader(configFile))
Review comment:
This change is unnecessary. The `configFileReader` is closed in
`ObjectMapper`. If you really need to make sure that `configFileReader` is
closed in the code where it is created, you can simply add `try-with-resource`
block around the code reading from json
```
try (FileReader configFileReader = new FileReader(configFile))
{
records = configuredObjectRecordConverter.readFromJson(_rootClass,
_parent, configFileReader);
}
```
##########
File path:
broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java
##########
@@ -437,7 +437,10 @@ public String getRequestURL()
{
if ("data".equals(part.getName()) &&
"application/json".equals(part.getContentType()))
{
- items = mapper.readValue(part.getInputStream(),
LinkedHashMap.class);
+ try (InputStream inputStream = part.getInputStream())
Review comment:
The change is unnecessary as methods `ObjectMapper#readValue ` close the
input stream/reader..
##########
File path:
perftests/src/main/java/org/apache/qpid/disttest/controller/config/ConfigReader.java
##########
@@ -37,10 +37,10 @@
public Config getConfigFromFile(String fileName) throws IOException
{
- Reader reader = getConfigReader(fileName);
-
- Config config = readConfig(reader);
- return config;
+ try (Reader reader = getConfigReader(fileName))
Review comment:
The reader is closed in `ObjectMapper#read...`. Thus, the change is
unnecessary...
##########
File path: broker-core/src/main/java/org/apache/qpid/server/util/FileUtils.java
##########
@@ -205,8 +205,10 @@ public static void copy(File src, File dst)
*/
public static void copyCheckedEx(File src, File dst) throws IOException
{
- InputStream in = new FileInputStream(src);
- copy(in, dst);
+ try (InputStream in = new FileInputStream(src))
Review comment:
The `InputStream` is closed in `copy`. Arguably the method `copy` should
indicate that it closes the input stream. It make more sense to rename it into
`copyInputStreamIntoFileAndClose`. Though, I suppose for a better code
relaibility the 'try-with-resource` would be good here.
##########
File path:
broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractVirtualHostNode.java
##########
@@ -430,40 +430,42 @@ public PreferenceStore createPreferenceStore()
protected final ConfiguredObjectRecord[] getInitialRecords() throws
IOException
{
- ConfiguredObjectRecordConverter converter = new
ConfiguredObjectRecordConverter(getModel());
+ final ConfiguredObjectRecordConverter converter = new
ConfiguredObjectRecordConverter(getModel());
- Collection<ConfiguredObjectRecord> records =
- new
ArrayList<>(converter.readFromJson(VirtualHost.class,this,getInitialConfigReader()));
-
- if(!records.isEmpty())
+ try (final Reader initialConfigReader = getInitialConfigReader())
Review comment:
Try-with-resource block is unnecessary here as reading code is closing
the reader. If you really have to change to `try-with-resource`, it would be
preferable to keep the `try` block small and only use it for reading the
records. IMHO, that makes code more readable
```
final Collection<ConfiguredObjectRecord> records;
try (final Reader initialConfigReader = getInitialConfigReader())
{
records = new ArrayList<>(converter.readFromJson(VirtualHost.class,
this, initialConfigReader));
}
```
--
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]
