Skip to content
Open
27 changes: 26 additions & 1 deletion app/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,32 @@
from flask import Flask
from flask_sqlalchemy import SQLAlchemy
from flask_migrate import Migrate
from dotenv import load_dotenv
import os

db = SQLAlchemy()
migrate = Migrate()
load_dotenv()

def create_app(test_config=None):
app = Flask(__name__)

return app
if not test_config:
app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False
app.config['SQLALCHEMY_DATABASE_URI'] = os.environ.get(
"SQLALCHEMY_DATABASE_URI")
else:
app.config["TESTING"] = True
app.config["SQLALCHEMY_TRACK_MODIFICATIONS"] = False
app.config["SQLALCHEMY_DATABASE_URI"] = os.environ.get(
"SQLALCHEMY_TEST_DATABASE_URI")

db.init_app(app)
migrate.init_app(app, db)

from app.models.planet import Planet

from .routes import planets_bp
app.register_blueprint(planets_bp)

return app
Empty file added app/models/__init__.py
Empty file.
44 changes: 44 additions & 0 deletions app/models/planet.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
from app import db

class Planet(db.Model):
id = db.Column(db.Integer, primary_key=True, autoincrement=True)
name = db.Column(db.String, nullable=False)
description = db.Column(db.String, nullable=False)
has_moons = db.Column(db.Boolean, nullable=False)

required_attributes = {
"name":True,
"description":True,
"has_moons":True
}
Comment on lines +9 to +13

Choose a reason for hiding this comment

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

Since the values for required_attributes aren't used, this could be a list of strings. create_from_dict uses the .keys lists currently for the comparisons, so performance shouldn't be impacted.

# Instance methods:

def self_to_dict(self):
return dict(
id=self.id,
name=self.name,
description=self.description,
has_moons=self.has_moons)

def update_self(self, data_dict):
for key in data_dict.keys():
if hasattr(self, key):
setattr(self, key, data_dict[key])
Comment on lines +24 to +26

Choose a reason for hiding this comment

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

Really nice solution for updating the planets!

else:
raise ValueError(key)

Choose a reason for hiding this comment

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

This will return the first bad key in the update request with the error, but what if there are several? The create_from_dict function below does a great job of showing the user all the keys that are wrong from the request. Its error handling code could be factored out into a helper function and used here as well, providing more consistency in how request data errors are handled.



# Class methods

@classmethod
def create_from_dict(cls, data_dict):
if data_dict.keys() == cls.required_attributes.keys():
return cls(
name=data_dict["name"],
description = data_dict["description"],
has_moons = data_dict["has_moons"]
)
else:
remaining_keys= set(data_dict.keys())-set(cls.required_attributes.keys())

Choose a reason for hiding this comment

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

Really nice use of set operations!

response=list(remaining_keys)
raise ValueError(response)
114 changes: 113 additions & 1 deletion app/routes.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,114 @@
from flask import Blueprint
from flask import Blueprint, jsonify, abort, make_response, request
from app import db
from app.models.planet import Planet


# planets = [
# Planet(1, "Arrakis", "A desert planet, source of Spice.", True),
# Planet(2, "Bela Tegeuse", "The fifth planet of Keuntsing.", False),
# Planet(3, "Tupile", "Sanctuary planet for defeated houses of the Imperium.", True),
# Planet(4, "Ix", "Supreme machine culture.", False)
# ]

planets_bp = Blueprint("Planets", __name__, url_prefix="/planets")

# Helper Functions:

Choose a reason for hiding this comment

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

Nice use of helpers!


def error_message(message, status_code):
abort(make_response(jsonify(dict(details=message)), status_code))

def success_message(message, status_code=200):
return make_response(jsonify(dict(details=message)), status_code)

def return_database_info(return_value):
return make_response(jsonify(return_value))

def get_planet_record_by_id(id):
try:
id = int(id)
except ValueError:
error_message(f"Invalid id: {id}", 400)
planet = Planet.query.get(id)
if planet:
return planet
else:
error_message(f"Planet id: {id} not found", 404)

def discover_planet_safely(data_dict):
try:
return Planet.create_from_dict(data_dict)
except ValueError as err:
error_message(f"Invalid key(s):{err}. Planet not added to database.", 400)
except KeyError as err:

Choose a reason for hiding this comment

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

Are there situations where this error gets raised? Looking at the code for creating a Planet, if there are missing required keys, we'll enter the else and raise a ValueError. Is that something you want to differentiate?

error_message(f"Missing key(s): {err}. Planet not added to database.", 400)


def update_planet_safely(planet, data_dict):
try:
planet.update_self(data_dict)
except ValueError as err:
error_message(f"Invalid key(s): {err}. Planet not updated.", 400)
except KeyError as err:
error_message(f"Missing key(s): {err}. Planet not updated.", 400)




# Route Functions:

@planets_bp.route("", methods=["GET"])
def get_all_planets():
moon_param = request.args.get("has_moons")
description_param = request.args.get("description")
if moon_param:
planets = Planet.query.filter_by(has_moons=moon_param)
Comment on lines +63 to +64

Choose a reason for hiding this comment

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

Does this work as expected for moon_param? Since this if-block is independent of the if/else below, when the request contains a has_moons query parameter and no description query parameter, it looks like the flow of execution would be that we hit line 63, enter the if and assign planets, then line 65 would be evaluated. Since there's no description query param we would enter the else and the planets variable would be reassigned to Planet.query.all().

if description_param:
planets = Planet.query.filter(Planet.description.like('%'+description_param+'%')).all()

Choose a reason for hiding this comment

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

Love the wildcard use to match partial descriptions!

else:
planets = Planet.query.all()

all_planets = [
planet.self_to_dict()
for planet in planets
]
return return_database_info(all_planets)


@planets_bp.route("/<planet_id>", methods=["GET"])
def read_one_planet(planet_id):
planet = get_planet_record_by_id(planet_id)
return return_database_info(planet.self_to_dict())


@planets_bp.route("", methods=["POST"])
def discover_planet():
request_body = request.get_json()
new_planet = discover_planet_safely(request_body)

db.session.add(new_planet)
db.session.commit()

return success_message(f"Planet {new_planet.name} successfully added to the Planets Database.", 201)


@planets_bp.route("/<planet_id>", methods=["PUT", "PATCH"])
def update_planet_by_id(planet_id):
planet = get_planet_record_by_id(planet_id)

request_body = request.get_json()
update_planet_safely(planet, request_body)

db.session.commit()

return return_database_info(planet.self_to_dict())


@planets_bp.route("/<planet_id>", methods=["DELETE"])
def delete_planet(planet_id):
planet = get_planet_record_by_id(planet_id)

db.session.delete(planet)
db.session.commit()

return success_message(f"Planet {planet.name} successfully deleted from the Planets Database.")

Empty file added app/tests/__init__.py
Empty file.
38 changes: 38 additions & 0 deletions app/tests/conftest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import pytest
from app import create_app
from app import db
from app.models.planet import Planet
from flask.signals import request_finished


@pytest.fixture
def app():
app = create_app({"TESTING": True})

@request_finished.connect_via(app)
def expire_session(sender, response, **extra):
db.session.remove()

with app.app_context():
db.create_all()
yield app

with app.app_context():
db.drop_all()


@pytest.fixture
def client(app):
return app.test_client()

@pytest.fixture
def sample_data_with_two_planets(app):
ocean_planet = Planet(name="Ocean Planet",
description="It's wet!",
has_moons=True)
jungle_planet = Planet(name="Jungle Planet",
description="Full of big cats. And trees!",
has_moons=False)

db.session.add_all([ocean_planet, jungle_planet])
db.session.commit()
73 changes: 73 additions & 0 deletions app/tests/test_routes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
def test_get_all_returns_200_and_empty_array(client):
response = client.get("/planets")
response_body = response.get_json()

assert response.status_code == 200
assert response_body == []


def test_get_one_planet(client, sample_data_with_two_planets):
# Act
response = client.get("/planets/1")
response_body = response.get_json()

# Assert
assert response.status_code == 200
assert response_body == {"id":1,
"name":"Ocean Planet",
"description":"It's wet!",
"has_moons":True}

def test_get_one_planet_without_data_returns_404(client):
# Act
response = client.get("/planets/1")
response_body = response.get_json()

# Assert
assert response.status_code == 404
assert response_body == {"details":"Planet id: 1 not found"}

def test_get_all_planets(client, sample_data_with_two_planets):
# Act
response = client.get("/planets")
response_body = response.get_json()

# Assert
assert response.status_code == 200
assert response_body == [{"id":1,
"name":"Ocean Planet",
"description":"It's wet!",
"has_moons":True},
{"id":2,
"name":"Jungle Planet",
"description":"Full of big cats. And trees!",
"has_moons":False}]

def test_create_a_planet(client):
# Act
response = client.post("/planets", json={
"name":"Ice Planet",
"description":"Right next to Canada",
"has_moons":True
})
response_body = response.get_json()

# Assert
assert response.status_code == 201
assert response_body == {"details":"Planet Ice Planet successfully added to the Planets Database."}

def test_update_planet_patches_properly(client, sample_data_with_two_planets):
pre_patch_response = client.get("/planets/1")

Choose a reason for hiding this comment

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

We generally want to start with hardcoded data rather than the result of a call to our application. While ideally there wouldn't be, if there were a bug in the GET route it could affect this test's ability to accurately test what we want/hide issues with the route we're actively testing.

pre_patch_response_body = pre_patch_response.get_json()

response = client.patch("/planets/1", json={"description":"It's wet and full of fish."})
response_body= response.get_json()

assert response.status_code == 200
assert response_body["description"] == "It's wet and full of fish."
assert response_body["name"] == pre_patch_response_body["name"]
assert response_body["has_moons"] == pre_patch_response_body["has_moons"]
assert response_body["id"] == pre_patch_response_body["id"]
assert response_body["description"] != pre_patch_response_body["description"]
Comment on lines +66 to +71

Choose a reason for hiding this comment

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

Great checking all the relevant data!



1 change: 1 addition & 0 deletions migrations/README
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Generic single-database configuration.
45 changes: 45 additions & 0 deletions migrations/alembic.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# A generic, single database configuration.

[alembic]
# template used to generate migration files
# file_template = %%(rev)s_%%(slug)s

# set to 'true' to run the environment during
# the 'revision' command, regardless of autogenerate
# revision_environment = false


# Logging configuration
[loggers]
keys = root,sqlalchemy,alembic

[handlers]
keys = console

[formatters]
keys = generic

[logger_root]
level = WARN
handlers = console
qualname =

[logger_sqlalchemy]
level = WARN
handlers =
qualname = sqlalchemy.engine

[logger_alembic]
level = INFO
handlers =
qualname = alembic

[handler_console]
class = StreamHandler
args = (sys.stderr,)
level = NOTSET
formatter = generic

[formatter_generic]
format = %(levelname)-5.5s [%(name)s] %(message)s
datefmt = %H:%M:%S
Loading