InputStream in ServiceImplementation template should be closed after loading

[fishermans]

Hi Tobias,

From my point of view, the InputStream should be closed once the Properties have been loaded.:

    private static ServiceImplementation getServiceImplementationByProperty(ServiceImplementation previousConfig, ${ simpleName } serviceImpl) {

        // first try to get config from property file
        final String propertyFileName = "/META-INF/spiap/" + ${ simpleName }.class.getCanonicalName() + "/" + serviceImpl.getClass().getCanonicalName() + ".properties";
        InputStream inputStream = ${ simpleName }ServiceLocator.class.getResourceAsStream(propertyFileName);
        if (inputStream != null) {

            try {
                Properties properties = new Properties();
                properties.load(inputStream);

                String id = properties.getProperty("${constants.id}");
                String description = properties.getProperty("${constants.description}");
                Integer priority = null;
                try {
                    String priorityString = properties.getProperty("${constants.priority}");
                    if (priorityString != null) {
                        priority = Integer.valueOf(priorityString);
                    }
                } catch (NumberFormatException e) {
                    // ignore
                }

                Boolean outOfService = null;
                String outOfServiceString = properties.getProperty("${constants.outOfService}");
                if (outOfServiceString != null) {
                    outOfService = Boolean.valueOf(outOfServiceString);
                }

                return new ServiceImplementation(previousConfig, id, description, priority, outOfService);

            } catch (Exception e) {
                // do nothing
            }
        }

        return previousConfig;

    }

[tobiasstamann]

Hi, yes you are right.

I will convert it into an auto close try catch block and also add an error message in case of an error.

[fishermans]

Try-with-resource would limit this annotation processor to be only compatible to JDK 8 and up. That would be OK for me but it could break the compatibility for other people.

[tobiasstamann]

My plan was to drop Java 7 support soon, since extended support ends in july 2022.
And in the end Java 8 makes development a lot easier :)

[fishermans]

Should just be mentioned in the readme.

[tobiasstamann]

I am currently doing the Java 8 migration for APTK and will do it afterwards.
The README will then be extended by a compatibilty matrix.

In the meantime i will add a manual close for that inputstream.
I will do that somewhen later today.

[fishermans]

Thanks, Tobias. The template can then be refactored in some other points, like diamond list implementations like:

final List<${ simpleName }> services = new ArrayList<${ simpleName }>();

or

return new ArrayList(serviceImplementations);

Also, I suggest to implement the Comparable interface on the ServiceImplementation class instead of a static comparator. The null check is obsolete here:

public static class ServiceImplementation implements Comparable<ServiceImplementation> {
...
    @Override
    public int compareTo(final ServiceImplementation o) {

      if (this == o) {
        return 0;
      }
      if (this.getPriority() == o.getPriority()) {
        return 0;
      } else {
        return this.getPriority() < o.getPriority() ? -1 : 1;
      }
    }

Then the comparator can be dropped and the list sort would look like that:

Collections.sort(tmpServiceImplementations);

[fishermans]

Maybe, I find some time to create a PR. Then, we could look on it together.

[tobiasstamann]

#18 was accepted and released which resolves the issue