Coding conventions

Change-Id: Icfaf09cab7b145d1ace1d9f29bea99ee801a86f4

lib/acm/config.rb

@@ -87,7 +87,7 @@ def configure(config)
 
         puts("Configuration complete")
         @logger.info("ACM running #{@revision}")
-        if(!@log_file.nil?)
+        unless @log_file.nil?
           puts("Logs are at #{@log_file}")
         end
       end

lib/acm/models/objects.rb

@@ -63,7 +63,7 @@ def to_json
           subject = access_control_entry.subject
           @logger.debug("Subject #{subject.inspect}")
           subject_immutable_id = nil
-          if(subject.type.to_sym == :user)
+          if subject.type.to_sym == :user
             subject_immutable_id = "u-#{subject.immutable_id}"
           else
             subject_immutable_id = "g-#{subject.immutable_id}"

lib/acm/models/subjects.rb

@@ -34,7 +34,7 @@ def before_create
     end
 
     def set_immutable_id
-      if(self.immutable_id.nil?)
+      if self.immutable_id.nil?
         self.immutable_id = SecureRandom.uuid()
       end
       @logger.debug("Immutable id for subject #{self.type} is #{self.immutable_id}")
@@ -49,14 +49,14 @@ def to_json
         :additional_info => self.additional_info
       }
 
-      if(self.type == :group.to_s)
+      if self.type == :group.to_s
         members = self.members
         output_members = []
         members.each { |member|
           @logger.debug("Member #{member.inspect} user #{member.user.inspect}")
           output_members.insert(0, member.user.immutable_id)
         }
-        if(output_members.size() > 0)
+        if output_members.size() > 0
           output_group[:members] = output_members
         end
       end

lib/acm/routes/access_controller.rb

@@ -20,7 +20,7 @@ class ApiController < Sinatra::Base
       content_type 'application/json', :charset => 'utf-8', :schema => ACM::Config.default_schema_version
 
       object_id = params[:object_id]
-      if(params[:id].nil? || params[:p].nil?)
+      if params[:id].nil? || params[:p].nil?
         @logger.error("check_access empty subject or permissions")
         raise ACM::InvalidRequest.new("Could not find subject or permissions in the request")
       end

lib/acm/routes/group_controller.rb

@@ -41,7 +41,7 @@ class ApiController < Sinatra::Base
       end
       @logger.debug("request is #{request_json.inspect}")
 
-      if(request_json.nil?)
+      if request_json.nil?
         @logger.error("Invalid request")
         raise ACM::InvalidRequest.new("Request is empty")
       end
@@ -58,7 +58,7 @@ class ApiController < Sinatra::Base
       #Set the Location response header
       group = Yajl::Parser.parse(group_json, :symbolize_keys => true)
       request_url = request.url
-      if(request_url.end_with? ["/"])
+      if request_url.end_with? ["/"]
         request_url.chop()
       end
       headers "Location" => "#{request_url}/#{group[:id]}"
@@ -78,7 +78,7 @@ class ApiController < Sinatra::Base
       end
       @logger.debug("request is #{request_json.inspect}")
 
-      if(request_json.nil?)
+      if request_json.nil?
         @logger.error("Invalid request")
         raise ACM::InvalidRequest.new("Request is empty")
       end
@@ -95,7 +95,7 @@ class ApiController < Sinatra::Base
       #Set the Location response header
       group = Yajl::Parser.parse(group_json, :symbolize_keys => true)
       request_url = request.url
-      if(request_url.end_with? ["/"])
+      if request_url.end_with? ["/"]
         request_url.chop()
       end
       headers "Location" => "#{request_url}"

lib/acm/routes/object_controller.rb

@@ -50,7 +50,7 @@ class ApiController < Sinatra::Base
       end
       @logger.debug("request is #{request_json.inspect}")
 
-      if(request_json.nil?)
+      if request_json.nil?
         @logger.error("Invalid request")
         raise ACM::InvalidRequest.new("Request is empty")
       end
@@ -69,7 +69,7 @@ class ApiController < Sinatra::Base
       #Set the Location response header
       object = Yajl::Parser.parse(object_json, :symbolize_keys => true)
       request_url = request.url
-      if(request_url.end_with? ["/"])
+      if request_url.end_with? ["/"]
         request_url.chop()
       end
       headers "Location" => "#{request_url}/#{object[:id]}"
@@ -80,7 +80,7 @@ class ApiController < Sinatra::Base
     put '/objects/:object_id' do
       content_type 'application/json', :charset => 'utf-8', :schema => ACM::Config.default_schema_version
 
-      if(params[:object_id].nil?)
+      if params[:object_id].nil?
         @logger.error("Empty object id")
         raise ACM::InvalidRequest.new("Empty object id")
       end
@@ -90,7 +90,7 @@ class ApiController < Sinatra::Base
       additional_info = nil
       acl = nil
 
-      if(!request.body.nil? && request.body.size > 0)
+      if !request.body.nil? && request.body.size > 0
         request_json = nil
         begin
           request_json = Yajl::Parser.new.parse(request.body)
@@ -116,7 +116,7 @@ class ApiController < Sinatra::Base
       #Set the Location response header
       object = Yajl::Parser.parse(object_json, :symbolize_keys => true)
       request_url = request.url
-      if(request_url.end_with? ["/"])
+      if request_url.end_with? ["/"]
         request_url.chop()
       end
       headers "Location" => "#{request_url}"
@@ -153,7 +153,7 @@ class ApiController < Sinatra::Base
       @logger.debug("Permissions requested to be removed are #{permissions_request.inspect}")
 
       subject = params[:id]
-      if(subject.start_with?("u-") || subject.start_with?("g-"))
+      if subject.start_with?("u-") || subject.start_with?("g-")
         subject = subject[2..subject.length]
         @logger.debug("Stripping subject of prefix #{subject}")
       end

lib/acm/routes/permission_set_controller.rb

@@ -35,7 +35,7 @@ class ApiController < Sinatra::Base
       end
       @logger.debug("request is #{request_json.inspect}")
 
-      if(request_json.nil?)
+      if request_json.nil?
         @logger.error("Invalid request")
         raise ACM::InvalidRequest.new("Request is empty")
       end
@@ -45,7 +45,7 @@ class ApiController < Sinatra::Base
       permissions = request_json[:permissions.to_s]
       additional_info = request_json[:additional_info.to_s]
 
-      if(!permissions.nil? && !permissions.kind_of?(Array))
+      if !permissions.nil? && !permissions.kind_of?(Array)
         @logger.error("Invalid request. Permissions must be an arrary")
         raise ACM::InvalidRequest.new("Permissions in the input must be an array")
       end
@@ -57,7 +57,7 @@ class ApiController < Sinatra::Base
       #Set the Location response header
       ps = Yajl::Parser.parse(ps_json, :symbolize_keys => true)
       request_url = request.url
-      if(request_url.end_with? ["/"])
+      if request_url.end_with? ["/"]
         request_url.chop()
       end
       headers "Location" => "#{request_url}/#{ps[:name]}"

lib/acm/services/access_control_service.rb

@@ -22,16 +22,16 @@ class AccessControlService < ACMService
     def check_access(object_id, subject_id, permissions)
       @logger.debug("Request to check_access #{object_id} #{subject_id} #{permissions}")
 
-      if(subject_id.nil?)
+      if subject_id.nil?
         @logger.debug("Subject is nil")
         raise ACM::ObjectNotFound.new("")
       end
 
-      if(permissions.nil? || permissions.size() == 0)
+      if permissions.nil? || permissions.size() == 0
         @logger.debug("No permissions")
         raise ACM::ObjectNotFound.new("")
       else
-        if(!permissions.kind_of?(Array)) #The code below only works with arrays
+        unless permissions.kind_of?(Array) #The code below only works with arrays
           permissions = [permissions]
         end
       end
@@ -40,15 +40,15 @@ def check_access(object_id, subject_id, permissions)
       object = ACM::Models::Objects.filter(:immutable_id => object_id).first()
       @logger.debug("Object #{object.inspect}")
 
-      if(object.nil?)
+      if object.nil?
         @logger.debug("Could not find the object #{object_id}")
         raise ACM::ObjectNotFound.new("")
       end
 
       #Find the permission entities that need to be checked
       permission_ids = ACM::Models::Permissions.filter(:name => permissions).all().map{|p| p.id}
       #All the permissions must exist. That's why the size of the input must match the size of the permissions query
-      if(permission_ids.nil? || permission_ids.size() == 0 || permission_ids.size() < permissions.size())
+      if permission_ids.nil? || permission_ids.size() == 0 || permission_ids.size() < permissions.size()
         @logger.debug("Permissions did not match #{permission_ids.inspect} #{permissions.inspect} #{permission_ids.size()} #{permissions.size()}")
         raise ACM::ObjectNotFound.new("")
       end
@@ -58,7 +58,7 @@ def check_access(object_id, subject_id, permissions)
       permission_ids.each { |permission_id|
         acl = ACM::Models::AccessControlEntries.filter(:object_id => object.id,
                                                        :permission_id => permission_id).all()
-        if(acl.nil? || acl.size() == 0)
+        if acl.nil? || acl.size() == 0
           @logger.debug("ACL did not match")
           raise ACM::ObjectNotFound.new("")
         end
@@ -70,7 +70,7 @@ def check_access(object_id, subject_id, permissions)
 
           subject = ace.subject #Search the subject for each ace
           @logger.debug("Subject being checked #{subject.inspect}")
-          if(subject.type == :user.to_s)
+          if subject.type == :user.to_s
             #If the subject has not already been found and we have a match, return a true
             found = (found == false && subject.immutable_id.eql?(subject_id.to_s)) ? true : false
             @logger.debug("Subject #{subject.inspect} found #{found}")
@@ -80,23 +80,23 @@ def check_access(object_id, subject_id, permissions)
             @logger.debug("Subject #{subject.inspect} found #{found}")
 
             #If the group was not the subject being searched for, search the members
-            if(!found)
+            unless found
               ACM::Models::Members.filter(:group_id => subject.id).all().each { |member|
                 found = (found == false && member.user.immutable_id.eql?(subject_id.to_s)) ? true : false
                 @logger.debug("Subject #{member.user.inspect} found #{found}")
-                if(found)
+                if found
                   break
                 end
               }
             end
           end
 
           #Don't go any further in this search if the subject has already been found
-          if(found)
+          if found
             break
           end
         }
-        if(!found)  #If the acl does not contain the subject, the operation fails
+        unless found  #If the acl does not contain the subject, the operation fails
           @logger.debug("No matching subjects for acl #{acl.inspect}")
           raise ACM::ObjectNotFound.new("")
         end

lib/acm/services/group_service.rb

@@ -40,21 +40,21 @@ def create_group(opts = {})
       ACM::Config.db.transaction do
         begin
           existing_group = ACM::Models::Subjects.filter(:immutable_id => group.immutable_id).first()
-          if(existing_group.nil?)
+          if existing_group.nil?
             group.save
           else
             @logger.error("Group id #{existing_group.immutable_id} already used")
             raise ACM::InvalidRequest.new("Group id #{existing_group.immutable_id} already used")
           end
 
-          if(!opts[:members].nil?)
+          unless opts[:members].nil?
             members = opts[:members]
-            if(members.kind_of?(Array))
+            if members.kind_of?(Array)
               members.each {|member|
-                if(!member.nil?)
+                unless member.nil?
                   begin
                     user = ACM::Models::Subjects.filter(:immutable_id => member).first()
-                    if(user.nil?)
+                    if user.nil?
                       @logger.error("Could not find user #{member}.")
                       raise ACM::ObjectNotFound.new("User #{member}")
                     end
@@ -84,14 +84,14 @@ def create_group(opts = {})
     def update_group(opts = {})
       @logger.debug("update_group parameters #{opts.inspect}")
 
-      if(opts[:id].nil?)
+      if opts[:id].nil?
         @logger.error("Empty group id to update")
         raise ACM::InvalidRequest.new("Empty group id")
       end
 
       group = ACM::Models::Subjects.filter(:immutable_id => opts[:id], :type => :group.to_s).first()
 
-      if(group.nil?)
+      if group.nil?
         @logger.error("Could not find group with id #{group_id.inspect}")
         raise ACM::ObjectNotFound.new("#{group_id.inspect}")
       else
@@ -102,14 +102,14 @@ def update_group(opts = {})
         ACM::Config.db.transaction do
           group[:additional_info] = opts[:additional_info]
           group.remove_all_members()
-          if(!opts[:members].nil?)
+          unless opts[:members].nil?
             members = opts[:members]
-            if(members.kind_of?(Array))
+            if members.kind_of?(Array)
               members.each {|member|
-                if(!member.nil?)
+                unless member.nil?
                   begin
                     user = ACM::Models::Subjects.filter(:immutable_id => member).first()
-                    if(user.nil?)
+                    if user.nil?
                       @logger.error("Could not find user #{member}.")
                       raise ACM::ObjectNotFound.new("User #{member}")
                     end
@@ -147,7 +147,7 @@ def find_group(group_id)
       @logger.debug("find_group parameters #{group_id.inspect}")
       group = ACM::Models::Subjects.filter(:immutable_id => group_id, :type => :group.to_s).first()
 
-      if(group.nil?)
+      if group.nil?
         @logger.error("Could not find group with id #{group_id.inspect}")
         raise ACM::ObjectNotFound.new("#{group_id.inspect}")
       else
@@ -166,7 +166,7 @@ def add_user_to_group(group_id, user_id)
       @logger.debug("find_group parameters #{group_id.inspect} #{user_id}")
       group = ACM::Models::Subjects.filter(:immutable_id => group_id, :type => :group.to_s).first()
 
-      if(group.nil?)
+      if group.nil?
         @logger.error("Could not group user with id #{group_id.inspect}")
         raise ACM::ObjectNotFound.new("#{group_id.inspect}")
       else
@@ -190,7 +190,7 @@ def add_user_to_group(group_id, user_id)
       #Is the user already a member of the group?
       group_members = group.members_dataset.filter(:user_id => user.id).all()
       @logger.debug("Existing group members #{group_members.inspect}")
-      if(group_members.nil? || group_members.size() == 0)
+      if group_members.nil? || group_members.size() == 0
         user = ACM::Models::Subjects.filter(:immutable_id => user_id, :type => :user.to_s).first()
         @logger.debug("new user #{user.id} group #{group.id}")
         group.add_member(:user_id => user.id)
@@ -209,7 +209,7 @@ def remove_user_from_group(group_id, user_id)
       @logger.debug("remove_user_from_group parameters #{group_id.inspect} #{user_id.inspect}")
       group = ACM::Models::Subjects.filter(:immutable_id => group_id, :type => :group.to_s).first()
 
-      if(group.nil?)
+      if group.nil?
         @logger.error("Could not group user with id #{group_id.inspect}")
         raise ACM::ObjectNotFound.new("#{group_id.inspect}")
       else
@@ -254,7 +254,7 @@ def delete_group(group_id)
       @logger.debug("delete parameters #{group_id.inspect}")
       group = ACM::Models::Subjects.filter(:immutable_id => group_id, :type => :group.to_s).first()
 
-      if(group.nil?)
+      if group.nil?
         @logger.error("Could not find group with id #{group_id.inspect}")
         raise ACM::ObjectNotFound.new("#{group_id.inspect}")
       else