Commit eb28ba8f authored by Lin Jen-Shin's avatar Lin Jen-Shin

Always check override where prepend happens

Because otherwise helpers won't be verified, and since we're using
prepend to override direct methods mostly, it should work anyway.

We also skip instrumentation when we're doing static verification.

Also we need to move some methods from geo_git_access to git_access,
because that's where it's trying to override methods, not in geo itself.

This also assumes GitAccessWiki does not use those methods, of course.
It won't work anyway, because it's not getting methods from GitAccess!
parent 8c3165ef
...@@ -205,4 +205,4 @@ module TodosHelper ...@@ -205,4 +205,4 @@ module TodosHelper
end end
end end
TodosHelper.prepend_if_ee('EE::NotesHelper'); TodosHelper.prepend_if_ee('EE::TodosHelper') # rubocop: disable Style/Semicolon TodosHelper.prepend_if_ee('EE::TodosHelper')
...@@ -8,6 +8,8 @@ ...@@ -8,6 +8,8 @@
# #
# rubocop:disable Metrics/AbcSize # rubocop:disable Metrics/AbcSize
def instrument_classes(instrumentation) def instrument_classes(instrumentation)
return if ENV['STATIC_VERIFICATION']
instrumentation.instrument_instance_methods(Gitlab::Shell) instrumentation.instrument_instance_methods(Gitlab::Shell)
instrumentation.instrument_methods(Gitlab::Git) instrumentation.instrument_methods(Gitlab::Git)
......
...@@ -53,7 +53,7 @@ Refer to <https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/utils/over ...@@ -53,7 +53,7 @@ Refer to <https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/utils/over
- This utility can help you check if one method would override - This utility can help you check if one method would override
another or not. It is the same concept as Java's `@Override` annotation another or not. It is the same concept as Java's `@Override` annotation
or Scala's `override` keyword. However, you should only do this check when or Scala's `override` keyword. However, we only run this check when
`ENV['STATIC_VERIFICATION']` is set to avoid production runtime overhead. `ENV['STATIC_VERIFICATION']` is set to avoid production runtime overhead.
This is useful for checking: This is useful for checking:
...@@ -94,6 +94,15 @@ Refer to <https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/utils/over ...@@ -94,6 +94,15 @@ Refer to <https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/utils/over
end end
``` ```
Note that the check will only happen when either:
- The overriding method is defined in a class, or:
- The overriding method is defined in a module, and it's prepended to
a class or a module.
Because only a class or prepended module can actually override a method.
Including or extending a module into another cannot override anything.
## `StrongMemoize` ## `StrongMemoize`
Refer to <https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/utils/strong_memoize.rb>: Refer to <https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/utils/strong_memoize.rb>:
......
...@@ -4,8 +4,8 @@ module EE ...@@ -4,8 +4,8 @@ module EE
module NavHelper module NavHelper
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
override :show_separator? override :has_extra_nav_icons?
def show_separator? def has_extra_nav_icons?
super || can?(current_user, :read_operations_dashboard) super || can?(current_user, :read_operations_dashboard)
end end
......
...@@ -10,21 +10,6 @@ module EE ...@@ -10,21 +10,6 @@ module EE
GEO_SERVER_DOCS_URL = 'https://docs.gitlab.com/ee/administration/geo/replication/using_a_geo_server.html'.freeze GEO_SERVER_DOCS_URL = 'https://docs.gitlab.com/ee/administration/geo/replication/using_a_geo_server.html'.freeze
override :check_custom_action
def check_custom_action(cmd)
custom_action = custom_action_for(cmd)
return custom_action if custom_action
super
end
override :check_for_console_messages
def check_for_console_messages(cmd)
super.push(
*current_replication_lag_message
)
end
protected protected
def project_or_wiki def project_or_wiki
......
...@@ -34,6 +34,21 @@ module EE ...@@ -34,6 +34,21 @@ module EE
private private
override :check_custom_action
def check_custom_action(cmd)
custom_action = custom_action_for(cmd)
return custom_action if custom_action
super
end
override :check_for_console_messages
def check_for_console_messages(cmd)
super.push(
*current_replication_lag_message
)
end
override :check_download_access! override :check_download_access!
def check_download_access! def check_download_access!
return if geo? return if geo?
......
...@@ -146,7 +146,8 @@ module Gitlab ...@@ -146,7 +146,8 @@ module Gitlab
def prepended(base = nil) def prepended(base = nil)
super super
queue_verification(base) if base # prepend can override methods, thus we need to verify it like classes
queue_verification(base, verify: true) if base
end end
def extended(mod = nil) def extended(mod = nil)
...@@ -155,11 +156,15 @@ module Gitlab ...@@ -155,11 +156,15 @@ module Gitlab
queue_verification(mod.singleton_class) if mod queue_verification(mod.singleton_class) if mod
end end
def queue_verification(base) def queue_verification(base, verify: false)
return unless ENV['STATIC_VERIFICATION'] return unless ENV['STATIC_VERIFICATION']
if base.is_a?(Class) # We could check for Class in `override` # We could check for Class in `override`
# This could be `nil` if `override` was never called # This could be `nil` if `override` was never called.
# We also force verification for prepend because it can also override
# a method like a class, but not the cases for include or extend.
# This includes Rails helpers but not limited to.
if base.is_a?(Class) || verify
Override.extensions[self]&.add_class(base) Override.extensions[self]&.add_class(base)
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment