Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed passing context argument to is_observed #607

Merged
merged 2 commits into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions core/constraints/abstract.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,9 @@ class ConstraintVerification(ABC):
app_name = ConstraintApp.GENERAL.value
__response_text = ""

def __init__(self, user_profile, context=None) -> None:
def __init__(self, user_profile) -> None:
self.user_profile = user_profile
self._param_values = {}
self.context = context

def get_info(self, *args, **kwargs):
pass
Expand Down
8 changes: 5 additions & 3 deletions core/constraints/captcha.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,19 @@ class HasVerifiedCloudflareCaptcha(ConstraintVerification):

def is_observed(self, *args, **kwargs) -> bool:

if self.context is None or self.context.get("requset") is None:
context = kwargs.get("context")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider refactoring to use self.context directly.

The new code introduces unnecessary complexity by using kwargs to extract context, which is redundant since self.context is already available. This adds an extra layer of indirection without clear benefits and can lead to inconsistencies, especially if the rest of the class relies on instance variables. Additionally, the code does not check if context is provided in kwargs, potentially leading to errors. Consider refactoring to maintain simplicity and consistency with the original design, using self.context directly. This approach reduces complexity and aligns with object-oriented principles.


if context is None or context.get("request") is None:
return False

cloudflare = CloudflareUtil()

request_context: RequestContextExtractor = RequestContextExtractor(
self.context["requset"]
context["request"]
)

turnstile_token = request_context.data.get("cf-turnstile-response")

return turnstile_token is not None and cloudflare.is_verified(
return request_context.ip is not None and turnstile_token is not None and cloudflare.is_verified(
turnstile_token, request_context.ip
)
6 changes: 3 additions & 3 deletions prizetap/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def __init__(self, *args, **kwargs):
self.user_profile: UserProfile = kwargs["user_profile"]
self.raffle: Raffle = kwargs["raffle"]
self.raffle_data: dict = kwargs.get("raffle_data", dict())
self.request = kwargs.get("requset")
self.request = kwargs.get("request")

def can_enroll_in_raffle(self):
if not self.raffle.is_claimable:
Expand All @@ -30,7 +30,7 @@ def check_user_constraints(self, raise_exception=True):
result = dict()
for c in self.raffle.constraints.all():
constraint: ConstraintVerification = get_constraint(c.name)(
self.user_profile, context={"request": self.request}
self.user_profile
)
constraint.response = c.response
try:
Expand All @@ -56,7 +56,7 @@ def check_user_constraints(self, raise_exception=True):
)
else:
is_verified = constraint.is_observed(
**cdata, from_time=int(self.raffle.start_at.timestamp())
**cdata, from_time=int(self.raffle.start_at.timestamp()), context={"request": self.request}
)
caching_time = 60 * 60 if is_verified else 60
expiration_time = time.time() + caching_time
Expand Down
2 changes: 1 addition & 1 deletion tokenTap/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ def check_user_permissions(self, raise_exception=True):
for c in self.td.constraints.all():
constraint: ConstraintVerification = get_constraint(c.name)(
self.user_profile,
context={"request": self.request}
)
constraint.response = c.response
try:
Expand All @@ -85,6 +84,7 @@ def check_user_permissions(self, raise_exception=True):
is_verified = constraint.is_observed(
**cdata,
token_distribution=self.td,
context={"request": self.request}
)
caching_time = 60 * 60 if is_verified else 60
expiration_time = time.time() + caching_time
Expand Down
Loading