Add a unit test for the Rules engine (#52)

* Add a unit test for the Rules engine
* Fix path for GHA
* Rules Engine linter

Author: lavjain

.github/workflows/unit_test_linter_rules_engine.yaml

@@ -11,8 +11,8 @@ on:
       - main
     paths:
       - "components/common/**"
-      - "components/rules_engine"
-      - ".github/workflows/unit_test_rules_engine.yaml"
+      - "components/rules_engine/**"
+      - ".github/workflows/unit_test_linter_rules_engine.yaml"
       - ".pylintrc"
       - "!components/rules_engine/**.md"
   workflow_dispatch:
@@ -26,9 +26,7 @@ jobs:
       fail-fast: false
       matrix:
         python-version: [3.9]
-        target-folder: [
-            components/rules_engine,
-          ]
+        target-folder: [components/rules_engine]
 # copier:raw 
     steps:
       - uses: actions/checkout@v3
@@ -62,7 +60,7 @@ jobs:
           BASE_DIR=$(pwd)
           python -m pip install --upgrade pip
           python -m pip install pytest pytest-custom_exit_code pytest-cov pylint pytest-mock mock
-          if [ -f $BASE_DIR/common/requirements.txt ]; then pip install -r $BASE_DIR/common/requirements.txt; fi
+          if [ -f $BASE_DIR/components/common/requirements.txt ]; then pip install -r $BASE_DIR/components/common/requirements.txt; fi
           cd ${{ matrix.target-folder }}
           if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
           if [ -f requirements-test.txt ]; then pip install -r requirements-test.txt; fi
@@ -71,15 +69,15 @@ jobs:
         run: |
           BASE_DIR=$(pwd)
           cd ${{ matrix.target-folder }}/src
-          PYTEST_ADDOPTS="--cache-clear --cov . " PYTHONPATH=$BASE_DIR/common/src python -m pytest
+          PYTEST_ADDOPTS="--cache-clear --cov . " PYTHONPATH=$BASE_DIR/components/common/src python -m pytest
 
   linter:
     runs-on: ubuntu-latest
     strategy:
       fail-fast: false
       matrix:
         python-version: [3.9]
-        target-folder: [common]
+        target-folder: [components/rules_engine]
     steps:
       - uses: actions/checkout@v3
       - name: Set up Python ${{ matrix.python-version }}

components/rules_engine/src/config.py

@@ -12,11 +12,14 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+"""
+  Rules Service config file
+"""
 
 import os
 
 PORT = os.environ["PORT"] if os.environ.get("PORT") is not None else 80
-PROJECT_ID = os.environ.get("PROJECT_ID") or \
-    os.environ.get("GOOGLE_CLOUD_PROJECT")
+PROJECT_ID = os.environ.get("PROJECT_ID",
+                            os.environ.get("GOOGLE_CLOUD_PROJECT"))
 DATABASE_PREFIX = os.getenv("DATABASE_PREFIX", "")
 SERVICE_NAME = os.getenv("SERVICE_NAME")

components/rules_engine/src/main.py

@@ -12,13 +12,12 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# Rules RESTful Microservice
+"""Rules RESTful Microservice"""
 
 import uvicorn
 import config
 from fastapi import FastAPI
 from fastapi.responses import HTMLResponse
-from google.cloud.firestore import Client
 from common.utils.http_exceptions import add_exception_handlers
 
 from routes import rules, rulesets

components/rules_engine/src/models/rule.py

@@ -11,31 +11,20 @@
 # 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.
-# Data model for Rule. See https://octabyte.io/FireO/ for details.
 
+"""Data model for Rule. See https://octabyte.io/FireO/ for details."""
 
-import os
-
-from fireo.models import Model
-from fireo.fields import IDField, TextField, BooleanField, DateTime, ListField
+from fireo.fields import IDField, TextField, DateTime, ListField
 from fireo.queries.errors import ReferenceDocNotExist
-from datetime import datetime
 from common.models.base_model import BaseModel
 
-# GCP project_id from system's environment variable.
-PROJECT_ID = os.environ.get("PROJECT_ID", "")
-
-# Database prefix for integration and e2e test purpose.
-DATABASE_PREFIX = os.getenv("DATABASE_PREFIX", "")
-
-
 # Firebase data model in "rules" collection.
 class Rule(BaseModel):
   """Rule ORM class"""
 
   class Meta:
     ignore_none_field = False
-    collection_name = DATABASE_PREFIX + "rules"
+    collection_name = BaseModel.DATABASE_PREFIX + "rules"
 
   id = IDField()
   title = TextField()
@@ -46,9 +35,9 @@ class Meta:
   modified_at = DateTime()
 
   @classmethod
-  def find_by_doc_id(cls, id):
+  def find_by_doc_id(cls, doc_id):
     try:
-      rule = Rule.collection.get(f"rules/{id}")
+      rule = Rule.collection.get(f"rules/{doc_id}")
     except ReferenceDocNotExist:
       return None
 

components/rules_engine/src/models/rule_test.py

@@ -0,0 +1,43 @@
+# Copyright 2023 Google LLC
+#
+# Licensed 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.
+
+"""
+Unit test for Rule.py
+"""
+# disabling these rules, as they cause issues with pytest fixtures
+# pylint: disable=unused-import,unused-argument,redefined-outer-name
+from models.rule import Rule
+from common.testing.firestore_emulator import firestore_emulator, clean_firestore
+from datetime import datetime
+
+
+TEST_RULE = {
+  "name": "Rule name",
+  "title": "Rule Title",
+  "type": "Rule Type",
+  "description": "Rule Description",
+  "fields": ["Foo", "Bar"],
+  "sql_query": "select employee_name from employee",
+  "created_at": datetime(year=2022, month=10, day=14),
+  "modified_at": datetime(year=2022, month=12, day=25)
+}
+
+def test_rule(clean_firestore):
+  """Test for creating, loading and deleting of a new rule"""
+  new_rule = Rule.from_dict(TEST_RULE)
+  new_rule.save()
+  rule = Rule.find_by_id(new_rule.id)
+  assert rule.title == TEST_RULE["title"]
+  assert rule.description == TEST_RULE["description"]
+  Rule.soft_delete_by_id(new_rule.id)

components/rules_engine/src/models/ruleset.py

@@ -11,32 +11,19 @@
 # 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.
-# Data model for Rule. See https://octabyte.io/FireO/ for details.
 
+"""Firebase Data model for RuleSet"""
 
-import os
-
-from fireo.models import Model
-from fireo.fields import IDField, TextField, BooleanField, DateTime, ListField
+from fireo.fields import IDField, TextField, DateTime, ListField
 from fireo.queries.errors import ReferenceDocNotExist
-from datetime import datetime
 from common.models.base_model import BaseModel
-from rule import Rule
-
-# GCP project_id from system's environment variable.
-PROJECT_ID = os.environ.get("PROJECT_ID", "")
-
-# Database prefix for integration and e2e test purpose.
-DATABASE_PREFIX = os.getenv("DATABASE_PREFIX", "")
-
 
-# Firebase data model in "rules" collection.
 class RuleSet(BaseModel):
   """RuleSet ORM class"""
 
   class Meta:
     ignore_none_field = False
-    collection_name = DATABASE_PREFIX + "rulesets"
+    collection_name = BaseModel.DATABASE_PREFIX + "rulesets"
 
   id = IDField()
   name = TextField()
@@ -47,9 +34,9 @@ class Meta:
   modified_at = DateTime()
 
   @classmethod
-  def find_by_doc_id(cls, id):
+  def find_by_doc_id(cls, doc_id):
     try:
-      ruleset = RuleSet.collection.get(f"rulesets/{id}")
+      ruleset = RuleSet.collection.get(f"rulesets/{doc_id}")
     except ReferenceDocNotExist:
       return None
 

components/rules_engine/src/routes/rules.py

@@ -12,25 +12,24 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+""" Rule endpoints """
+
 from fastapi import APIRouter, HTTPException
 from models.rule import Rule
 from schemas.rule import RuleSchema
 import datetime
 
-# disabling for linting to pass
-# pylint: disable = broad-except
-
 router = APIRouter(prefix="/rule", tags=["rule"])
 
 SUCCESS_RESPONSE = {"status": "Success"}
 
 
-@router.get("/{id}", response_model=RuleSchema)
-async def get(id: str):
+@router.get("/{rule_id}", response_model=RuleSchema)
+async def get(rule_id: str):
   """Get a Rule
 
   Args:
-    id (str): unique id of the rule
+    rule_id (str): unique id of the rule
 
   Raises:
     HTTPException: 404 Not Found if rule doesn't exist for the given rule id
@@ -39,10 +38,10 @@ async def get(id: str):
   Returns:
     [rule]: rule object for the provided rule id
   """
-  rule = Rule.find_by_doc_id(id)
+  rule = Rule.find_by_doc_id(rule_id)
 
   if rule is None:
-    raise HTTPException(status_code=404, detail=f"Rule {id} not found.")
+    raise HTTPException(status_code=404, detail=f"Rule {rule_id} not found.")
   return rule
 
 
@@ -59,12 +58,12 @@ async def post(data: RuleSchema):
   Returns:
     [JSON]: rule ID of the rule if the rule is successfully created
   """
-  id = data.id
-  existing_rule = Rule.find_by_doc_id(id)
+  rule_id = data.id
+  existing_rule = Rule.find_by_doc_id(rule_id)
 
   if existing_rule:
     raise HTTPException(status_code=409,
-                        detail=f"Rule {id} already exists.")
+                        detail=f"Rule {rule_id} already exists.")
 
   new_rule = Rule()
   new_rule = new_rule.from_dict({**data.dict()})
@@ -88,26 +87,26 @@ async def put(data: RuleSchema):
   Returns:
     [JSON]: {'status': 'Succeed'} if the rule is updated
   """
-  id = data.id
-  rule = Rule.find_by_doc_id(id)
+  rule_id = data.id
+  rule = Rule.find_by_doc_id(rule_id)
 
   if rule:
     rule = rule.from_dict({**data.dict()})
     rule.modified_at = datetime.datetime.utcnow()
     rule.save()
 
   else:
-    raise HTTPException(status_code=404, detail=f"Rule {id} not found.")
+    raise HTTPException(status_code=404, detail=f"Rule {rule_id} not found.")
 
   return SUCCESS_RESPONSE
 
 
-@router.delete("/{id}")
-async def delete(id: str):
+@router.delete("/{rule_id}")
+async def delete(rule_id: str):
   """Delete a Rule
 
   Args:
-    id (str): unique id of the rule
+    rule_id (str): unique id of the rule
 
   Raises:
     HTTPException: 500 Internal Server Error if something fails
@@ -116,9 +115,9 @@ async def delete(id: str):
     [JSON]: {'status': 'Succeed'} if the rule is deleted
   """
 
-  rule = Rule.find_by_doc_id(id)
+  rule = Rule.find_by_doc_id(rule_id)
   if rule is None:
-    raise HTTPException(status_code=404, detail=f"Rule {id} not found.")
+    raise HTTPException(status_code=404, detail=f"Rule {rule_id} not found.")
 
   Rule.collection.delete(rule.key)