From 45fb75660f2f3dc9ff883bc64f89debc659eb32e Mon Sep 17 00:00:00 2001 From: Jan David Date: Fri, 11 Mar 2022 13:06:54 +0100 Subject: [PATCH 01/13] Add specification for endpoint to update flight plans --- protobufs/atc/v1/airplane.proto | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/protobufs/atc/v1/airplane.proto b/protobufs/atc/v1/airplane.proto index 379624a..9bb5d8c 100644 --- a/protobufs/atc/v1/airplane.proto +++ b/protobufs/atc/v1/airplane.proto @@ -37,6 +37,14 @@ message GetAirplaneResponse { Airplane airplane = 1; } +message UpdateFlightPlanRequest { + string id = 1; + repeated Node flight_plan = 2; +} + +message UpdateFlightPlanResponse {} + service AirplaneService { rpc GetAirplane(GetAirplaneRequest) returns (GetAirplaneResponse); + rpc UpdateFlightPlan(UpdateFlightPlanRequest) returns (UpdateFlightPlanResponse); } From 95c630fe3b519a7865fa55aeeae0a0e5afbdeaf6 Mon Sep 17 00:00:00 2001 From: Jan David Date: Fri, 11 Mar 2022 14:42:23 +0100 Subject: [PATCH 02/13] Add validation errors to airplane ProtoBuf --- protobufs/atc/v1/airplane.proto | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/protobufs/atc/v1/airplane.proto b/protobufs/atc/v1/airplane.proto index 9bb5d8c..8a8d943 100644 --- a/protobufs/atc/v1/airplane.proto +++ b/protobufs/atc/v1/airplane.proto @@ -42,7 +42,15 @@ message UpdateFlightPlanRequest { repeated Node flight_plan = 2; } -message UpdateFlightPlanResponse {} +message UpdateFlightPlanResponse { + enum Error { + ERROR_UNKNOWN = 0; + ERROR_NODE_OUT_OF_BOUNDS = 1; + ERROR_NOT_IN_LOGICAL_ORDER = 2; + ERROR_HAS_SHARP_TURNS = 3; + } + repeated Error validation_errors = 1; +} service AirplaneService { rpc GetAirplane(GetAirplaneRequest) returns (GetAirplaneResponse); From 1156ff3bbacbd385850c0519b5a76b3d42e2ba9d Mon Sep 17 00:00:00 2001 From: Jan David Date: Fri, 11 Mar 2022 14:43:39 +0100 Subject: [PATCH 03/13] Mock update flight plan endpoint --- src/atc-game/src/api/airplane.rs | 52 +++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/src/atc-game/src/api/airplane.rs b/src/atc-game/src/api/airplane.rs index 43373d4..e3d440c 100644 --- a/src/atc-game/src/api/airplane.rs +++ b/src/atc-game/src/api/airplane.rs @@ -2,9 +2,13 @@ use std::sync::Arc; use tonic::{Request, Response, Status}; -use atc::v1::{GetAirplaneRequest, GetAirplaneResponse}; +use atc::v1::{ + GetAirplaneRequest, GetAirplaneResponse, UpdateFlightPlanRequest, UpdateFlightPlanResponse, +}; use crate::command::CommandSender; +use crate::components::{FlightPlan, ValidationError}; +use crate::map::Tile; use crate::store::Store; #[derive(Clone, Debug)] @@ -38,4 +42,50 @@ impl atc::v1::airplane_service_server::AirplaneService for AirplaneService { ))) } } + + async fn update_flight_plan( + &self, + request: Request, + ) -> Result, Status> { + let request = request.into_inner(); + let id = request.id; + + if let Some(_airplane) = self.store.get(&id) { + let new_flight_plan = request.flight_plan; + let tiles = new_flight_plan + .iter() + .map(|node| Tile::new(node.x, node.y)) + .collect(); + + let errors = match FlightPlan::new(tiles) { + Ok(_flight_plan) => { + // TODO: Queue a command on the command bus + Vec::new() + } + Err(errors) => errors + .iter() + .map(|error| match error { + ValidationError::HasSharpTurns => { + atc::v1::update_flight_plan_response::Error::HasSharpTurns.into() + } + ValidationError::NodeOutOfBounds => { + atc::v1::update_flight_plan_response::Error::NodeOutOfBounds.into() + } + ValidationError::NotInLogicalOrder => { + atc::v1::update_flight_plan_response::Error::NotInLogicalOrder.into() + } + }) + .collect(), + }; + + // TODO: Create different responses for successful and failed updates + Ok(Response::new(UpdateFlightPlanResponse { + validation_errors: errors, + })) + } else { + Err(Status::not_found(&format!( + "No airplane with id {id} was found" + ))) + } + } } From 82b7a9959ad71c190010cf93176e8957b3d44f41 Mon Sep 17 00:00:00 2001 From: Jan David Date: Mon, 14 Mar 2022 09:54:13 +0100 Subject: [PATCH 04/13] Rename default state for enum to follow conventions --- protobufs/atc/v1/airplane.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protobufs/atc/v1/airplane.proto b/protobufs/atc/v1/airplane.proto index 8a8d943..12f3447 100644 --- a/protobufs/atc/v1/airplane.proto +++ b/protobufs/atc/v1/airplane.proto @@ -44,7 +44,7 @@ message UpdateFlightPlanRequest { message UpdateFlightPlanResponse { enum Error { - ERROR_UNKNOWN = 0; + ERROR_UNSPECIFIED = 0; ERROR_NODE_OUT_OF_BOUNDS = 1; ERROR_NOT_IN_LOGICAL_ORDER = 2; ERROR_HAS_SHARP_TURNS = 3; From 26a2c3ee25e3419f8e4ae09f8014f188dffd6d52 Mon Sep 17 00:00:00 2001 From: Jan David Date: Mon, 14 Mar 2022 13:29:53 +0100 Subject: [PATCH 05/13] Queue update flight plan command --- src/atc-game/src/api/airplane.rs | 15 ++++++++++----- src/atc-game/src/components/airplane_id.rs | 4 ++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/atc-game/src/api/airplane.rs b/src/atc-game/src/api/airplane.rs index e3d440c..0f44688 100644 --- a/src/atc-game/src/api/airplane.rs +++ b/src/atc-game/src/api/airplane.rs @@ -7,13 +7,13 @@ use atc::v1::{ }; use crate::command::CommandSender; -use crate::components::{FlightPlan, ValidationError}; +use crate::components::{AirplaneId, FlightPlan, ValidationError}; use crate::map::Tile; use crate::store::Store; +use crate::Command; #[derive(Clone, Debug)] pub struct AirplaneService { - #[allow(dead_code)] // TODO: Remove when updating a flight plan command_bus: CommandSender, store: Arc, } @@ -58,9 +58,14 @@ impl atc::v1::airplane_service_server::AirplaneService for AirplaneService { .collect(); let errors = match FlightPlan::new(tiles) { - Ok(_flight_plan) => { - // TODO: Queue a command on the command bus - Vec::new() + Ok(flight_plan) => { + match self + .command_bus + .send(Command::UpdateFlightPlan(AirplaneId::new(id), flight_plan)) + { + Ok(_) => Vec::new(), + Err(_) => return Err(Status::internal("failed to queue command")), + } } Err(errors) => errors .iter() diff --git a/src/atc-game/src/components/airplane_id.rs b/src/atc-game/src/components/airplane_id.rs index 7d637c2..f00da8a 100644 --- a/src/atc-game/src/components/airplane_id.rs +++ b/src/atc-game/src/components/airplane_id.rs @@ -6,6 +6,10 @@ use crate::api::IntoApi; pub struct AirplaneId(String); impl AirplaneId { + pub fn new(id: String) -> Self { + Self(id) + } + #[allow(dead_code)] // TODO: Remove when the id is read pub fn get(&self) -> &str { &self.0 From c8a6b0b6f0a67a059a002d7865a0f8c70f09fa0c Mon Sep 17 00:00:00 2001 From: Jan David Date: Mon, 14 Mar 2022 14:20:48 +0100 Subject: [PATCH 06/13] Refactor update flight plan API interface --- protobufs/atc/v1/airplane.proto | 1 + src/atc-game/src/api/airplane.rs | 82 ++++++++++++---------- src/atc-game/src/components/flight_plan.rs | 13 +++- 3 files changed, 57 insertions(+), 39 deletions(-) diff --git a/protobufs/atc/v1/airplane.proto b/protobufs/atc/v1/airplane.proto index 12f3447..8151c8e 100644 --- a/protobufs/atc/v1/airplane.proto +++ b/protobufs/atc/v1/airplane.proto @@ -48,6 +48,7 @@ message UpdateFlightPlanResponse { ERROR_NODE_OUT_OF_BOUNDS = 1; ERROR_NOT_IN_LOGICAL_ORDER = 2; ERROR_HAS_SHARP_TURNS = 3; + ERROR_INVALID_FIRST_NODE = 4; } repeated Error validation_errors = 1; } diff --git a/src/atc-game/src/api/airplane.rs b/src/atc-game/src/api/airplane.rs index 0f44688..5cbd9a1 100644 --- a/src/atc-game/src/api/airplane.rs +++ b/src/atc-game/src/api/airplane.rs @@ -8,7 +8,6 @@ use atc::v1::{ use crate::command::CommandSender; use crate::components::{AirplaneId, FlightPlan, ValidationError}; -use crate::map::Tile; use crate::store::Store; use crate::Command; @@ -50,47 +49,56 @@ impl atc::v1::airplane_service_server::AirplaneService for AirplaneService { let request = request.into_inner(); let id = request.id; - if let Some(_airplane) = self.store.get(&id) { - let new_flight_plan = request.flight_plan; - let tiles = new_flight_plan - .iter() - .map(|node| Tile::new(node.x, node.y)) - .collect(); + let airplane = match self.store.get(&id) { + Some(airplane) => airplane, + None => { + return Err(Status::not_found(&format!( + "No airplane with id {id} was found" + ))); + } + }; - let errors = match FlightPlan::new(tiles) { - Ok(flight_plan) => { - match self - .command_bus - .send(Command::UpdateFlightPlan(AirplaneId::new(id), flight_plan)) - { - Ok(_) => Vec::new(), - Err(_) => return Err(Status::internal("failed to queue command")), + let previous_flight_plan = (&airplane.flight_plan).into(); + let new_flight_plan: FlightPlan = (&request.flight_plan).into(); + + if let Err(errors) = new_flight_plan.validate(&previous_flight_plan) { + let errors = errors + .iter() + .map(|error| match error { + ValidationError::InvalidFirstNode => { + atc::v1::update_flight_plan_response::Error::InvalidFirstNode.into() } - } - Err(errors) => errors - .iter() - .map(|error| match error { - ValidationError::HasSharpTurns => { - atc::v1::update_flight_plan_response::Error::HasSharpTurns.into() - } - ValidationError::NodeOutOfBounds => { - atc::v1::update_flight_plan_response::Error::NodeOutOfBounds.into() - } - ValidationError::NotInLogicalOrder => { - atc::v1::update_flight_plan_response::Error::NotInLogicalOrder.into() - } - }) - .collect(), - }; + ValidationError::HasSharpTurns => { + atc::v1::update_flight_plan_response::Error::HasSharpTurns.into() + } + ValidationError::NodeOutOfBounds => { + atc::v1::update_flight_plan_response::Error::NodeOutOfBounds.into() + } + ValidationError::NotInLogicalOrder => { + atc::v1::update_flight_plan_response::Error::NotInLogicalOrder.into() + } + }) + .collect(); // TODO: Create different responses for successful and failed updates - Ok(Response::new(UpdateFlightPlanResponse { + return Ok(Response::new(UpdateFlightPlanResponse { validation_errors: errors, - })) - } else { - Err(Status::not_found(&format!( - "No airplane with id {id} was found" - ))) + })); + }; + + if self + .command_bus + .send(Command::UpdateFlightPlan( + AirplaneId::new(id), + new_flight_plan, + )) + .is_err() + { + return Err(Status::internal("failed to queue command")); } + + Ok(Response::new(UpdateFlightPlanResponse { + validation_errors: Vec::new(), + })) } } diff --git a/src/atc-game/src/components/flight_plan.rs b/src/atc-game/src/components/flight_plan.rs index 0a4fb04..80cd460 100644 --- a/src/atc-game/src/components/flight_plan.rs +++ b/src/atc-game/src/components/flight_plan.rs @@ -6,7 +6,6 @@ use crate::api::IntoApi; use crate::map::{Tile, MAP_HEIGHT_RANGE, MAP_WIDTH_RANGE}; #[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)] -#[allow(dead_code)] // Remove when flight plans get validated pub enum ValidationError { InvalidFirstNode, // TODO: Find a more descriptive name HasSharpTurns, @@ -30,7 +29,6 @@ impl FlightPlan { &mut self.0 } - #[allow(dead_code)] // Remove when flight plans get validated pub fn validate(&self, previous_flight_plan: &FlightPlan) -> Result<(), Vec> { let errors: Vec = vec![ self.is_within_map_bounds(), @@ -100,6 +98,17 @@ impl FlightPlan { } } +impl From<&Vec> for FlightPlan { + fn from(api_flight_plan: &Vec) -> Self { + let tiles = api_flight_plan + .iter() + .map(|node| Tile::new(node.x, node.y)) + .collect(); + + FlightPlan(tiles) + } +} + impl IntoApi for FlightPlan { type ApiType = Vec; From fef6b5a872d9a48587a46a9f5c54e321665ed5d5 Mon Sep 17 00:00:00 2001 From: Jan David Date: Mon, 14 Mar 2022 14:35:12 +0100 Subject: [PATCH 07/13] Replace custom validation errors with API types --- protobufs/atc/v1/airplane.proto | 14 ++++++------ protobufs/buf.yaml | 2 ++ src/atc-game/src/api/airplane.rs | 26 ++++------------------ src/atc-game/src/components/flight_plan.rs | 12 +++------- 4 files changed, 16 insertions(+), 38 deletions(-) diff --git a/protobufs/atc/v1/airplane.proto b/protobufs/atc/v1/airplane.proto index 8151c8e..471717f 100644 --- a/protobufs/atc/v1/airplane.proto +++ b/protobufs/atc/v1/airplane.proto @@ -43,14 +43,14 @@ message UpdateFlightPlanRequest { } message UpdateFlightPlanResponse { - enum Error { - ERROR_UNSPECIFIED = 0; - ERROR_NODE_OUT_OF_BOUNDS = 1; - ERROR_NOT_IN_LOGICAL_ORDER = 2; - ERROR_HAS_SHARP_TURNS = 3; - ERROR_INVALID_FIRST_NODE = 4; + enum ValidationError { + UNSPECIFIED = 0; + NODE_OUT_OF_BOUNDS = 1; + NOT_IN_LOGICAL_ORDER = 2; + HAS_SHARP_TURNS = 3; + INVALID_FIRST_NODE = 4; } - repeated Error validation_errors = 1; + repeated ValidationError errors = 1; } service AirplaneService { diff --git a/protobufs/buf.yaml b/protobufs/buf.yaml index 60e99d4..a5c7809 100644 --- a/protobufs/buf.yaml +++ b/protobufs/buf.yaml @@ -4,6 +4,8 @@ version: v1 lint: use: - DEFAULT + except: + - ENUM_VALUE_PREFIX breaking: use: diff --git a/src/atc-game/src/api/airplane.rs b/src/atc-game/src/api/airplane.rs index 5cbd9a1..5a05e00 100644 --- a/src/atc-game/src/api/airplane.rs +++ b/src/atc-game/src/api/airplane.rs @@ -7,7 +7,7 @@ use atc::v1::{ }; use crate::command::CommandSender; -use crate::components::{AirplaneId, FlightPlan, ValidationError}; +use crate::components::{AirplaneId, FlightPlan}; use crate::store::Store; use crate::Command; @@ -62,28 +62,10 @@ impl atc::v1::airplane_service_server::AirplaneService for AirplaneService { let new_flight_plan: FlightPlan = (&request.flight_plan).into(); if let Err(errors) = new_flight_plan.validate(&previous_flight_plan) { - let errors = errors - .iter() - .map(|error| match error { - ValidationError::InvalidFirstNode => { - atc::v1::update_flight_plan_response::Error::InvalidFirstNode.into() - } - ValidationError::HasSharpTurns => { - atc::v1::update_flight_plan_response::Error::HasSharpTurns.into() - } - ValidationError::NodeOutOfBounds => { - atc::v1::update_flight_plan_response::Error::NodeOutOfBounds.into() - } - ValidationError::NotInLogicalOrder => { - atc::v1::update_flight_plan_response::Error::NotInLogicalOrder.into() - } - }) - .collect(); + let errors = errors.iter().map(|error| (*error).into()).collect(); // TODO: Create different responses for successful and failed updates - return Ok(Response::new(UpdateFlightPlanResponse { - validation_errors: errors, - })); + return Ok(Response::new(UpdateFlightPlanResponse { errors })); }; if self @@ -98,7 +80,7 @@ impl atc::v1::airplane_service_server::AirplaneService for AirplaneService { } Ok(Response::new(UpdateFlightPlanResponse { - validation_errors: Vec::new(), + errors: Vec::new(), })) } } diff --git a/src/atc-game/src/components/flight_plan.rs b/src/atc-game/src/components/flight_plan.rs index 80cd460..ef23c6e 100644 --- a/src/atc-game/src/components/flight_plan.rs +++ b/src/atc-game/src/components/flight_plan.rs @@ -1,18 +1,11 @@ use bevy::prelude::*; +use atc::v1::update_flight_plan_response::ValidationError; use atc::v1::Node as ApiNode; use crate::api::IntoApi; use crate::map::{Tile, MAP_HEIGHT_RANGE, MAP_WIDTH_RANGE}; -#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)] -pub enum ValidationError { - InvalidFirstNode, // TODO: Find a more descriptive name - HasSharpTurns, - NodeOutOfBounds, - NotInLogicalOrder, -} - #[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Default, Component)] pub struct FlightPlan(Vec); @@ -119,7 +112,8 @@ impl IntoApi for FlightPlan { #[cfg(test)] mod tests { - use crate::components::ValidationError; + use atc::v1::update_flight_plan_response::ValidationError; + use crate::map::{Tile, MAP_HEIGHT_RANGE, MAP_WIDTH_RANGE}; use super::FlightPlan; From cdb9f071d4c09dcd359c1215c3ffd7a8f20666de Mon Sep 17 00:00:00 2001 From: Jan David Date: Mon, 14 Mar 2022 14:50:31 +0100 Subject: [PATCH 08/13] Create success and error response for flight plan updates --- protobufs/atc/v1/airplane.proto | 8 ++++++++ src/atc-game/src/api/airplane.rs | 11 +++++++---- src/atc-game/src/components/flight_plan.rs | 4 ++-- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/protobufs/atc/v1/airplane.proto b/protobufs/atc/v1/airplane.proto index 471717f..7a194af 100644 --- a/protobufs/atc/v1/airplane.proto +++ b/protobufs/atc/v1/airplane.proto @@ -43,6 +43,14 @@ message UpdateFlightPlanRequest { } message UpdateFlightPlanResponse { + oneof payload { + UpdateFlightPlanSuccess success = 1; + UpdateFlightPlanError error = 2; + } +} + +message UpdateFlightPlanSuccess {} +message UpdateFlightPlanError { enum ValidationError { UNSPECIFIED = 0; NODE_OUT_OF_BOUNDS = 1; diff --git a/src/atc-game/src/api/airplane.rs b/src/atc-game/src/api/airplane.rs index 5a05e00..c020f21 100644 --- a/src/atc-game/src/api/airplane.rs +++ b/src/atc-game/src/api/airplane.rs @@ -2,8 +2,10 @@ use std::sync::Arc; use tonic::{Request, Response, Status}; +use atc::v1::update_flight_plan_response::Payload; use atc::v1::{ - GetAirplaneRequest, GetAirplaneResponse, UpdateFlightPlanRequest, UpdateFlightPlanResponse, + GetAirplaneRequest, GetAirplaneResponse, UpdateFlightPlanError, UpdateFlightPlanRequest, + UpdateFlightPlanResponse, UpdateFlightPlanSuccess, }; use crate::command::CommandSender; @@ -64,8 +66,9 @@ impl atc::v1::airplane_service_server::AirplaneService for AirplaneService { if let Err(errors) = new_flight_plan.validate(&previous_flight_plan) { let errors = errors.iter().map(|error| (*error).into()).collect(); - // TODO: Create different responses for successful and failed updates - return Ok(Response::new(UpdateFlightPlanResponse { errors })); + return Ok(Response::new(UpdateFlightPlanResponse { + payload: Some(Payload::Error(UpdateFlightPlanError { errors })), + })); }; if self @@ -80,7 +83,7 @@ impl atc::v1::airplane_service_server::AirplaneService for AirplaneService { } Ok(Response::new(UpdateFlightPlanResponse { - errors: Vec::new(), + payload: Some(Payload::Success(UpdateFlightPlanSuccess {})), })) } } diff --git a/src/atc-game/src/components/flight_plan.rs b/src/atc-game/src/components/flight_plan.rs index ef23c6e..57dd321 100644 --- a/src/atc-game/src/components/flight_plan.rs +++ b/src/atc-game/src/components/flight_plan.rs @@ -1,6 +1,6 @@ use bevy::prelude::*; -use atc::v1::update_flight_plan_response::ValidationError; +use atc::v1::update_flight_plan_error::ValidationError; use atc::v1::Node as ApiNode; use crate::api::IntoApi; @@ -112,7 +112,7 @@ impl IntoApi for FlightPlan { #[cfg(test)] mod tests { - use atc::v1::update_flight_plan_response::ValidationError; + use atc::v1::update_flight_plan_error::ValidationError; use crate::map::{Tile, MAP_HEIGHT_RANGE, MAP_WIDTH_RANGE}; From 99818c4cde43b40cc04fd97f55c2469326022216 Mon Sep 17 00:00:00 2001 From: Jan David Date: Mon, 14 Mar 2022 15:25:41 +0100 Subject: [PATCH 09/13] Test get airplane API method --- src/atc-game/src/api/airplane.rs | 65 +++++++++++++++++++++++++ src/atc-game/src/components/location.rs | 5 ++ 2 files changed, 70 insertions(+) diff --git a/src/atc-game/src/api/airplane.rs b/src/atc-game/src/api/airplane.rs index c020f21..5f9fe18 100644 --- a/src/atc-game/src/api/airplane.rs +++ b/src/atc-game/src/api/airplane.rs @@ -87,3 +87,68 @@ impl atc::v1::airplane_service_server::AirplaneService for AirplaneService { })) } } + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use tokio::sync::broadcast::channel; + use tonic::{Code, Request}; + + use atc::v1::airplane_service_server::AirplaneService as ServiceTrait; + use atc::v1::Airplane; + use atc::v1::GetAirplaneRequest; + + use crate::api::airplane::AirplaneService; + use crate::api::IntoApi; + use crate::command::CommandReceiver; + use crate::components::{AirplaneId, FlightPlan, Location}; + use crate::{Command, Store}; + + fn setup() -> (CommandReceiver, Arc, AirplaneService) { + let (command_sender, command_receiver) = channel::(1024); + let store = Arc::new(Store::new()); + let service = AirplaneService::new(command_sender, store.clone()); + + (command_receiver, store, service) + } + + #[tokio::test] + async fn get_airplane_for_existing_plane() { + let (_command_bus, store, service) = setup(); + + let id = AirplaneId::new("AT-4321".into()); + let location = Location::new(0, 0); + let flight_plan = FlightPlan::new(Vec::new()); + + let airplane = Airplane { + id: id.into_api(), + location: Some(location.into_api()), + flight_plan: flight_plan.into_api(), + }; + + store.insert("AT-4321".into(), airplane); + + let request = Request::new(GetAirplaneRequest { + id: "AT-4321".into(), + }); + let response = service.get_airplane(request).await.unwrap(); + + let payload = response.into_inner(); + let airplane = payload.airplane.unwrap(); + + assert_eq!("AT-4321", &airplane.id); + } + + #[tokio::test] + async fn get_airplane_with_wrong_id() { + let (_command_bus, _store, service) = setup(); + + let request = Request::new(GetAirplaneRequest { + id: "AT-4321".into(), + }); + let status = service.get_airplane(request).await.unwrap_err(); + + assert_eq!(status.code(), Code::NotFound); + } +} diff --git a/src/atc-game/src/components/location.rs b/src/atc-game/src/components/location.rs index 6378836..a62c357 100644 --- a/src/atc-game/src/components/location.rs +++ b/src/atc-game/src/components/location.rs @@ -16,6 +16,11 @@ pub struct Location { } impl Location { + #[cfg(test)] + pub fn new(x: i32, y: i32) -> Self { + Location { x, y } + } + #[allow(dead_code)] // TODO: Remove when the value is read pub fn x(&self) -> i32 { self.x From d2cdf94a5ad9e1a1e65a9bc6aea16f0fb8606e05 Mon Sep 17 00:00:00 2001 From: Jan David Date: Mon, 14 Mar 2022 16:23:53 +0100 Subject: [PATCH 10/13] Test update flight plan API method --- src/atc-game/src/api/airplane.rs | 138 +++++++++++++++++++-- src/atc-game/src/components/flight_plan.rs | 1 + 2 files changed, 126 insertions(+), 13 deletions(-) diff --git a/src/atc-game/src/api/airplane.rs b/src/atc-game/src/api/airplane.rs index 5f9fe18..84b8af4 100644 --- a/src/atc-game/src/api/airplane.rs +++ b/src/atc-game/src/api/airplane.rs @@ -96,13 +96,15 @@ mod tests { use tonic::{Code, Request}; use atc::v1::airplane_service_server::AirplaneService as ServiceTrait; - use atc::v1::Airplane; - use atc::v1::GetAirplaneRequest; + use atc::v1::update_flight_plan_error::ValidationError; + use atc::v1::update_flight_plan_response::Payload; + use atc::v1::{Airplane, GetAirplaneRequest, UpdateFlightPlanRequest}; use crate::api::airplane::AirplaneService; use crate::api::IntoApi; use crate::command::CommandReceiver; use crate::components::{AirplaneId, FlightPlan, Location}; + use crate::map::{Tile, MAP_HEIGHT_RANGE, MAP_WIDTH_RANGE}; use crate::{Command, Store}; fn setup() -> (CommandReceiver, Arc, AirplaneService) { @@ -113,22 +115,39 @@ mod tests { (command_receiver, store, service) } - #[tokio::test] - async fn get_airplane_for_existing_plane() { - let (_command_bus, store, service) = setup(); - - let id = AirplaneId::new("AT-4321".into()); + fn init_airplane(id: &str, store: &Arc) -> (AirplaneId, Location, FlightPlan) { + let id = AirplaneId::new(id.into()); let location = Location::new(0, 0); - let flight_plan = FlightPlan::new(Vec::new()); + let flight_plan = FlightPlan::new(vec![Tile::new(0, 0)]); let airplane = Airplane { - id: id.into_api(), + id: id.clone().into_api(), location: Some(location.into_api()), - flight_plan: flight_plan.into_api(), + flight_plan: flight_plan.clone().into_api(), }; store.insert("AT-4321".into(), airplane); + (id, location, flight_plan) + } + + #[tokio::test] + async fn get_airplane_with_wrong_id() { + let (_command_bus, _store, service) = setup(); + + let request = Request::new(GetAirplaneRequest { + id: "AT-4321".into(), + }); + let status = service.get_airplane(request).await.unwrap_err(); + + assert_eq!(status.code(), Code::NotFound); + } + + #[tokio::test] + async fn get_airplane_for_existing_plane() { + let (_command_bus, store, service) = setup(); + let (_id, _location, _flight_plan) = init_airplane("AT-4321", &store); + let request = Request::new(GetAirplaneRequest { id: "AT-4321".into(), }); @@ -141,14 +160,107 @@ mod tests { } #[tokio::test] - async fn get_airplane_with_wrong_id() { + async fn update_flight_plan_with_wrong_id() { let (_command_bus, _store, service) = setup(); - let request = Request::new(GetAirplaneRequest { + let request = Request::new(UpdateFlightPlanRequest { id: "AT-4321".into(), + flight_plan: vec![Tile::new(0, 0).into_api()], }); - let status = service.get_airplane(request).await.unwrap_err(); + let status = service.update_flight_plan(request).await.unwrap_err(); assert_eq!(status.code(), Code::NotFound); } + + #[tokio::test] + async fn update_flight_plan_with_invalid_plan() { + let (mut command_bus, store, service) = setup(); + let (_id, _location, _flight_plan) = init_airplane("AT-4321", &store); + + let request = Request::new(UpdateFlightPlanRequest { + id: "AT-4321".into(), + flight_plan: vec![ + Tile::new(1, 0).into_api(), + Tile::new(3, 0).into_api(), + Tile::new(1, 0).into_api(), + Tile::new(MAP_WIDTH_RANGE.start() - 1, MAP_HEIGHT_RANGE.start() - 1).into_api(), + ], + }); + let response = service.update_flight_plan(request).await.unwrap(); + + let actual_errors = match response.into_inner().payload.unwrap() { + Payload::Error(error) => error.errors, + _ => panic!("unexpected payload"), + }; + let expected_errors: Vec = vec![ + ValidationError::NodeOutOfBounds.into(), + ValidationError::NotInLogicalOrder.into(), + ValidationError::InvalidFirstNode.into(), + ValidationError::HasSharpTurns.into(), + ]; + + assert_eq!(expected_errors, actual_errors); + assert!(command_bus.try_recv().is_err()); + } + + #[tokio::test] + async fn update_flight_plan_fails_to_queue_command() { + let (command_bus, store, service) = setup(); + std::mem::drop(command_bus); + + let id = AirplaneId::new("AT-4321".into()); + let location = Location::new(0, 0); + let flight_plan = FlightPlan::new(vec![Tile::new(0, 0)]); + + let airplane = Airplane { + id: id.into_api(), + location: Some(location.into_api()), + flight_plan: flight_plan.into_api(), + }; + + store.insert("AT-4321".into(), airplane); + + let request = Request::new(UpdateFlightPlanRequest { + id: "AT-4321".into(), + flight_plan: vec![Tile::new(0, 0).into_api()], + }); + let status = service.update_flight_plan(request).await.unwrap_err(); + + assert_eq!(status.code(), Code::Internal); + } + + #[tokio::test] + async fn update_flight_plan_with_valid_plan() { + let (mut command_bus, store, service) = setup(); + + let id = AirplaneId::new("AT-4321".into()); + let location = Location::new(0, 0); + let flight_plan = FlightPlan::new(vec![Tile::new(0, 0)]); + + let airplane = Airplane { + id: id.into_api(), + location: Some(location.into_api()), + flight_plan: flight_plan.into_api(), + }; + + store.insert("AT-4321".into(), airplane); + + let new_flight_plan = FlightPlan::new(vec![Tile::new(0, 0), Tile::new(1, 0)]); + + let request = Request::new(UpdateFlightPlanRequest { + id: "AT-4321".into(), + flight_plan: new_flight_plan.clone().into_api(), + }); + let response = service.update_flight_plan(request).await.unwrap(); + + if let Payload::Error(_) = response.into_inner().payload.unwrap() { + panic!("unexpected payload"); + } + + let command = command_bus.try_recv().unwrap(); + let Command::UpdateFlightPlan(airplane_id, flight_plan) = command; + + assert_eq!("AT-4321", airplane_id.get()); + assert_eq!(new_flight_plan, flight_plan); + } } diff --git a/src/atc-game/src/components/flight_plan.rs b/src/atc-game/src/components/flight_plan.rs index 57dd321..0d2ca8b 100644 --- a/src/atc-game/src/components/flight_plan.rs +++ b/src/atc-game/src/components/flight_plan.rs @@ -95,6 +95,7 @@ impl From<&Vec> for FlightPlan { fn from(api_flight_plan: &Vec) -> Self { let tiles = api_flight_plan .iter() + .rev() .map(|node| Tile::new(node.x, node.y)) .collect(); From 59f257074ab3e85f15958af6b2e6a385ce317b23 Mon Sep 17 00:00:00 2001 From: Jan David Date: Mon, 14 Mar 2022 16:26:34 +0100 Subject: [PATCH 11/13] Send event when a flight plan is updated --- src/atc-game/src/systems/update_flight_plan.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/atc-game/src/systems/update_flight_plan.rs b/src/atc-game/src/systems/update_flight_plan.rs index 154464b..43dbe36 100644 --- a/src/atc-game/src/systems/update_flight_plan.rs +++ b/src/atc-game/src/systems/update_flight_plan.rs @@ -1,19 +1,29 @@ use bevy::prelude::*; -use crate::command::CommandBus; +use crate::command::{Command, CommandBus}; use crate::components::{AirplaneId, FlightPlan}; -use crate::Command; +use crate::event::{Event, EventBus}; pub fn update_flight_plan( mut query: Query<(&AirplaneId, &mut FlightPlan)>, mut command_bus: Local, + event_bus: Local, ) { while let Ok(command) = command_bus.receiver().try_recv() { let Command::UpdateFlightPlan(airplane_id, new_flight_plan) = command; for (id, mut current_flight_plan) in query.iter_mut() { if *id == airplane_id { - *current_flight_plan = new_flight_plan; + *current_flight_plan = new_flight_plan.clone(); + + event_bus + .sender() + .send(Event::FlightPlanUpdated( + airplane_id.clone(), + new_flight_plan, + )) + .expect("failed to send event"); // TODO: Handle error + break; } } From c2935a42ccd170fadf0047f0edbb2ffdfa384721 Mon Sep 17 00:00:00 2001 From: Jan David Date: Mon, 14 Mar 2022 17:50:45 +0100 Subject: [PATCH 12/13] Silence Protobuf style violation --- protobufs/atc/v1/airplane.proto | 2 ++ protobufs/buf.yaml | 3 +-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/protobufs/atc/v1/airplane.proto b/protobufs/atc/v1/airplane.proto index 7a194af..c235840 100644 --- a/protobufs/atc/v1/airplane.proto +++ b/protobufs/atc/v1/airplane.proto @@ -51,6 +51,8 @@ message UpdateFlightPlanResponse { message UpdateFlightPlanSuccess {} message UpdateFlightPlanError { + // buf:lint:ignore ENUM_VALUE_PREFIX + // buf:lint:ignore ENUM_ZERO_VALUE_SUFFIX enum ValidationError { UNSPECIFIED = 0; NODE_OUT_OF_BOUNDS = 1; diff --git a/protobufs/buf.yaml b/protobufs/buf.yaml index a5c7809..9148903 100644 --- a/protobufs/buf.yaml +++ b/protobufs/buf.yaml @@ -4,8 +4,7 @@ version: v1 lint: use: - DEFAULT - except: - - ENUM_VALUE_PREFIX + allow_comment_ignores: true breaking: use: From dba2b37fdb706b1f53e0a52dd4d84790de1f0556 Mon Sep 17 00:00:00 2001 From: Jan David Date: Mon, 14 Mar 2022 18:09:55 +0100 Subject: [PATCH 13/13] Validate flight plan in Bevy system before using it --- .../src/systems/update_flight_plan.rs | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/atc-game/src/systems/update_flight_plan.rs b/src/atc-game/src/systems/update_flight_plan.rs index 43dbe36..7608d02 100644 --- a/src/atc-game/src/systems/update_flight_plan.rs +++ b/src/atc-game/src/systems/update_flight_plan.rs @@ -14,17 +14,21 @@ pub fn update_flight_plan( for (id, mut current_flight_plan) in query.iter_mut() { if *id == airplane_id { - *current_flight_plan = new_flight_plan.clone(); + if new_flight_plan.validate(¤t_flight_plan).is_ok() { + *current_flight_plan = new_flight_plan.clone(); - event_bus - .sender() - .send(Event::FlightPlanUpdated( - airplane_id.clone(), - new_flight_plan, - )) - .expect("failed to send event"); // TODO: Handle error + event_bus + .sender() + .send(Event::FlightPlanUpdated( + airplane_id.clone(), + new_flight_plan, + )) + .expect("failed to send event"); // TODO: Handle error - break; + break; + } else { + // TODO: Handle error + } } } }