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

[Bug] [Jac Cloud] Private & Protected Walker Attributes are queryable in endpoint #1338

Open
ChrisIsKing opened this issue Oct 4, 2024 · 5 comments · May be fixed by #1364
Open

[Bug] [Jac Cloud] Private & Protected Walker Attributes are queryable in endpoint #1338

ChrisIsKing opened this issue Oct 4, 2024 · 5 comments · May be fixed by #1364
Assignees
Labels
jac cloud Issues related to the jac-cloud package jivas Issues of high priority to the Jivas Product

Comments

@ChrisIsKing
Copy link
Collaborator

Describe the bug

Private (:priv) and protected (:protect) walker attributes are queryable in Jac Cloud endpoint responses, which violates the expected encapsulation of such attributes. The expected behavior is that only public attributes (:pub) should be visible, while private and protected attributes should remain inaccessible to external queries.

To Reproduce

  1. Create a walker with private, protected, and public attributes as shown below.
  2. Use an endpoint to query the walker's state after execution.
  3. Observe that both private (var_a) and protected (var_b) attributes are included in the endpoint request body and the endpoint response, even though they should not be.
walker test_walker {
    has :priv var_a: `Any = "";
    has :protect var_b: `Any = "";
    has :pub var_c: `Any = "";

    obj __specs__ {
        static has auth: bool = False;
    }

    can update_values with `root entry {
        self.var_a = self.var_a + "Updated value";
        self.var_b = self.var_b + "Updated value";
        self.var_c = self.var_c + "Updated value";
    }

    can report_value with `root exit {
        report self;
    }
}
Screenshot 2024-10-04 at 12 18 46 AM Screenshot 2024-10-04 at 12 19 01 AM
@ChrisIsKing ChrisIsKing added the jac cloud Issues related to the jac-cloud package label Oct 4, 2024
@ChrisIsKing ChrisIsKing added the jivas Issues of high priority to the Jivas Product label Oct 8, 2024
@amadolid
Copy link
Collaborator

amadolid commented Oct 9, 2024

private, protected, public approach might be for different context like inheritance and encapsulation.

We can use additional option on __specs__, however, It might cause some restriction.

  • since we are using instance variable, it requires dev to declare default/default_factory as it's not included in request payload (url/query params, body, file) parsing.
  • it will be included in dataclass fields. This include constructor and deserialization. Devs needs to override those methods just to remove it.

I think the right way to do it is by using python's behavior (ClassVar).
Although, it's declared as Class' variable, we usually do it for type hinting that the current class has this variable.
It will also work with inheritance and they may set it as optional variable.

from dataclasses import dataclass, field
from typing import ClassVar

@dataclass
class WithPrivate1:
    immutable_var: int = 1
    mutable_var: dict = field(default_factory=dict)

    var_for_type_hint: ClassVar[int]
    
# same with
class WithPrivate2:
    var_for_type_hint: int

    def __init__(self, immutable_var: int = 1, mutable_var: dict = None) -> None:
        self.immutable_var = immutable_var
        self.mutable_var = mutable_var or {}

using has static val: int will be converted to val: ClassVar[int]

walker sample{
    has static val: int;
}

@ChrisIsKing
Copy link
Collaborator Author

Yes, I agree that private, protected and public are for preserving inheritance and encapsulation. So how do you cater for the case where you want the variable to be instance specific but also not exposed on the api. Since static vars are shared among all instances of the class, future instances of the walker may run into issues when accessing these vars no?.

@amadolid
Copy link
Collaborator

Python handles ClassVar (equivalent to has static var) differently .

Since Python doesn't statically define variables in a class similar to other OOP languages, ClassVar can be used in different purposes.

For example:

class Sample:
    val1: int = 1
    val2: int
    # similar to 
    # val1: ClassVar[int] = 1
    # val2: ClassVar[int]

val1 will be a class variable that's accessible in instance and child classes/instances.
this can be access via self.val1 or Sample.val1

val2 is different. Since we don't specify it's value, it will not be available in instance and child classes/instances. However, type hinting will still be available. It's like Sample classs/instance "should" have val2 with int type. This will require to be assigned first before it can be accessed.

val1 and val2 might be different but both can still be utilize as instance variable.
assigning variable to both using instance variable (ex: self) will only assign it to the instance. It will not update class variable value.

s = Sample()
s.val1 = 10

print(s.val1) # prints 10
print(Sample.val1) # prints 1

print(s.val2) # will error
s.val2 = 1
print(s.val2) # prints 1

In this context, I think using has static var is the right way to have a private like field for walker API

@amadolid
Copy link
Collaborator

adding option to hide fields using __specs__ might add some confusion/restriction.
Since that field will no longer be available in API request, we will require dev to set a default value/factory.
We will also require them to override __serialize__ and __deserialize__ if they wish to don't include it on db or response

However, I will add it just to support this edge case:
Hidden in API, included in db and don't want to override __serialize__ and __deserialize__

@amadolid amadolid linked a pull request Oct 11, 2024 that will close this issue
@ypkang
Copy link
Contributor

ypkang commented Oct 14, 2024

from __future__ import annotations
import typing as _jac_typ
from jaclang.plugin.feature import JacFeature as _Jac
from jaclang.plugin.builtin import *
from dataclasses import dataclass as __jac_dataclass__

@_Jac.make_walker(on_entry=[_Jac.DSFunc('update_values', _Jac.RootType)], on_exit=[_Jac.DSFunc('report_value', _Jac.RootType)])
@__jac_dataclass__(eq=False)
class test_walker(_Jac.Walker):
    var_a: _jac_typ.Any = _Jac.has_instance_default(gen_func=lambda: '')
    var_b: _jac_typ.Any = _Jac.has_instance_default(gen_func=lambda: '')
    var_c: _jac_typ.Any = _Jac.has_instance_default(gen_func=lambda: '')

    @_Jac.make_obj(on_entry=[], on_exit=[])
    @__jac_dataclass__(eq=False)
    class __specs__(_Jac.Obj):
        auth: _jac_typ.ClassVar[bool] = False

    def update_values(self, _jac_here_: _Jac.RootType) -> None:
        self.var_a = self.var_a + 'Updated value'
        self.var_b = self.var_b + 'Updated value'
        self.var_c = self.var_c + 'Updated value'

    def report_value(self, _jac_here_: _Jac.RootType) -> None:
        _Jac.report(self)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jac cloud Issues related to the jac-cloud package jivas Issues of high priority to the Jivas Product
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants