Merge pull request heroku#126 from heroku/schneems/plugin-warnings

Clean up plugin logic and warning text

CHANGELOG.md

@@ -1,5 +1,9 @@
 ## Master
 
+Features:
+
+* Don't add plugins if already gems
+
 Bugfixes:
 
 * Fix issue #127 Race condition with LPXC

lib/language_pack.rb

@@ -25,6 +25,7 @@ def self.detect(*args)
 
 require 'dotenv'
 require 'language_pack/instrument'
+require "language_pack/helpers/plugin_installer"
 require "language_pack/ruby"
 require "language_pack/rack"
 require "language_pack/rails2"

lib/language_pack/helpers/plugin_installer.rb

@@ -0,0 +1,38 @@
+require "language_pack/shell_helpers"
+
+module LanguagePack
+  module Helpers
+    # Takes an array of plugin names and vendor_url
+    # fetches plugins from url, installs them
+    class PluginsInstaller
+      attr_accessor :plugins, :vendor_url
+      include LanguagePack::ShellHelpers
+
+      def initialize(plugins, vendor_url = LanguagePack::Base::VENDOR_URL)
+        @plugins    = plugins || []
+        @vendor_url = vendor_url
+      end
+
+      # vendors all the plugins into the slug
+      def install
+        return true unless plugins.any?
+        plugins.each { |plugin| vendor(plugin) }
+      end
+
+      def plugin_dir(name = "")
+        Pathname.new("vendor/plugins").join(name)
+      end
+
+      # vendors an individual plugin
+      # @param [String] name of the plugin
+      def vendor(name)
+        directory = plugin_dir(name)
+        return true if directory.exist?
+        directory.mkpath
+        Dir.chdir(directory) do |dir|
+          run("curl #{vendor_url}/#{name}.tgz -s -o - | tar xzf -")
+        end
+      end
+    end
+  end
+end

lib/language_pack/rails2.rb

@@ -4,7 +4,7 @@
 
 # Rails 2 Language Pack. This is for any Rails 2.x apps.
 class LanguagePack::Rails2 < LanguagePack::Ruby
-
+  PLUGINS = ["rails_log_stdout"]
   # detects if this is a valid Rails 2 app
   # @return [Boolean] true if it's a Rails 2 app
   def self.use?
@@ -45,54 +45,24 @@ def default_process_types
 
   def compile
     instrument "rails2.compile" do
-      super
       install_plugins
+      super
     end
   end
 
 private
 
-  # list of plugins to be installed
-  # @return [Array] resulting list in a String Array
   def plugins
-    %w( rails_log_stdout )
+    @plugins ||= PLUGINS.reject { |plugin| gem_is_bundled?(plugin) }
   end
 
-  # the root path of where the plugins are to be installed from
-  # @return [String] the resulting path
-  def plugin_root
-    File.expand_path("../../../vendor/plugins", __FILE__)
-  end
-
-  # vendors all the plugins into the slug
   def install_plugins
     instrument "rails2.install_plugins" do
-      plugins_to_install = plugins.select { |plugin| install_plugin?(plugin) }
-      if plugins_to_install.any?
-        topic "Rails plugin injection"
-        plugins_to_install.each { |plugin| install_plugin(plugin) }
-      end
+      topic "Rails plugin injection"
+      LanguagePack::Helpers::PluginsInstaller.new(plugins).install
     end
   end
 
-  # vendors an individual plugin
-  # @param [String] name of the plugin
-  def install_plugin(name)
-    puts "Injecting #{name}; add #{name} to your Gemfile to avoid plugin injection"
-    plugin_dir = "vendor/plugins/#{name}"
-    FileUtils.mkdir_p plugin_dir
-    Dir.chdir(plugin_dir) do |dir|
-      run("curl #{VENDOR_URL}/#{name}.tgz -s -o - | tar xzf -")
-    end
-  end
-
-  # determines if a particular plugin should be installed
-  # @param [String] name of the plugin
-  def install_plugin?(name)
-    plugin_dir = "vendor/plugins/#{name}"
-    !File.exist?(plugin_dir) && !gem_is_bundled?(name)
-  end
-
   # most rails apps need a database
   # @return [Array] shared database addon
   def add_dev_database_addon

lib/language_pack/rails3.rb

@@ -3,6 +3,7 @@
 
 # Rails 3 Language Pack. This is for all Rails 3.x apps.
 class LanguagePack::Rails3 < LanguagePack::Rails2
+  PLUGINS = ["rails_log_stdout", "rails3_serve_static_assets"]
   # detects if this is a Rails 3.x app
   # @return [Boolean] true if it's a Rails 3.x app
   def self.use?
@@ -40,8 +41,12 @@ def compile
 
 private
 
-  def plugins
-    super.concat(%w( rails3_serve_static_assets )).uniq
+  def install_plugins
+    return false unless plugins.any?
+    plugins.each do |name|
+      warn "Injecting plugin '#{name}', to skip add 'rails_12factor' gem to your Gemfile"
+    end
+    super
   end
 
   # runs the tasks for the Rails 3.1 asset pipeline

lib/language_pack/rails4.rb

@@ -30,7 +30,6 @@ def default_process_types
   def build_bundler
     instrument "rails4.build_bundler" do
       super
-      check_for_rails_gems
     end
   end
 
@@ -41,23 +40,14 @@ def compile
   end
 
   private
-  def rails_gems
-    %w(rails_stdout_logging rails_serve_static_assets)
-  end
 
-  def check_for_rails_gems
-    instrument "rails4.check_for_rails_gems" do
-      if rails_gems.any? {|gem| !gem_is_bundled?(gem) }
-        warn(<<WARNING)
-Include "rails_12factor" gem to enable all platform features
+  def install_plugins
+    return false unless plugins.any?
+    warn <<-WARNING
+Include 'rails_12factor' gem to enable all platform features
 See https://devcenter.heroku.com/articles/rails-integration-gems for more information.
 WARNING
-      end
-    end
-  end
-
-  def plugins
-    []
+    # do not install plugins, do not call super
   end
 
   def public_assets_folder

spec/rails3_spec.rb

@@ -5,6 +5,9 @@
     Hatchet::Runner.new("rails3_mri_193").deploy do |app, heroku|
       expect(app.output).to include("Asset precompilation completed")
       add_database(app, heroku)
+
+      expect(app.output).to match("WARNINGS")
+      expect(app.output).to match("Injecting plugin 'rails_log_stdout', to skip add 'rails_12factor' gem to your Gemfile")
       expect(successful_body(app)).to eq("hello")
     end
   end