Merge pull request #830 from stoutput/fix-fillable-guarded-behavior

Fix fillable and guarded attribute behavior on mass-assignment

.envrc

@@ -0,0 +1,2 @@
+use asdf
+layout python

.github/workflows/pythonapp.yml

@@ -4,7 +4,7 @@ on: [push, pull_request]
 
 jobs:
   build:
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-20.04
 
     services:
       postgres:
@@ -58,7 +58,7 @@ jobs:
           python orm migrate --connection mysql
           make test
   lint:
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-20.04
     name: Lint
     steps:
       - uses: actions/checkout@v1

.github/workflows/pythonpublish.yml

@@ -6,7 +6,7 @@ on:
 
 jobs:
   build:
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-20.04
 
     services:
       postgres:

.gitignore

@@ -1,4 +1,5 @@
 venv
+.direnv
 .python-version
 .vscode
 .pytest_*
@@ -15,4 +16,6 @@ htmlcov/*
 coverage.xml
 .coverage
 *.log
-build
+build
+/orm.sqlite3
+/.bootstrapped-pip

.tool-versions

@@ -0,0 +1 @@
+python 3.8.10

config/test-database.py

@@ -0,0 +1,35 @@
+from src.masoniteorm.connections import ConnectionResolver
+
+DATABASES = {
+  "default": "mysql",
+  "mysql": {
+    "host": "127.0.0.1",
+    "driver": "mysql",
+    "database": "masonite",
+    "user": "root",
+    "password": "",
+    "port": 3306,
+    "log_queries": False,
+    "options": {
+      #
+    }
+  },
+  "postgres": {
+    "host": "127.0.0.1",
+    "driver": "postgres",
+    "database": "masonite",
+    "user": "root",
+    "password": "",
+    "port": 5432,
+    "log_queries": False,
+    "options": {
+      #
+    }
+  },
+  "sqlite": {
+    "driver": "sqlite",
+    "database": "masonite.sqlite3",
+  }
+}
+
+DB = ConnectionResolver().set_connection_details(DATABASES)

makefile

@@ -1,22 +1,25 @@
-init:
+init: .env .bootstrapped-pip
+
+.bootstrapped-pip: requirements.txt requirements.dev
+	pip install -r requirements.txt -r requirements.dev
+	touch .bootstrapped-pip
+
+.env:
 	cp .env-example .env
-	pip install -r requirements.txt
-	pip install .
-	# Create MySQL Database
-	# Create Postgres Database
-test:
+
+# 	Create MySQL Database
+# 	Create Postgres Database
+test: init
 	python -m pytest tests
 ci:
 	make test
+check: format sort lint
 lint:
 	python -m flake8 src/masoniteorm/ --ignore=E501,F401,E203,E128,E402,E731,F821,E712,W503,F811
-format:
-	black src/masoniteorm
-	black tests/
-	make lint
-sort:
-	isort tests
-	isort src/masoniteorm
+format: init
+	black src/masoniteorm tests/
+sort: init
+	isort src/masoniteorm tests/
 coverage:
 	python -m pytest --cov-report term --cov-report xml --cov=src/masoniteorm tests/
 	python -m coveralls

pytest.ini

@@ -0,0 +1,3 @@
+[pytest]
+env = 
+    D:DB_CONFIG_PATH=config/test-database

requirements.dev

@@ -0,0 +1,8 @@
+flake8==3.7.9
+black
+faker
+pytest
+pytest-cov
+pytest-env
+pymysql
+isort

requirements.txt

@@ -1,13 +1,6 @@
-flake8==3.7.9
-black==19.3b0
-faker
-pytest
-pytest-cov
-pymysql
-isort
 inflection==0.3.1
 psycopg2-binary
-python-dotenv==0.14.0
 pyodbc
 pendulum>=2.1,<2.2
-cleo>=0.8.0,<0.9
+cleo>=0.8.0,<0.9
+python-dotenv==0.14.0

src/masoniteorm/config.py

@@ -2,7 +2,8 @@
 import pydoc
 import urllib.parse as urlparse
 
-from .exceptions import ConfigurationNotFound, InvalidUrlConfiguration
+from .exceptions import ConfigurationNotFound
+from .exceptions import InvalidUrlConfiguration
 
 
 def load_config(config_path=None):
@@ -11,11 +12,11 @@ def load_config(config_path=None):
         1. try to load from DB_CONFIG_PATH environment variable
         2. else try to load from default config_path: config/database
     """
+    selected_config_path = (
+        os.getenv("DB_CONFIG_PATH", None) or config_path or "config/database"
+    )
 
-    if not os.getenv("DB_CONFIG_PATH", None):
-        os.environ["DB_CONFIG_PATH"] = config_path or "config/database"
-
-    selected_config_path = os.environ["DB_CONFIG_PATH"]
+    os.environ["DB_CONFIG_PATH"] = selected_config_path
 
     # format path as python module if needed
     selected_config_path = (

src/masoniteorm/models/Model.py

@@ -1,19 +1,21 @@
+import inspect
 import json
-from datetime import datetime, date as datetimedate, time as datetimetime
 import logging
+from datetime import date as datetimedate
+from datetime import datetime
+from datetime import time as datetimetime
 from decimal import Decimal
-
-from inflection import tableize, underscore
-import inspect
+from typing import Any, Dict
 
 import pendulum
+from inflection import tableize, underscore
 
-from ..query import QueryBuilder
 from ..collection import Collection
-from ..observers import ObservesEvents
-from ..scopes import TimeStampsMixin
 from ..config import load_config
 from ..exceptions import ModelNotFound
+from ..observers import ObservesEvents
+from ..query import QueryBuilder
+from ..scopes import TimeStampsMixin
 
 """This is a magic class that will help using models like User.first() instead of having to instatiate a class like
 User().first()
@@ -133,7 +135,7 @@ class Model(TimeStampsMixin, ObservesEvents, metaclass=ModelMeta):
     """
 
     __fillable__ = ["*"]
-    __guarded__ = ["*"]
+    __guarded__ = []
     __dry__ = False
     __table__ = None
     __connection__ = "default"
@@ -160,6 +162,8 @@ class Model(TimeStampsMixin, ObservesEvents, metaclass=ModelMeta):
     date_created_at = "created_at"
     date_updated_at = "updated_at"
 
+    builder: QueryBuilder
+
     """Pass through will pass any method calls to the model directly through to the query builder.
     Anytime one of these methods are called on the model it will actually be called on the query builder class.
     """
@@ -260,7 +264,7 @@ class Model(TimeStampsMixin, ObservesEvents, metaclass=ModelMeta):
             "with_",
             "with_count",
             "latest",
-            "oldest"
+            "oldest",
         )
     )
 
@@ -366,6 +370,15 @@ def boot(self):
 
                 if class_name.endswith("Mixin"):
                     getattr(self, "boot_" + class_name)(self.get_builder())
+                elif (
+                    base_class != Model
+                    and issubclass(base_class, Model)
+                    and "__fillable__" in base_class.__dict__
+                    and "__guarded__" in base_class.__dict__
+                ):
+                    raise AttributeError(
+                        f"{type(self).__name__} must specify either __fillable__ or __guarded__ properties, but not both."
+                    )
 
             self._booted = True
             self.observe_events(self, "booted")
@@ -526,45 +539,39 @@ def new_collection(cls, data):
         return Collection(data)
 
     @classmethod
-    def create(cls, dictionary=None, query=False, cast=False, **kwargs):
+    def create(
+        cls,
+        dictionary: Dict[str, Any] = None,
+        query: bool = False,
+        cast: bool = False,
+        **kwargs,
+    ):
         """Creates new records based off of a dictionary as well as data set on the model
         such as fillable values.
 
         Args:
             dictionary (dict, optional): [description]. Defaults to {}.
             query (bool, optional): [description]. Defaults to False.
+            cast (bool, optional): [description]. Whether or not to cast passed values.
 
         Returns:
             self: A hydrated version of a model
         """
-
-        if not dictionary:
-            dictionary = kwargs
-
-        if cls.__fillable__ != ["*"]:
-            d = {}
-            for x in cls.__fillable__:
-                if x in dictionary:
-                    if cast == True:
-                        d.update({x: cls._set_casted_value(x, dictionary[x])})
-                    else:
-                        d.update({x: dictionary[x]})
-            dictionary = d
-
-        if cls.__guarded__ != ["*"]:
-            for x in cls.__guarded__:
-                if x in dictionary:
-                    dictionary.pop(x)
-
         if query:
             return cls.builder.create(
-                dictionary, query=True, id_key=cls.__primary_key__
+                dictionary, query=True, id_key=cls.__primary_key__, cast=cast, **kwargs
             ).to_sql()
 
-        return cls.builder.create(dictionary, id_key=cls.__primary_key__)
+        return cls.builder.create(
+            dictionary, id_key=cls.__primary_key__, cast=cast, **kwargs
+        )
 
     @classmethod
-    def _set_casted_value(cls, attribute, value):
+    def cast_value(cls, attribute: str, value: Any):
+        """
+        Given an attribute name and a value, casts the value using the model's registered caster.
+        If no registered caster exists, returns the unmodified value.
+        """
         cast_method = cls.__casts__.get(attribute)
         cast_map = cls.get_cast_map(cls)
 
@@ -578,6 +585,15 @@ def _set_casted_value(cls, attribute, value):
             return cast_method(value)
         return value
 
+    @classmethod
+    def cast_values(cls, dictionary: Dict[str, Any]) -> Dict[str, Any]:
+        """
+        Runs provided dictionary through all model casters and returns the result.
+
+        Does not mutate the passed dictionary.
+        """
+        return {x: cls.cast_value(x, dictionary[x]) for x in dictionary}
+
     def fresh(self):
         return (
             self.get_builder()
@@ -974,7 +990,8 @@ def get_new_date(self, _datetime=None):
 
     def get_new_datetime_string(self, _datetime=None):
         """
-        Get the attributes that should be converted to dates.
+        Given an optional datetime value, constructs and returns a new datetime string.
+        If no datetime is specified, returns the current time.
 
         :rtype: list
         """
@@ -1104,3 +1121,35 @@ def attach_related(self, relation, related_record):
             related_record.save()
 
         return related.attach_related(self, related_record)
+
+    @classmethod
+    def filter_fillable(cls, dictionary: Dict[str, Any]) -> Dict[str, Any]:
+        """
+        Filters provided dictionary to only include fields specified in the model's __fillable__ property
+
+        Passed dictionary is not mutated.
+        """
+        if cls.__fillable__ != ["*"]:
+            dictionary = {x: dictionary[x] for x in cls.__fillable__ if x in dictionary}
+        return dictionary
+
+    @classmethod
+    def filter_mass_assignment(cls, dictionary: Dict[str, Any]) -> Dict[str, Any]:
+        """
+        Filters the provided dictionary in preparation for a mass-assignment operation
+
+        Wrapper around filter_fillable() & filter_guarded(). Passed dictionary is not mutated.
+        """
+        return cls.filter_guarded(cls.filter_fillable(dictionary))
+
+    @classmethod
+    def filter_guarded(cls, dictionary: Dict[str, Any]) -> Dict[str, Any]:
+        """
+        Filters provided dictionary to exclude fields specified in the model's __guarded__ property
+
+        Passed dictionary is not mutated.
+        """
+        if cls.__guarded__ == ["*"]:
+            # If all fields are guarded, all data should be filtered
+            return {}
+        return {f: dictionary[f] for f in dictionary if f not in cls.__guarded__}