Merge branch 'dmeshcharakou/rubocop-packages-fix-strong-memoize-attr' into 'master'

Fix strong_memoize_attr rubocop violations

See merge request gitlab-org/gitlab!122479



Merged-by: Gary Holtz <gholtz@gitlab.com>
Approved-by: Gary Holtz <gholtz@gitlab.com>
Reviewed-by: Tiger Watson <twatson@gitlab.com>
Co-authored-by: Dzmitry Meshcharakou <12459192-dmeshcharakou@users.noreply.gitlab.com>

.rubocop_todo/gitlab/strong_memoize_attr.yml

@@ -218,28 +218,6 @@ Gitlab/StrongMemoizeAttr:
     - 'app/services/metrics/dashboard/dynamic_embed_service.rb'
     - 'app/services/metrics/dashboard/gitlab_alert_embed_service.rb'
     - 'app/services/namespaces/package_settings/update_service.rb'
-    - 'app/services/packages/cleanup/execute_policy_service.rb'
-    - 'app/services/packages/cleanup/update_policy_service.rb'
-    - 'app/services/packages/composer/create_package_service.rb'
-    - 'app/services/packages/debian/extract_changes_metadata_service.rb'
-    - 'app/services/packages/debian/generate_distribution_key_service.rb'
-    - 'app/services/packages/debian/generate_distribution_service.rb'
-    - 'app/services/packages/debian/process_changes_service.rb'
-    - 'app/services/packages/helm/process_file_service.rb'
-    - 'app/services/packages/maven/metadata/base_create_xml_service.rb'
-    - 'app/services/packages/maven/metadata/create_plugins_xml_service.rb'
-    - 'app/services/packages/maven/metadata/create_versions_xml_service.rb'
-    - 'app/services/packages/maven/metadata/sync_service.rb'
-    - 'app/services/packages/npm/create_package_service.rb'
-    - 'app/services/packages/npm/create_tag_service.rb'
-    - 'app/services/packages/nuget/metadata_extraction_service.rb'
-    - 'app/services/packages/nuget/search_service.rb'
-    - 'app/services/packages/pypi/create_package_service.rb'
-    - 'app/services/packages/rpm/parse_package_service.rb'
-    - 'app/services/packages/rubygems/dependency_resolver_service.rb'
-    - 'app/services/packages/rubygems/process_gem_service.rb'
-    - 'app/services/packages/terraform_module/create_package_service.rb'
-    - 'app/services/packages/update_tags_service.rb'
     - 'app/services/projects/container_repository/cleanup_tags_base_service.rb'
     - 'app/services/projects/container_repository/third_party/cleanup_tags_service.rb'
     - 'app/services/projects/create_from_template_service.rb'

app/services/packages/cleanup/execute_policy_service.rb

@@ -79,10 +79,9 @@ def installable_package_files
       end
 
       def batch_deadline
-        strong_memoize(:batch_deadline) do
-          MAX_EXECUTION_TIME.from_now
-        end
+        MAX_EXECUTION_TIME.from_now
       end
+      strong_memoize_attr :batch_deadline
 
       def response_success(timeout:)
         ServiceResponse.success(

app/services/packages/cleanup/update_policy_service.rb

@@ -18,10 +18,9 @@ def execute
       private
 
       def policy
-        strong_memoize(:policy) do
-          project.packages_cleanup_policy
-        end
+        project.packages_cleanup_policy
       end
+      strong_memoize_attr :policy
 
       def allowed?
         can?(current_user, :admin_package, project)

app/services/packages/composer/create_package_service.rb

@@ -27,10 +27,9 @@ def created_package
       end
 
       def composer_json
-        strong_memoize(:composer_json) do
-          ::Packages::Composer::ComposerJsonService.new(project, target).execute
-        end
+        ::Packages::Composer::ComposerJsonService.new(project, target).execute
       end
+      strong_memoize_attr :composer_json
 
       def package_name
         composer_json['name']

app/services/packages/debian/extract_changes_metadata_service.rb

@@ -26,10 +26,9 @@ def execute
       private
 
       def metadata
-        strong_memoize(:metadata) do
-          ::Packages::Debian::ExtractMetadataService.new(@package_file).execute
-        end
+        ::Packages::Debian::ExtractMetadataService.new(@package_file).execute
       end
+      strong_memoize_attr :metadata
 
       def file_type
         metadata[:file_type]
@@ -40,20 +39,19 @@ def fields
       end
 
       def files
-        strong_memoize(:files) do
-          raise ExtractionError, "is not a changes file" unless file_type == :changes
-          raise ExtractionError, "Files field is missing" if fields['Files'].blank?
-          raise ExtractionError, "Checksums-Sha1 field is missing" if fields['Checksums-Sha1'].blank?
-          raise ExtractionError, "Checksums-Sha256 field is missing" if fields['Checksums-Sha256'].blank?
-
-          init_entries_from_files
-          entries_from_checksums_sha1
-          entries_from_checksums_sha256
-          entries_from_package_files
-
-          @entries
-        end
+        raise ExtractionError, "is not a changes file" unless file_type == :changes
+        raise ExtractionError, "Files field is missing" if fields['Files'].blank?
+        raise ExtractionError, "Checksums-Sha1 field is missing" if fields['Checksums-Sha1'].blank?
+        raise ExtractionError, "Checksums-Sha256 field is missing" if fields['Checksums-Sha256'].blank?
+
+        init_entries_from_files
+        entries_from_checksums_sha1
+        entries_from_checksums_sha256
+        entries_from_package_files
+
+        @entries
       end
+      strong_memoize_attr :files
 
       def init_entries_from_files
         each_lines_for('Files') do |line|

app/services/packages/debian/generate_distribution_key_service.rb

@@ -43,10 +43,9 @@ def execute
       attr_reader :params
 
       def passphrase
-        strong_memoize(:passphrase) do
-          params[:passphrase] || ::User.random_password
-        end
+        params[:passphrase] || ::User.random_password
       end
+      strong_memoize_attr :passphrase
 
       def pinentry_script_content
         escaped_passphrase = Shellwords.escape(passphrase)

app/services/packages/debian/generate_distribution_service.rb

@@ -213,10 +213,9 @@ def generate_release
       end
 
       def release_content
-        strong_memoize(:release_content) do
-          release_header + release_sums
-        end
+        release_header + release_sums
       end
+      strong_memoize_attr :release_content
 
       def release_header
         [
@@ -235,10 +234,9 @@ def release_header
       end
 
       def release_date
-        strong_memoize(:release_date) do
-          Time.now.utc
-        end
+        Time.now.utc
       end
+      strong_memoize_attr :release_date
 
       def release_sums
         # NB: MD5Sum was removed for FIPS compliance

app/services/packages/debian/process_changes_service.rb

@@ -76,10 +76,9 @@ def update_changes_metadata
       end
 
       def metadata
-        strong_memoize(:metadata) do
-          ::Packages::Debian::ExtractChangesMetadataService.new(package_file).execute
-        end
+        ::Packages::Debian::ExtractChangesMetadataService.new(package_file).execute
       end
+      strong_memoize_attr :metadata
 
       def files
         metadata[:files]
@@ -90,16 +89,15 @@ def project
       end
 
       def package
-        strong_memoize(:package) do
-          params = {
-            'name': metadata[:fields]['Source'],
-            'version': metadata[:fields]['Version'],
-            'distribution_name': metadata[:fields]['Distribution']
-          }
-          response = Packages::Debian::FindOrCreatePackageService.new(project, creator, params).execute
-          response.payload[:package]
-        end
+        params = {
+          'name': metadata[:fields]['Source'],
+          'version': metadata[:fields]['Version'],
+          'distribution_name': metadata[:fields]['Distribution']
+        }
+        response = Packages::Debian::FindOrCreatePackageService.new(project, creator, params).execute
+        response.payload[:package]
       end
+      strong_memoize_attr :package
 
       # used by ExclusiveLeaseGuard
       def lease_key

app/services/packages/helm/process_file_service.rb

@@ -57,28 +57,25 @@ def cleanup_temp_package
       end
 
       def temp_package
-        strong_memoize(:temp_package) do
-          package_file.package
-        end
+        package_file.package
       end
+      strong_memoize_attr :temp_package
 
       def package
-        strong_memoize(:package) do
-          project_packages = package_file.package.project.packages
-          package = project_packages.with_package_type(:helm)
-                                    .with_name(metadata['name'])
-                                    .with_version(metadata['version'])
-                                    .not_pending_destruction
-                                    .last
-          package || temp_package
-        end
+        project_packages = package_file.package.project.packages
+        package = project_packages.with_package_type(:helm)
+                                  .with_name(metadata['name'])
+                                  .with_version(metadata['version'])
+                                  .not_pending_destruction
+                                  .last
+        package || temp_package
       end
+      strong_memoize_attr :package
 
       def metadata
-        strong_memoize(:metadata) do
-          ::Packages::Helm::ExtractFileMetadataService.new(package_file).execute
-        end
+        ::Packages::Helm::ExtractFileMetadataService.new(package_file).execute
       end
+      strong_memoize_attr :metadata
 
       def file_name
         "#{metadata['name']}-#{metadata['version']}.tgz"

app/services/packages/maven/metadata/base_create_xml_service.rb

@@ -19,12 +19,11 @@ def initialize(metadata_content:, package:, logger: nil)
         attr_reader :logger
 
         def xml_doc
-          strong_memoize(:xml_doc) do
-            Nokogiri::XML(@metadata_content) do |config|
-              config.default_xml.noblanks
-            end
+          Nokogiri::XML(@metadata_content) do |config|
+            config.default_xml.noblanks
           end
         end
+        strong_memoize_attr :xml_doc
 
         def xml_node(name, content)
           xml_doc.create_element(name).tap { |e| e.content = content }

app/services/packages/maven/metadata/create_plugins_xml_service.rb

@@ -40,37 +40,34 @@ def update_plugins_list
         end
 
         def plugins_xml_node
-          strong_memoize(:plugins_xml_node) do
-            xml_doc.xpath(XPATH_PLUGINS)
+          xml_doc.xpath(XPATH_PLUGINS)
                    .first
-          end
         end
+        strong_memoize_attr :plugins_xml_node
 
         def plugin_artifact_ids_from_xml
-          strong_memoize(:plugin_artifact_ids_from_xml) do
-            plugins_xml_node.xpath(XPATH_PLUGIN_ARTIFACT_ID)
+          plugins_xml_node.xpath(XPATH_PLUGIN_ARTIFACT_ID)
                             .map(&:content)
-          end
         end
+        strong_memoize_attr :plugin_artifact_ids_from_xml
 
         def plugin_artifact_ids_from_database
-          strong_memoize(:plugin_artifact_ids_from_database) do
-            package_names = plugin_artifact_ids_from_xml.map do |artifact_id|
-              "#{@package.name}/#{artifact_id}"
-            end
-
-            packages = @package.project.packages
-                                       .maven
-                                       .displayable
-                                       .with_name(package_names)
-                                       .has_version
-
-            ::Packages::Maven::Metadatum.for_package_ids(packages.select(:id))
-                                        .order_created
-                                        .pluck_app_name
-                                        .uniq
+          package_names = plugin_artifact_ids_from_xml.map do |artifact_id|
+            "#{@package.name}/#{artifact_id}"
           end
+
+          packages = @package.project.packages
+                                     .maven
+                                     .displayable
+                                     .with_name(package_names)
+                                     .has_version
+
+          ::Packages::Maven::Metadatum.for_package_ids(packages.select(:id))
+                                      .order_created
+                                      .pluck_app_name
+                                      .uniq
         end
+        strong_memoize_attr :plugin_artifact_ids_from_database
 
         def plugin_node_for(artifact_id)
           xml_doc.create_element('plugin').tap do |plugin_node|

app/services/packages/maven/metadata/create_versions_xml_service.rb

@@ -91,49 +91,43 @@ def update_last_updated_timestamp
         end
 
         def versioning_xml_node
-          strong_memoize(:versioning_xml_node) do
-            xml_doc.xpath(XPATH_VERSIONING).first
-          end
+          xml_doc.xpath(XPATH_VERSIONING).first
         end
+        strong_memoize_attr :versioning_xml_node
 
         def versions_xml_node
-          strong_memoize(:versions_xml_node) do
-            versioning_xml_node&.xpath(XPATH_VERSIONS)
+          versioning_xml_node&.xpath(XPATH_VERSIONS)
                                &.first
-          end
         end
+        strong_memoize_attr :versions_xml_node
 
         def version_xml_nodes
           versions_xml_node&.xpath(XPATH_VERSION)
         end
 
         def latest_xml_node
-          strong_memoize(:latest_xml_node) do
-            versioning_xml_node&.xpath(XPATH_LATEST)
+          versioning_xml_node&.xpath(XPATH_LATEST)
                                &.first
-          end
         end
+        strong_memoize_attr :latest_xml_node
 
         def release_xml_node
-          strong_memoize(:release_xml_node) do
-            versioning_xml_node&.xpath(XPATH_RELEASE)
+          versioning_xml_node&.xpath(XPATH_RELEASE)
                                &.first
-          end
         end
+        strong_memoize_attr :release_xml_node
 
         def last_updated_xml_node
-          strong_memoize(:last_updated_xml_mode) do
-            versioning_xml_node.xpath(XPATH_LAST_UPDATED)
+          versioning_xml_node.xpath(XPATH_LAST_UPDATED)
                                .first
-          end
         end
+        strong_memoize_attr :last_updated_xml_node
 
         def versions_from_xml
-          strong_memoize(:versions_from_xml) do
-            versions_xml_node.xpath(XPATH_VERSION)
+          versions_xml_node.xpath(XPATH_VERSION)
                              .map(&:text)
-          end
         end
+        strong_memoize_attr :versions_from_xml
 
         def latest_from_xml
           latest_xml_node&.text
@@ -144,27 +138,25 @@ def release_from_xml
         end
 
         def versions_from_database
-          strong_memoize(:versions_from_database) do
-            @package.project.packages
+          @package.project.packages
                             .maven
                             .displayable
                             .with_name(@package.name)
                             .has_version
                             .order_created
                             .pluck_versions
-          end
         end
+        strong_memoize_attr :versions_from_database
 
         def latest_from_database
           versions_from_database.last
         end
 
         def release_from_database
-          strong_memoize(:release_from_database) do
-            non_snapshot_versions_from_database = versions_from_database.reject { |v| v.ends_with?('SNAPSHOT') }
-            non_snapshot_versions_from_database.last
-          end
+          non_snapshot_versions_from_database = versions_from_database.reject { |v| v.ends_with?('SNAPSHOT') }
+          non_snapshot_versions_from_database.last
         end
+        strong_memoize_attr :release_from_database
 
         def log_malformed_content(reason)
           logger.warn(

app/services/packages/maven/metadata/sync_service.rb

@@ -70,25 +70,22 @@ def update_xml(kind:, package_file:, service_class:, payload_empty_field:)
         end
 
         def metadata_package_file_for_versions
-          strong_memoize(:metadata_file_for_versions) do
-            metadata_package_file_for(versionless_package_for_versions)
-          end
+          metadata_package_file_for(versionless_package_for_versions)
         end
+        strong_memoize_attr :metadata_package_file_for_versions
 
         def versionless_package_for_versions
-          strong_memoize(:versionless_package_for_versions) do
-            versionless_package_named(package_name)
-          end
+          versionless_package_named(package_name)
         end
+        strong_memoize_attr :versionless_package_for_versions
 
         def metadata_package_file_for_plugins
-          strong_memoize(:metadata_package_file_for_plugins) do
-            pkg_name = package_name_for_plugins
-            next unless pkg_name
+          pkg_name = package_name_for_plugins
+          return unless pkg_name
 
-            metadata_package_file_for(versionless_package_named(package_name_for_plugins))
-          end
+          metadata_package_file_for(versionless_package_named(package_name_for_plugins))
         end
+        strong_memoize_attr :metadata_package_file_for_plugins
 
         def metadata_package_file_for(package)
           return unless package

app/services/packages/npm/create_package_service.rb

@@ -61,10 +61,9 @@ def name
       end
 
       def version
-        strong_memoize(:version) do
-          params[:versions].each_key.first
-        end
+        params[:versions].each_key.first
       end
+      strong_memoize_attr :version
 
       def version_data
         params[:versions][version]
@@ -79,30 +78,27 @@ def dist_tag
       end
 
       def package_file_name
-        strong_memoize(:package_file_name) do
-          "#{name}-#{version}.tgz"
-        end
+        "#{name}-#{version}.tgz"
       end
+      strong_memoize_attr :package_file_name
 
       def attachment
-        strong_memoize(:attachment) do
-          params['_attachments'][package_file_name]
-        end
+        params['_attachments'][package_file_name]
       end
+      strong_memoize_attr :attachment
 
       # TODO (technical debt): Extract the package size calculation to its own component and unit test it separately.
       def calculated_package_file_size
-        strong_memoize(:calculated_package_file_size) do
-          # This calculation is based on:
-          # 1. 4 chars in a Base64 encoded string are 3 bytes in the original string. Meaning 1 char is 0.75 bytes.
-          # 2. The encoded string may have 1 or 2 extra '=' chars used for padding. Each padding char means 1 byte less in the original string.
-          # Reference:
-          # - https://blog.aaronlenoir.com/2017/11/10/get-original-length-from-base-64-string/
-          # - https://en.wikipedia.org/wiki/Base64#Decoding_Base64_with_padding
-          encoded_data = attachment['data']
-          ((encoded_data.length * 0.75) - encoded_data[-2..].count('=')).to_i
-        end
+        # This calculation is based on:
+        # 1. 4 chars in a Base64 encoded string are 3 bytes in the original string. Meaning 1 char is 0.75 bytes.
+        # 2. The encoded string may have 1 or 2 extra '=' chars used for padding. Each padding char means 1 byte less in the original string.
+        # Reference:
+        # - https://blog.aaronlenoir.com/2017/11/10/get-original-length-from-base-64-string/
+        # - https://en.wikipedia.org/wiki/Base64#Decoding_Base64_with_padding
+        encoded_data = attachment['data']
+        ((encoded_data.length * 0.75) - encoded_data[-2..].count('=')).to_i
       end
+      strong_memoize_attr :calculated_package_file_size
 
       def file_params
         {
@@ -134,29 +130,26 @@ def lease_timeout
       end
 
       def field_sizes
-        strong_memoize(:field_sizes) do
-          package_json.transform_values do |value|
-            value.to_s.size
-          end
+        package_json.transform_values do |value|
+          value.to_s.size
         end
       end
+      strong_memoize_attr :field_sizes
 
       def filtered_field_sizes
-        strong_memoize(:filtered_field_sizes) do
-          field_sizes.select do |_, size|
-            size >= ::Packages::Npm::Metadatum::MIN_PACKAGE_JSON_FIELD_SIZE_FOR_ERROR_TRACKING
-          end
+        field_sizes.select do |_, size|
+          size >= ::Packages::Npm::Metadatum::MIN_PACKAGE_JSON_FIELD_SIZE_FOR_ERROR_TRACKING
         end
       end
+      strong_memoize_attr :filtered_field_sizes
 
       def largest_fields
-        strong_memoize(:largest_fields) do
-          field_sizes
+        field_sizes
             .sort_by { |a| a[1] }
             .reverse[0..::Packages::Npm::Metadatum::NUM_FIELDS_FOR_ERROR_TRACKING - 1]
             .to_h
-        end
       end
+      strong_memoize_attr :largest_fields
 
       def field_sizes_for_error_tracking
         filtered_field_sizes.empty? ? largest_fields : filtered_field_sizes

app/services/packages/npm/create_tag_service.rb

@@ -23,12 +23,11 @@ def execute
       private
 
       def existing_tag
-        strong_memoize(:existing_tag) do
-          Packages::TagsFinder
+        Packages::TagsFinder
             .new(package.project, package.name, package_type: package.package_type)
             .find_by_name(tag_name)
-        end
       end
+      strong_memoize_attr :existing_tag
     end
   end
 end

app/services/packages/nuget/metadata_extraction_service.rb

@@ -39,10 +39,9 @@ def execute
       private
 
       def package_file
-        strong_memoize(:package_file) do
-          ::Packages::PackageFile.find_by_id(@package_file_id)
-        end
+        ::Packages::PackageFile.find_by_id(@package_file_id)
       end
+      strong_memoize_attr :package_file
 
       def valid_package_file?
         package_file &&

app/services/packages/nuget/search_service.rb

@@ -89,17 +89,16 @@ def non_paginated_matching_package_names
       end
 
       def base_matching_package_names
-        strong_memoize(:base_matching_package_names) do
-          # rubocop: disable CodeReuse/ActiveRecord
-          pkgs = nuget_packages.order_name
+        # rubocop: disable CodeReuse/ActiveRecord
+        pkgs = nuget_packages.order_name
                                .select_distinct_name
                                .where(project_id: project_ids)
-          pkgs = pkgs.without_version_like(PRE_RELEASE_VERSION_MATCHING_TERM) unless include_prerelease_versions?
-          pkgs = pkgs.search_by_name(@search_term) if @search_term.present?
-          pkgs
-          # rubocop: enable CodeReuse/ActiveRecord
-        end
+        pkgs = pkgs.without_version_like(PRE_RELEASE_VERSION_MATCHING_TERM) unless include_prerelease_versions?
+        pkgs = pkgs.search_by_name(@search_term) if @search_term.present?
+        pkgs
+        # rubocop: enable CodeReuse/ActiveRecord
       end
+      strong_memoize_attr :base_matching_package_names
 
       def nuget_packages
         Packages::Package.nuget
@@ -111,11 +110,10 @@ def nuget_packages
       def project_ids_cte
         return unless use_project_ids_cte?
 
-        strong_memoize(:project_ids_cte) do
-          query = projects_visible_to_user(@current_user, within_group: @project_or_group)
-          Gitlab::SQL::CTE.new(:project_ids, query.select(:id))
-        end
+        query = projects_visible_to_user(@current_user, within_group: @project_or_group)
+        Gitlab::SQL::CTE.new(:project_ids, query.select(:id))
       end
+      strong_memoize_attr :project_ids_cte
 
       def project_ids
         return @project_or_group.id if project?

app/services/packages/pypi/create_package_service.rb

@@ -29,10 +29,9 @@ def execute
       private
 
       def created_package
-        strong_memoize(:created_package) do
-          find_or_create_package!(:pypi)
-        end
+        find_or_create_package!(:pypi)
       end
+      strong_memoize_attr :created_package
 
       def file_params
         {

app/services/packages/rpm/parse_package_service.rb

@@ -43,10 +43,9 @@ def valid_package?
       end
 
       def package_tags
-        strong_memoize(:package_tags) do
-          rpm.tags
-        end
+        rpm.tags
       end
+      strong_memoize_attr :package_tags
 
       def extract_static_attributes
         STATIC_ATTRIBUTES.index_with do |attribute|

app/services/packages/rubygems/dependency_resolver_service.rb

@@ -33,10 +33,9 @@ def execute
       private
 
       def packages
-        strong_memoize(:packages) do
-          project.packages.with_name(gem_name)
-        end
+        project.packages.with_name(gem_name)
       end
+      strong_memoize_attr :packages
 
       def gem_name
         params[:gem_name]