Polishing

This change polishes up the contribution to meet our coding standards.

[resolves #436]

Author: nebhale

docs/framework-java_memory_assistant.md

@@ -15,7 +15,7 @@ Tags are printed to standard output by the buildpack detect script.
 ## Configuration
 For general information on configuring the buildpack, including how to specify configuration values through environment variables, refer to [Configuration and Extension][].
 
-The framework can be configured by modifying the [``config/java_memory_assistant.yml``][] file in the buildpack fork.
+The framework can be configured by modifying the [`config/java_memory_assistant.yml`][] file in the buildpack fork.
 
 | Name | Description
 | ---- | -----------
@@ -47,7 +47,7 @@ The timestamp pattern `%ts:yyyyMMdd'T'mmssSSSZ%` is equivalent to the `%FT%T%z`
 | Survivor               | `survivor`         |
 | Old Generation         | `old_gen`          |
 
-The default values can be found in the [``config/java_memory_assistant.yml``][] file.
+The default values can be found in the [`config/java_memory_assistant.yml`][] file.
 
 ### Examples
 
@@ -112,4 +112,6 @@ To prevent heap dumps from "going down" with the container, you should consider
 
 #### Container-mounted volumes
 
-If you are using a filesystem service that mounts persistent volumes to the container, it is enough to name one of the volume services `heap-dump` or tag one volume with `heap-dump`, and the path specified as the `heap_dump_folder` configuration will be resolved against `<mount-point>/<space_name>-<space_id[0,8]>/<application_name>-<application_id[0-8]>`.
+If you are using a filesystem service that mounts persistent volumes to the container, it is enough to name one of the volume services `heap-dump` or tag one volume with `heap-dump`, and the path specified as the `heap_dump_folder` configuration will be resolved against `<mount-point>/<space_name>-<space_id[0,8]>/<application_name>-<application_id[0-8]>`.
+
+[`config/java_memory_assistant.yml`]: ../config/java_memory_assistant.yml

lib/java_buildpack/framework/java_memory_assistant.rb

@@ -28,9 +28,7 @@ class JavaMemoryAssistant < JavaBuildpack::Component::ModularComponent
       protected
 
       # (see JavaBuildpack::Component::ModularComponent#command)
-      def command
-        # Nothing to do here, the agent is initialized via java opts
-      end
+      def command; end
 
       # (see JavaBuildpack::Component::ModularComponent#sub_components)
       def sub_components(context)

lib/java_buildpack/framework/java_memory_assistant/agent.rb

@@ -34,67 +34,64 @@ def compile
 
       # (see JavaBuildpack::Component::BaseComponent#release)
       def release
-        @droplet.java_opts.add_javaagent @droplet.sandbox + jar_name
-
-        @droplet.java_opts.add_system_property 'jma.enabled', 'true'
-        @droplet.java_opts.add_system_property 'jma.heap_dump_name', %("#{name_pattern}")
+        @droplet.java_opts
+                .add_javaagent(@droplet.sandbox + jar_name)
+                .add_system_property('jma.enabled', 'true')
+                .add_system_property('jma.heap_dump_name', %("#{name_pattern}"))
+                .add_system_property 'jma.log_level', normalized_log_level
 
         add_system_prop_if_config_present 'check_interval', 'jma.check_interval'
         add_system_prop_if_config_present 'max_frequency', 'jma.max_frequency'
 
-        @droplet.java_opts.add_system_property 'jma.log_level', log_level
+        return unless @configuration.key?('thresholds')
 
-        (@configuration['thresholds'] || {}).each do |key, value|
+        @configuration['thresholds'].each do |key, value|
           @droplet.java_opts.add_system_property "jma.thresholds.#{key}", value.to_s
         end
       end
 
       protected
 
-      def supports?
-        true
-      end
-
       # (see JavaBuildpack::Component::VersionedDependencyComponent#jar_name)
       def jar_name
         "java-memory-assistant-#{@version}.jar"
       end
 
-      private
-
-      def name_pattern
-        # Double escaping quotes of doom. Nothing less would work.
-        %q(%env:CF_INSTANCE_INDEX%-%ts:yyyy-MM-dd'"'"'T'"'"'mm'"'"':'"'"'ss'"'"':'"'"'SSSZ%-) \
-          '%env:CF_INSTANCE_GUID[,8]%.hprof'
+      def supports?
+        true
       end
 
-      def add_system_prop_if_config_present(config_entry, system_property_name, quote_value = false)
-        return unless @configuration[config_entry]
+      private
+
+      LOG_LEVEL_MAPPING = {
+        'DEBUG' => 'DEBUG',
+        'WARN' => 'WARNING',
+        'INFO' => 'INFO',
+        'ERROR' => 'ERROR',
+        'FATAL' => 'ERROR'
+      }.freeze
 
-        config_value = @configuration[config_entry]
-        config_value = '"' + config_value + '"' if quote_value
+      private_constant :LOG_LEVEL_MAPPING
 
-        @droplet.java_opts.add_system_property(system_property_name, config_value)
+      def add_system_prop_if_config_present(config_entry, system_property_name)
+        return unless @configuration.key?(config_entry)
+        @droplet.java_opts.add_system_property(system_property_name, @configuration[config_entry])
       end
 
       def log_level
-        actual_log_level = @configuration['log_level'] || ENV['JBP_LOG_LEVEL'] || 'ERROR'
-
-        mapped_log_level = log_level_mapping[actual_log_level.upcase]
-
-        raise "Invalid value of the 'log_level' property: '#{actual_log_level}'" unless mapped_log_level
+        @configuration['log_level'] || ENV['JBP_LOG_LEVEL'] || 'ERROR'
+      end
 
-        mapped_log_level
+      def normalized_log_level
+        normalized_log_level = LOG_LEVEL_MAPPING[log_level.upcase]
+        raise "Invalid value of the 'log_level' property: '#{log_level}'" unless normalized_log_level
+        normalized_log_level
       end
 
-      def log_level_mapping
-        {
-          'DEBUG' => 'DEBUG',
-          'WARN' => 'WARNING',
-          'INFO' => 'INFO',
-          'ERROR' => 'ERROR',
-          'FATAL' => 'ERROR'
-        }
+      def name_pattern
+        # Double escaping quotes of doom. Nothing less would work.
+        %q(%env:CF_INSTANCE_INDEX%-%ts:yyyy-MM-dd'"'"'T'"'"'mm'"'"':'"'"'ss'"'"':'"'"'SSSZ%-) \
+          '%env:CF_INSTANCE_GUID[,8]%.hprof'
       end
 
     end

lib/java_buildpack/framework/java_memory_assistant/clean_up.rb

@@ -28,19 +28,18 @@ def compile
         return unless supports?
 
         download_zip false
-
-        @droplet.copy_resources
       end
 
       # (see JavaBuildpack::Component::BaseComponent#release)
       def release
         return unless supports?
 
-        environment_variables = @droplet.environment_variables
-        environment_variables.add_environment_variable 'JMA_MAX_DUMP_COUNT', @configuration['max_dump_count'].to_s
+        @droplet.environment_variables
+                .add_environment_variable 'JMA_MAX_DUMP_COUNT', @configuration['max_dump_count'].to_s
 
-        @droplet.java_opts.add_system_property('jma.command.interpreter', '')
-        @droplet.java_opts.add_system_property('jma.execute.before', @droplet.sandbox + 'cleanup')
+        @droplet.java_opts
+                .add_system_property('jma.command.interpreter', '')
+                .add_system_property('jma.execute.before', @droplet.sandbox + 'cleanup')
       end
 
       protected

lib/java_buildpack/framework/java_memory_assistant/heap_dump_folder.rb

@@ -38,9 +38,7 @@ def detect
       end
 
       # (see JavaBuildpack::Component::BaseComponent#compile)
-      def compile
-        # Nothing to do
-      end
+      def compile; end
 
       # (see JavaBuildpack::Component::BaseComponent#release)
       def release

spec/java_buildpack/framework/java_memory_assistant/heap_dump_folder_spec.rb

@@ -82,13 +82,13 @@
 
     before do
       allow(services).to receive(:find_service).with('heap-dump')
-        .and_return('volume_mounts' =>
-          [
-            {
-              'container_dir' => '/my_volume',
-              'mode'          => 'rw'
-            }
-          ])
+                                               .and_return('volume_mounts' =>
+                                                 [
+                                                   {
+                                                     'container_dir' => '/my_volume',
+                                                     'mode'          => 'rw'
+                                                   }
+                                                 ])
     end
 
     it 'adds \'jma.heap_dump_folder\' with volume container_dir as path root', :enable_log_file, log_level: 'INFO' do
@@ -112,13 +112,13 @@
 
     before do
       allow(services).to receive(:find_service).with('heap-dump')
-        .and_return('volume_mounts' =>
-          [
-            {
-              'container_dir' => '/my_volume',
-              'mode'          => 'r'
-            }
-          ])
+                                               .and_return('volume_mounts' =>
+                                                 [
+                                                   {
+                                                     'container_dir' => '/my_volume',
+                                                     'mode'          => 'r'
+                                                   }
+                                                 ])
     end
 
     it 'fails if volume does not have write mode active', :enable_log_file, log_level: 'DEBUG' do