Re: Support default_middleware configurator method

[Jeremy Evans <code@jeremyevans.net>]

On 09/14 10:56, Eric Wong wrote:
> Jeremy Evans <code@jeremyevans.net> wrote:
> > On 09/13 10:34, Eric Wong wrote:
> > > Worse case is we only support default_middleware as a config
> > > option (but I prefer not to add more options).  Embedded -N support
> > > is an anti-feature which leads to lock-in.
> > 
> > I don't really care about embedded -N support.  I actually didn't even
> > know embedded command file options were possible until I saw the
> > comment in Unicorn.builder.  I just want some way to specify not to
> > load default middleware without requiring a command line option each
> > time.
> 
> Alright, I can accept a patch for the unicorn-specific config
> option only; but it definitely must not be allowed to work in
> .ru files.  Thanks.

OK. To implement that, I modified the bin/unicorn file so -N
is only respected while parsing ARGV, and not while parsing
embedded configuration file options.

Thanks,
Jeremy

From d628cad9e5c4b6e15783f1089b3bd84f42061bfb Mon Sep 17 00:00:00 2001
From: Jeremy Evans <code@jeremyevans.net>
Date: Thu, 13 Sep 2018 10:48:25 -0700
Subject: [PATCH] Support default_middleware configuration option

This allows for the equivalent of the
-N/--no-default_middleware command line option to be
specified in the configuration file so it doesn't
need to be specified on the command line every time
unicorn is executed.

It explicitly excludes the use of -N/--no-default_middleware
as an embedded configuration option in the rackup file, by
ignoring the options after ARGV is parsed.

In order to allow the configuration method to work, have
the lambda that Unicorn.builder returns accept two arguments.
Technically, only one argument is needed for the HttpServer
instance, but I'm guessing if the lambda accepts a single
argument, we expect that to be a rack application instead
of a lambda that returns a rack application.

The command line option option to disable default middleware
will take precedence over the unicorn configuration file option
if both are present.

For backwards compatibility, if the lambda passed to
HttpServer accepts 0 arguments, then call it without
arguments.
---
 bin/unicorn                 |  4 +++-
 lib/unicorn.rb              |  8 ++------
 lib/unicorn/configurator.rb | 11 +++++++++++
 lib/unicorn/http_server.rb  |  8 +++++---
 4 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/bin/unicorn b/bin/unicorn
index 3c5e5cb..00c8464 100755
--- a/bin/unicorn
+++ b/bin/unicorn
@@ -6,6 +6,7 @@
 ENV["RACK_ENV"] ||= "development"
 rackup_opts = Unicorn::Configurator::RACKUP
 options = rackup_opts[:options]
+set_no_default_middleware = true
 
 op = OptionParser.new("", 24, '  ') do |opts|
   cmd = File.basename($0)
@@ -60,7 +61,7 @@
 
   opts.on("-N", "--no-default-middleware",
           "do not load middleware implied by RACK_ENV") do |e|
-    rackup_opts[:no_default_middleware] = true
+    rackup_opts[:no_default_middleware] = true if set_no_default_middleware
   end
 
   opts.on("-D", "--daemonize", "run daemonized in the background") do |d|
@@ -110,6 +111,7 @@
   opts.parse! ARGV
 end
 
+set_no_default_middleware = false
 app = Unicorn.builder(ARGV[0] || 'config.ru', op)
 op = nil
 
diff --git a/lib/unicorn.rb b/lib/unicorn.rb
index b6dae36..5f2134d 100644
--- a/lib/unicorn.rb
+++ b/lib/unicorn.rb
@@ -45,12 +45,8 @@ def self.builder(ru, op)
       abort "rack and Rack::Builder must be available for processing #{ru}"
     end
 
-    # Op is going to get cleared before the returned lambda is called, so
-    # save this value so that it's still there when we need it:
-    no_default_middleware = op[:no_default_middleware]
-
     # always called after config file parsing, may be called after forking
-    lambda do ||
+    lambda do |_, server|
       inner_app = case ru
       when /\.ru$/
         raw = File.read(ru)
@@ -66,7 +62,7 @@ def self.builder(ru, op)
         pp({ :inner_app => inner_app })
       end
 
-      return inner_app if no_default_middleware
+      return inner_app unless server.default_middleware
 
       middleware = { # order matters
         ContentLength: nil,
diff --git a/lib/unicorn/configurator.rb b/lib/unicorn/configurator.rb
index f34d38b..9c36dfe 100644
--- a/lib/unicorn/configurator.rb
+++ b/lib/unicorn/configurator.rb
@@ -265,6 +265,14 @@ def worker_processes(nr)
     set_int(:worker_processes, nr, 1)
   end
 
+  # sets whether to add default middleware in the development and
+  # deployment RACK_ENVs.
+  #
+  # default_middleware is only available in unicorn 5.5.0+
+  def default_middleware(bool)
+    set_bool(:default_middleware, bool)
+  end
+
   # sets listeners to the given +addresses+, replacing or augmenting the
   # current set.  This is for the global listener pool shared by all
   # worker processes.  For per-worker listeners, see the after_fork example
@@ -706,6 +714,9 @@ def parse_rackup_file # :nodoc:
 
     /^#\\(.*)/ =~ File.read(ru) or return
     RACKUP[:optparse].parse!($1.split(/\s+/))
+    if RACKUP[:no_default_middleware]
+      set[:default_middleware] = false
+    end
 
     if RACKUP[:daemonize]
       # unicorn_rails wants a default pid path, (not plain 'unicorn')
diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
index b2bbddb..7531886 100644
--- a/lib/unicorn/http_server.rb
+++ b/lib/unicorn/http_server.rb
@@ -14,7 +14,8 @@ class Unicorn::HttpServer
   attr_accessor :app, :timeout, :worker_processes,
                 :before_fork, :after_fork, :before_exec,
                 :listener_opts, :preload_app,
-                :orig_app, :config, :ready_pipe, :user
+                :orig_app, :config, :ready_pipe, :user,
+                :default_middleware
   attr_writer   :after_worker_exit, :after_worker_ready, :worker_exec
 
   attr_reader :pid, :logger
@@ -70,6 +71,7 @@ def initialize(app, options = {})
     @app = app
     @request = Unicorn::HttpRequest.new
     @reexec_pid = 0
+    @default_middleware = true
     options = options.dup
     @ready_pipe = options.delete(:ready_pipe)
     @init_listeners = options[:listeners] ? options[:listeners].dup : []
@@ -784,12 +786,12 @@ def listener_names(listeners = LISTENERS)
   end
 
   def build_app!
-    if app.respond_to?(:arity) && app.arity == 0
+    if app.respond_to?(:arity) && app.arity == 0 || app.arity == 2
       if defined?(Gem) && Gem.respond_to?(:refresh)
         logger.info "Refreshing Gem list"
         Gem.refresh
       end
-      self.app = app.call
+      self.app = app.arity == 0 ? app.call : app.call(nil, self)
     end
   end
 
-- 
2.17.1