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

Add check for conflicting events before creation #129

Merged
merged 2 commits into from
Apr 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions backend/src/flotilla/api/events_api.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from datetime import timedelta
from http import HTTPStatus
from logging import getLogger
from typing import List
Expand All @@ -12,6 +13,7 @@
create_event,
read_event_by_id,
read_events,
read_events_by_robot_id_and_time_span,
remove_event,
)
from flotilla.database.db import get_db
Expand All @@ -21,8 +23,7 @@

router = APIRouter()

NOT_FOUND_DESCRIPTION = "Not Found - No event with given id"
INTERNAL_SERVER_ERROR_DESCRIPTION = "Internal Server Error"
DEFAULT_EVENT_DURATION = timedelta(hours=1)


@router.get(
Expand Down Expand Up @@ -62,11 +63,24 @@ async def post_event(
) -> Event:
"""Add a new event to the robot schedule"""
try:
end_time = event_request.start_time + DEFAULT_EVENT_DURATION
Copy link
Contributor

@GodVenn GodVenn Apr 12, 2022

Choose a reason for hiding this comment

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

Will we always use the default duration? Maybe the user should have an event_duration optional field to this post endpoint so they can define it themselves?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently a default value is enough in my opinion. I don't think the user should be able to set the event_duration, but rather be based on the number of tags and/or based on data from previous missions with the same id. Can make a card for setting the event_duration based on number of tags.

overlapping_events: List[Event] = read_events_by_robot_id_and_time_span(
db=db,
robot_id=event_request.robot_id,
start_time=event_request.start_time,
end_time=end_time,
)
if overlapping_events:
raise HTTPException(
status_code=HTTPStatus.CONFLICT.value,
detail=f"Conflict with already existing event in the same time period. Events with id: {','.join(str(event.id) for event in overlapping_events)}",
Copy link
Contributor

Choose a reason for hiding this comment

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

Will all users be authorized to see all events for a given robot?
If not, this might be giving out too much information, maybe the first statement is enough?
I'm not sure, open for discussion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would say that if the user is authorized to post events (which I believe should have a "higher" access level than just read events) they will be allowed to see all events.

)
event_id: int = create_event(
db,
event_request.robot_id,
event_request.mission_id,
event_request.start_time,
DEFAULT_EVENT_DURATION,
)
db_event: EventDBModel = read_event_by_id(db, event_id)
event: Event = db_event.get_api_event()
Expand Down
55 changes: 38 additions & 17 deletions backend/src/flotilla/database/crud.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import datetime
from datetime import datetime, timedelta
from http import HTTPStatus
from typing import List, Optional

from fastapi import HTTPException
from sqlalchemy import and_
from sqlalchemy.orm import Session

from flotilla.database.models import (
Expand Down Expand Up @@ -64,38 +65,39 @@ def read_event_by_id(db: Session, event_id: int) -> EventDBModel:
return event


def create_report(
def read_events_by_robot_id_and_time_span(
db: Session,
robot_id: int,
isar_mission_id: str,
echo_mission_id: int,
report_status: ReportStatus,
) -> int:
report: ReportDBModel = ReportDBModel(
robot_id=robot_id,
isar_mission_id=isar_mission_id,
echo_mission_id=echo_mission_id,
log="",
status=report_status,
start_time: datetime,
end_time: datetime,
) -> List[EventDBModel]:
return (
db.query(EventDBModel)
.filter(EventDBModel.robot_id == robot_id)
.filter(
and_(
EventDBModel.start_time < end_time,
EventDBModel.end_time > start_time,
)
)
.all()
)
db.add(report)
db.commit()
return report.id


def create_event(
db: Session,
robot_id: int,
echo_mission_id: int,
start_time: datetime.datetime,
start_time: datetime,
estimated_duration: timedelta,
) -> int:

event: EventDBModel = EventDBModel(
robot_id=robot_id,
echo_mission_id=echo_mission_id,
report_id=None,
start_time=start_time,
estimated_duration=datetime.timedelta(hours=1),
estimated_duration=estimated_duration,
)
db.add(event)
db.commit()
Expand All @@ -107,3 +109,22 @@ def remove_event(db: Session, event_id: int) -> None:
db.delete(event)
db.commit()
return


def create_report(
db: Session,
robot_id: int,
isar_mission_id: str,
echo_mission_id: int,
report_status: ReportStatus,
) -> int:
report: ReportDBModel = ReportDBModel(
robot_id=robot_id,
isar_mission_id=isar_mission_id,
echo_mission_id=echo_mission_id,
log="",
status=report_status,
)
db.add(report)
db.commit()
return report.id
41 changes: 28 additions & 13 deletions backend/src/flotilla/database/models.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,20 @@
import datetime
import enum
from datetime import datetime, timezone

from flotilla_openapi.models.event import Event
from flotilla_openapi.models.report import Report
from flotilla_openapi.models.report_entry import ReportEntry
from flotilla_openapi.models.robot import Robot
from sqlalchemy import Column, DateTime, Enum, ForeignKey, Integer, Interval, String
from sqlalchemy import (
Boolean,
Column,
DateTime,
Enum,
ForeignKey,
Integer,
Interval,
String,
)
from sqlalchemy.orm import backref, relationship

from flotilla.database.db import Base
Expand Down Expand Up @@ -49,6 +58,7 @@ class RobotDBModel(Base):
status = Column(Enum(RobotStatus))
host = Column(String)
port = Column(Integer)
enabled = Column(Boolean, default=True)
telemetry_topics = relationship("TopicDBModel", backref=backref("robot"))
streams = relationship("VideoStreamDBModel", backref=backref("robot"))
capabilities = relationship("CapabilityDBModel", backref=backref("robot"))
Expand All @@ -61,6 +71,10 @@ def get_api_robot(self) -> Robot:
model=self.model,
status=self.status.value,
capabilities=[cap.capability.value for cap in self.capabilities],
serial_number=self.serial_number,
host=self.host,
port=self.port,
enabled=self.enabled,
)


Expand All @@ -72,16 +86,14 @@ class ReportDBModel(Base):
echo_mission_id = Column(Integer)
log = Column(String)
status = Column(Enum(ReportStatus))
start_time = Column(
DateTime(timezone=True), default=datetime.datetime.now(tz=datetime.timezone.utc)
)
start_time = Column(DateTime(timezone=True), default=datetime.now(tz=timezone.utc))
entries = relationship("ReportEntryDBModel", backref=backref("report"))

def get_api_report(self) -> Report:
return Report(
id=self.id,
start_time=self.start_time,
end_time=datetime.datetime.now(tz=datetime.timezone.utc),
end_time=datetime.now(tz=timezone.utc),
robot_id=self.robot_id,
mission_id=self.echo_mission_id,
status=self.status.value,
Expand Down Expand Up @@ -120,16 +132,21 @@ class CapabilityDBModel(Base):
capability = Column(Enum(InspectionType))


def event_end_time_default(context):
start_time = context.get_current_parameters()["start_time"]
duration = context.get_current_parameters()["estimated_duration"]
return start_time + duration


class EventDBModel(Base):
__tablename__ = "event"
id = Column(Integer, primary_key=True)
robot_id = Column(Integer, ForeignKey("robot.id"))
echo_mission_id = Column(Integer)
report_id = Column(Integer, ForeignKey("report.id"))
start_time = Column(
DateTime(timezone=True), default=datetime.datetime.now(tz=datetime.timezone.utc)
)
start_time = Column(DateTime(timezone=True), default=datetime.now(tz=timezone.utc))
estimated_duration = Column(Interval)
end_time = Column(DateTime(timezone=True), default=event_end_time_default)
# TODO: robot_id and report_id.robot_id can now point at different robots.
# Should there be a constraint forcing an event to point at only one robot?

Expand All @@ -139,7 +156,7 @@ def get_api_event(self) -> Event:
robot_id=self.robot_id,
mission_id=self.echo_mission_id,
start_time=self.start_time,
end_time=datetime.datetime.now(),
end_time=self.end_time,
)


Expand All @@ -150,9 +167,7 @@ class ReportEntryDBModel(Base):
tag_id = Column(String)
status = Column(Enum(ReportEntryStatus))
inspection_type = Column(Enum(InspectionType))
time = Column(
DateTime(timezone=True), default=datetime.datetime.now(tz=datetime.timezone.utc)
)
time = Column(DateTime(timezone=True), default=datetime.now(tz=timezone.utc))
file_location = Column(String)

def get_report_entry(self) -> ReportEntry:
Expand Down
17 changes: 14 additions & 3 deletions backend/tests/test_api/test_events_api.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import json
from datetime import datetime
from datetime import datetime, timedelta
from http import HTTPStatus

import pytest
Expand All @@ -8,7 +8,6 @@
from flotilla_openapi.models.event_request import EventRequest

from flotilla.database.crud import read_event_by_id
from flotilla.database.models import Event


def test_get_events(test_app: FastAPI):
Expand Down Expand Up @@ -46,9 +45,21 @@ def test_get_event(
"event_request, expected_status_code",
[
(
EventRequest(robot_id=1, mission_id=234, start_time=datetime.now()),
EventRequest(
robot_id=1,
mission_id=234,
start_time=datetime.utcnow() + timedelta(days=10),
),
HTTPStatus.CREATED.value,
),
(
EventRequest(
robot_id=1,
mission_id=234,
start_time=datetime.utcnow() + timedelta(hours=0.5),
),
HTTPStatus.CONFLICT.value,
),
],
)
def test_post_event(
Expand Down
37 changes: 35 additions & 2 deletions backend/tests/test_crud.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import datetime
from datetime import datetime, timedelta, timezone
from http import HTTPStatus
from typing import List

Expand All @@ -10,6 +10,7 @@
create_report,
read_event_by_id,
read_events,
read_events_by_robot_id_and_time_span,
read_report_by_id,
read_reports,
read_robot_by_id,
Expand Down Expand Up @@ -89,6 +90,37 @@ def test_read_event_by_id(event_id: int, session):
assert event


@pytest.mark.parametrize(
"start_time, duration, expected_len",
[
(datetime.now(tz=timezone.utc), timedelta(hours=1.5), 1),
(
datetime.now(tz=timezone.utc) - timedelta(days=1),
timedelta(hours=1),
0,
),
(
datetime.now(tz=timezone.utc) - timedelta(hours=0.5),
timedelta(hours=1),
1,
),
(
datetime.now(tz=timezone.utc) + timedelta(hours=1.5),
timedelta(hours=1),
0,
),
],
)
def test_read_event_by_robot_id_and_time_span(
start_time: datetime, duration: timedelta, expected_len: int, session
):
end_time = start_time + duration
events: List[EventDBModel] = read_events_by_robot_id_and_time_span(
db=session, robot_id=1, start_time=start_time, end_time=end_time
)
assert len(events) == expected_len


@pytest.mark.parametrize(
"event_id",
invalid_ids,
Expand Down Expand Up @@ -120,13 +152,14 @@ def test_create_report(session):
def test_create_event(session):
robot_id: int = 1
echo_mission_id: int = 12345
start_time = datetime.datetime.now()
start_time = datetime.now()
pre_count: int = len(read_events(session))
event_id: int = create_event(
db=session,
robot_id=robot_id,
echo_mission_id=echo_mission_id,
start_time=start_time,
estimated_duration=timedelta(hours=1),
)
post_count: int = len(read_events(session))
assert pre_count + 1 == post_count
Expand Down