Merge pull request #505 from ekohl/coding-style

Clean up coding styles

Author: bastelfreak

lib/puppet/provider/mongodb.rb

@@ -18,12 +18,11 @@ def mongorc_file
   end
 
   def self.mongod_conf_file
-    file = if File.exist? '/etc/mongod.conf'
-             '/etc/mongod.conf'
-           else
-             '/etc/mongodb.conf'
-           end
-    file
+    if File.exist? '/etc/mongod.conf'
+      '/etc/mongod.conf'
+    else
+      '/etc/mongodb.conf'
+    end
   end
 
   def self.mongo_conf
@@ -50,7 +49,7 @@ def self.ipv6_is_enabled(config = nil)
   def self.ssl_is_enabled(config = nil)
     config ||= mongo_conf
     ssl_mode = config.fetch('ssl')
-    ssl_mode.nil? ? false : ssl_mode != 'disabled'
+    !ssl_mode.nil? && ssl_mode != 'disabled'
   end
 
   def self.ssl_invalid_hostnames(config = nil)
@@ -113,7 +112,7 @@ def self.db_ismaster
     cmd_ismaster = mongorc_file + cmd_ismaster if mongorc_file
     db = 'admin'
     res = mongo_cmd(db, conn_string, cmd_ismaster).to_s.chomp
-    res.eql?('true') ? true : false
+    res.eql?('true')
   end
 
   def db_ismaster
@@ -132,19 +131,19 @@ def self.mongo_eval(cmd, db = 'admin', retries = 10, host = nil)
     cmd = mongorc_file + cmd if mongorc_file
 
     out = nil
-    retry_count.times do |n|
-      begin
-        out = if host
-                mongo_cmd(db, host, cmd)
-              else
-                mongo_cmd(db, conn_string, cmd)
-              end
-      rescue => e
+    begin
+      out = if host
+              mongo_cmd(db, host, cmd)
+            else
+              mongo_cmd(db, conn_string, cmd)
+            end
+    rescue => e
+      retry_count -= 1
+      if retry_count > 0
         Puppet.debug "Request failed: '#{e.message}' Retry: '#{n}'"
         sleep retry_sleep
-        next
+        retry
       end
-      break
     end
 
     unless out

lib/puppet/provider/mongodb_user/mongodb.rb

@@ -29,7 +29,7 @@ def self.instances
   def self.prefetch(resources)
     users = instances
     resources.each do |name, resource|
-      provider = users.find { |user| user.username == (resource[:username]) && user.database == (resource[:database]) }
+      provider = users.find { |user| user.username == resource[:username] && user.database == resource[:database] }
       resources[name].provider = provider if provider
     end
   end
@@ -39,22 +39,21 @@ def self.prefetch(resources)
   def create
     if db_ismaster
       password_hash = @resource[:password_hash]
-
-      if password_hash
-      elsif @resource[:password]
+      if !password_hash && @resource[:password]
         password_hash = Puppet::Util::MongodbMd5er.md5(@resource[:username], @resource[:password])
       end
-      cmd_json = <<-EOS.gsub(%r{^\s*}, '').gsub(%r{$\n}, '')
-  {
-    "createUser": "#{@resource[:username]}",
-      "pwd": "#{password_hash}",
-    "customData": {"createdBy": "Puppet Mongodb_user['#{@resource[:name]}']"},
-    "roles": #{@resource[:roles].to_json},
-    "digestPassword": false
-  }
-      EOS
-
-      mongo_eval("db.runCommand(#{cmd_json})", @resource[:database])
+
+      command = {
+        createUser: @resource[:username],
+        pwd: password_hash,
+        customData: {
+          createdBy: "Puppet Mongodb_user['#{@resource[:name]}']"
+        },
+        roles: @resource[:roles],
+        digestPassword: false
+      }
+
+      mongo_eval("db.runCommand(#{command.to_json})", @resource[:database])
     else
       Puppet.warning 'User creation is available only from master host'
 
@@ -64,12 +63,12 @@ def create
       @property_hash[:password_hash] = ''
       @property_hash[:roles] = @resource[:roles]
 
-      exists? ? (return true) : (return false)
+      exists?
     end
   end
 
   def destroy
-    mongo_eval("db.dropUser('#{@resource[:username]}')")
+    mongo_eval("db.dropUser(#{@resource[:username].to_json})")
   end
 
   def exists?
@@ -78,45 +77,42 @@ def exists?
 
   def password_hash=(_value)
     if db_ismaster
-      cmd_json = <<-EOS.gsub(%r{^\s*}, '').gsub(%r{$\n}, '')
-      {
-          "updateUser": "#{@resource[:username]}",
-          "pwd": "#{@resource[:password_hash]}",
-          "digestPassword": false
+      command = {
+        updateUser: @resource[:username],
+        pwd: @resource[:password_hash],
+        digestPassword: false
       }
-      EOS
-      mongo_eval("db.runCommand(#{cmd_json})", @resource[:database])
+
+      mongo_eval("db.runCommand(#{command.to_json})", @resource[:database])
     else
       Puppet.warning 'User password operations are available only from master host'
     end
   end
 
   def password=(value)
     if mongo_26?
-      mongo_eval("db.changeUserPassword('#{@resource[:username]}','#{value}')", @resource[:database])
+      mongo_eval("db.changeUserPassword(#{@resource[:username].to_json}, #{value.to_json})", @resource[:database])
     else
-      cmd_json = <<-EOS.gsub(%r{^\s*}, '').gsub(%r{$\n}, '')
-      {
-          "updateUser": "#{@resource[:username]}",
-          "pwd": "#{@resource[:password]}",
-          "digestpassword": true
+      command = {
+        updateUser: @resource[:username],
+        pwd: @resource[:password],
+        digestpassword: true
       }
-      EOS
 
-      mongo_eval("db.runCommand(#{cmd_json})", @resource[:database])
+      mongo_eval("db.runCommand(#{command.to_json})", @resource[:database])
     end
   end
 
   def roles=(roles)
     if db_ismaster
       grant = roles - @property_hash[:roles]
       unless grant.empty?
-        mongo_eval("db.getSiblingDB('#{@resource[:database]}').grantRolesToUser('#{@resource[:username]}', #{grant. to_json})")
+        mongo_eval("db.getSiblingDB(#{@resource[:database].to_json}).grantRolesToUser(#{@resource[:username].to_json}, #{grant.to_json})")
       end
 
       revoke = @property_hash[:roles] - roles
       unless revoke.empty?
-        mongo_eval("db.getSiblingDB('#{@resource[:database]}').revokeRolesFromUser('#{@resource[:username]}', #{revoke.to_json})")
+        mongo_eval("db.getSiblingDB(#{@resource[:database].to_json}).revokeRolesFromUser(#{@resource[:username].to_json}, #{revoke.to_json})")
       end
     else
       Puppet.warning 'User roles operations are available only from master host'

manifests/mongos/service.pp

@@ -17,12 +17,6 @@
     default   => true
   }
 
-  if $port {
-    $port_real = $port
-  } else {
-    $port_real = '27017'
-  }
-
   if $bind_ip == '0.0.0.0' {
     $bind_ip_real = '127.0.0.1'
   } else {
@@ -60,7 +54,7 @@
     if $service_ensure_real {
       mongodb_conn_validator { 'mongos':
         server  => $bind_ip_real,
-        port    => $port_real,
+        port    => pick($port, 27017),
         timeout => '240',
         require => Service['mongos'],
       }

spec/types/mongodb_database_spec.rb

@@ -0,0 +1,15 @@
+require 'spec_helper'
+
+describe 'mongodb_database' do
+  let(:title) { 'test' }
+
+  it do
+    is_expected.to be_valid_type.
+      with_provider(:mongodb).
+      with_properties('ensure').
+      with_parameters(%w[name tries])
+  end
+
+  it { is_expected.to be_valid_type.with_set_attributes(tries: 5) }
+  it { expect { is_expected.to be_valid_type.with_set_attributes(tries: 'a') }.to raise_error(Puppet::Error) }
+end

spec/unit/puppet/provider/mongodb_user/mongodb_spec.rb

@@ -53,11 +53,11 @@
     it 'creates a user' do
       cmd_json = <<-EOS.gsub(%r{^\s*}, '').gsub(%r{$\n}, '')
       {
-        "createUser": "new_user",
-        "pwd": "pass",
-        "customData": {"createdBy": "Puppet Mongodb_user['new_user']"},
-        "roles": ["role1","role2"],
-        "digestPassword": false
+        "createUser":"new_user",
+        "pwd":"pass",
+        "customData":{"createdBy":"Puppet Mongodb_user['new_user']"},
+        "roles":["role1","role2"],
+        "digestPassword":false
       }
       EOS
 
@@ -68,7 +68,7 @@
 
   describe 'destroy' do
     it 'removes a user' do
-      expect(provider).to receive(:mongo_eval).with("db.dropUser('new_user')")
+      expect(provider).to receive(:mongo_eval).with('db.dropUser("new_user")')
       provider.destroy
     end
   end
@@ -89,9 +89,9 @@
     it 'changes a password_hash' do
       cmd_json = <<-EOS.gsub(%r{^\s*}, '').gsub(%r{$\n}, '')
       {
-          "updateUser": "new_user",
-          "pwd": "pass",
-          "digestPassword": false
+          "updateUser":"new_user",
+          "pwd":"pass",
+          "digestPassword":false
       }
       EOS
       expect(provider).to receive(:mongo_eval).
@@ -128,24 +128,24 @@
     it 'grant a role' do
       resource.provider.set(name: 'new_user', ensure: :present, roles: %w[role1 role2])
       expect(provider).to receive(:mongo_eval).
-        with("db.getSiblingDB('new_database').grantRolesToUser('new_user', [\"role3\"])")
+        with('db.getSiblingDB("new_database").grantRolesToUser("new_user", ["role3"])')
       provider.roles = %w[role1 role2 role3]
     end
 
     it 'revokes a role' do
       resource.provider.set(name: 'new_user', ensure: :present, roles: %w[role1 role2])
       expect(provider).to receive(:mongo_eval).
-        with("db.getSiblingDB('new_database').revokeRolesFromUser('new_user', [\"role1\"])")
+        with('db.getSiblingDB("new_database").revokeRolesFromUser("new_user", ["role1"])')
       provider.roles = ['role2']
     end
 
     # rubocop:disable RSpec/MultipleExpectations
     it 'exchanges a role' do
       resource.provider.set(name: 'new_user', ensure: :present, roles: %w[role1 role2])
       expect(provider).to receive(:mongo_eval).
-        with("db.getSiblingDB('new_database').revokeRolesFromUser('new_user', [\"role1\"])")
+        with('db.getSiblingDB("new_database").revokeRolesFromUser("new_user", ["role1"])')
       expect(provider).to receive(:mongo_eval).
-        with("db.getSiblingDB('new_database').grantRolesToUser('new_user', [\"role3\"])")
+        with('db.getSiblingDB("new_database").grantRolesToUser("new_user", ["role3"])')
 
       provider.roles = %w[role2 role3]
     end

spec/unit/puppet/type/mongodb_database_spec.rb

@@ -18,12 +18,8 @@ function rsReconfigSettings(settings){
 
 function authRequired() {
   try {
-    if (db.serverCmdLineOpts().code == 13) {
-      return true;
-    }
-    return false;
-  }
-  catch (err) {
+    return db.serverCmdLineOpts().code == 13;
+  } catch (err) {
     return false;
   }
 }