[dashboard] Refactor API using SIP-35 (#9315)

* [dashboard] Refactor API using SIP-35

* [dashboard] Fix, import

* [dashboard] more tests

* [dashboards] a misc of improvements

* [charts] Fix, DAO and tests

* [dashboards] small exceptions refactor

* [dashboards] lint

* [dashboards] Improves comments on base classes

* [dashboards] lint

superset/app.py

@@ -152,7 +152,7 @@ def init_views(self) -> None:
         )
         from superset.views.chart.api import ChartRestApi
         from superset.views.chart.views import SliceModelView, SliceAsync
-        from superset.views.dashboard.api import DashboardRestApi
+        from superset.dashboards.api import DashboardRestApi
         from superset.views.dashboard.views import (
             DashboardModelView,
             Dashboard,

superset/commands/base.py

@@ -26,7 +26,7 @@ class BaseCommand(ABC):
     def run(self):
         """
         Run executes the command. Can raise command exceptions
-        :return:
+        :raises: CommandException
         """
         pass
 
@@ -35,5 +35,6 @@ def validate(self) -> None:
         """
         Validate is normally called by run to validate data.
         Will raise exception if validation fails
+        :raises: CommandException
         """
         pass

superset/commands/exceptions.py

@@ -14,30 +14,25 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-from typing import List, Optional
+from typing import List
 
+from flask_babel import lazy_gettext as _
 from marshmallow import ValidationError
 
+from superset.exceptions import SupersetException
 
-class CommandException(Exception):
-    """ Common base class for Command exceptions. """
-
-    message = ""
 
-    def __init__(self, message: str = "", exception: Optional[Exception] = None):
-        if message:
-            self.message = message
-        self._exception = exception
-        super().__init__(self.message)
+class CommandException(SupersetException):
+    """ Common base class for Command exceptions. """
 
-    @property
-    def exception(self):
-        return self._exception
+    pass
 
 
 class CommandInvalidError(CommandException):
     """ Common base class for Command Invalid errors. """
 
+    status = 422
+
     def __init__(self, message=""):
         self._invalid_exceptions = list()
         super().__init__(self.message)
@@ -56,16 +51,27 @@ def normalized_messages(self):
 
 
 class UpdateFailedError(CommandException):
+    status = 500
     message = "Command update failed"
 
 
 class CreateFailedError(CommandException):
+    status = 500
     message = "Command create failed"
 
 
 class DeleteFailedError(CommandException):
+    status = 500
     message = "Command delete failed"
 
 
 class ForbiddenError(CommandException):
+    status = 500
     message = "Action is forbidden"
+
+
+class OwnersNotFoundValidationError(ValidationError):
+    status = 422
+
+    def __init__(self):
+        super().__init__(_("Owners are invalid"), field_names=["owners"])

superset/datasets/commands/base.py → superset/commands/utils.py

@@ -18,15 +18,14 @@
 
 from flask_appbuilder.security.sqla.models import User
 
-from superset.datasets.commands.exceptions import OwnersNotFoundValidationError
-from superset.datasets.dao import DatasetDAO
+from superset.commands.exceptions import OwnersNotFoundValidationError
+from superset.extensions import security_manager
 
 
 def populate_owners(user: User, owners_ids: Optional[List[int]] = None) -> List[User]:
     """
     Helper function for commands, will fetch all users from owners id's
     Can raise ValidationError
-
     :param user: The current user
     :param owners_ids: A List of owners by id's
     """
@@ -36,7 +35,7 @@ def populate_owners(user: User, owners_ids: Optional[List[int]] = None) -> List[
     if user.id not in owners_ids:
         owners.append(user)
     for owner_id in owners_ids:
-        owner = DatasetDAO.get_owner_by_id(owner_id)
+        owner = security_manager.get_user_by_id(owner_id)
         if not owner:
             raise OwnersNotFoundValidationError()
         owners.append(owner)

superset/dao/__init__.py

@@ -0,0 +1,16 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.

superset/dao/base.py

@@ -0,0 +1,111 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from typing import Dict, Optional
+
+from flask_appbuilder.models.filters import BaseFilter
+from flask_appbuilder.models.sqla import Model
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+from sqlalchemy.exc import SQLAlchemyError
+
+from superset.dao.exceptions import (
+    DAOConfigError,
+    DAOCreateFailedError,
+    DAODeleteFailedError,
+    DAOUpdateFailedError,
+)
+from superset.extensions import db
+
+
+class BaseDAO:
+    """
+    Base DAO, implement base CRUD sqlalchemy operations
+    """
+
+    model_cls: Optional[Model] = None
+    """
+    Child classes need to state the Model class so they don't need to implement basic
+    create, update and delete methods
+    """  # pylint: disable=pointless-string-statement
+    base_filter: Optional[BaseFilter] = None
+    """
+    Child classes can register base filtering to be aplied to all filter methods
+    """  # pylint: disable=pointless-string-statement
+
+    @classmethod
+    def find_by_id(cls, model_id: int) -> Model:
+        """
+        Retrives a model by id, if defined applies `base_filter`
+        """
+        query = db.session.query(cls.model_cls)
+        if cls.base_filter:
+            data_model = SQLAInterface(cls.model_cls, db.session)
+            query = cls.base_filter(  # pylint: disable=not-callable
+                "id", data_model
+            ).apply(query, None)
+        return query.filter_by(id=model_id).one_or_none()
+
+    @classmethod
+    def create(cls, properties: Dict, commit=True) -> Optional[Model]:
+        """
+        Generic for creating models
+        :raises: DAOCreateFailedError
+        """
+        if cls.model_cls is None:
+            raise DAOConfigError()
+        model = cls.model_cls()  # pylint: disable=not-callable
+        for key, value in properties.items():
+            setattr(model, key, value)
+        try:
+            db.session.add(model)
+            if commit:
+                db.session.commit()
+        except SQLAlchemyError as e:  # pragma: no cover
+            db.session.rollback()
+            raise DAOCreateFailedError(exception=e)
+        return model
+
+    @classmethod
+    def update(cls, model: Model, properties: Dict, commit=True) -> Optional[Model]:
+        """
+        Generic update a model
+        :raises: DAOCreateFailedError
+        """
+        for key, value in properties.items():
+            setattr(model, key, value)
+        try:
+            db.session.merge(model)
+            if commit:
+                db.session.commit()
+        except SQLAlchemyError as e:  # pragma: no cover
+            db.session.rollback()
+            raise DAOUpdateFailedError(exception=e)
+        return model
+
+    @classmethod
+    def delete(cls, model: Model, commit=True):
+        """
+        Generic delete a model
+        :raises: DAOCreateFailedError
+        """
+        try:
+            db.session.delete(model)
+            if commit:
+                db.session.commit()
+        except SQLAlchemyError as e:  # pragma: no cover
+            db.session.rollback()
+            raise DAODeleteFailedError(exception=e)
+        return model

superset/dao/exceptions.py

@@ -0,0 +1,57 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from superset.exceptions import SupersetException
+
+
+class DAOException(SupersetException):
+    """
+    Base DAO exception class
+    """
+
+    pass
+
+
+class DAOCreateFailedError(DAOException):
+    """
+    DAO Create failed
+    """
+
+    message = "Create failed"
+
+
+class DAOUpdateFailedError(DAOException):
+    """
+    DAO Update failed
+    """
+
+    message = "Updated failed"
+
+
+class DAODeleteFailedError(DAOException):
+    """
+    DAO Delete failed
+    """
+
+    message = "Delete failed"
+
+
+class DAOConfigError(DAOException):
+    """
+    DAO is miss configured
+    """
+
+    message = "DAO is not configured correctly missing model definition"

superset/dashboards/__init__.py

@@ -0,0 +1,16 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.