Skip to content

Commit

Permalink
Merge pull request #479 from AndyTWF/remove-key-from-prenotes
Browse files Browse the repository at this point in the history
fix: remove key from prenote dependency
  • Loading branch information
AndyTWF authored Jul 31, 2022
2 parents 5368347 + 9209653 commit 1d17631
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 53 deletions.
5 changes: 2 additions & 3 deletions src/plugin/prenote/PublishedPrenote.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@
#include "controller/ControllerPositionHierarchy.h"

namespace UKControllerPlugin::Prenote {
PublishedPrenote::PublishedPrenote(
int id, std::string key, std::unique_ptr<Controller::ControllerPositionHierarchy> controllers)
: id(id), key(std::move(key)), controllers(std::move(controllers))
PublishedPrenote::PublishedPrenote(int id, std::unique_ptr<Controller::ControllerPositionHierarchy> controllers)
: id(id), controllers(std::move(controllers))
{
}

Expand Down
5 changes: 1 addition & 4 deletions src/plugin/prenote/PublishedPrenote.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace UKControllerPlugin::Prenote {
*/
using PublishedPrenote = struct PublishedPrenote
{
PublishedPrenote(int id, std::string key, std::unique_ptr<Controller::ControllerPositionHierarchy> controllers);
PublishedPrenote(int id, std::unique_ptr<Controller::ControllerPositionHierarchy> controllers);
~PublishedPrenote();
PublishedPrenote(const PublishedPrenote&) = delete;
PublishedPrenote(PublishedPrenote&&) noexcept;
Expand All @@ -22,9 +22,6 @@ namespace UKControllerPlugin::Prenote {
// The id of the prenote
int id;

// A string key to identify the prenote
std::string key;

// The controllers that this prenote refers to
std::unique_ptr<Controller::ControllerPositionHierarchy> controllers;
};
Expand Down
8 changes: 3 additions & 5 deletions src/plugin/prenote/PublishedPrenoteCollectionFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ namespace UKControllerPlugin::Prenote {
}

collection->Add(std::make_shared<PublishedPrenote>(
prenote.at("id").get<int>(),
prenote.at("key").get<std::string>(),
hierarchyFactory.CreateFromJsonById(prenote.at("controller_positions"))));
prenote.at("id").get<int>(), hierarchyFactory.CreateFromJsonById(prenote.at("controller_positions"))));
}

LogInfo("Loaded " + std::to_string(collection->Count()) + " published prenotes");
Expand All @@ -36,8 +34,8 @@ namespace UKControllerPlugin::Prenote {
-> bool
{
return prenote.is_object() && prenote.contains("id") && prenote.at("id").is_number_integer() &&
prenote.contains("key") && prenote.at("key").is_string() && prenote.contains("controller_positions") &&
prenote.at("controller_positions").is_array() && !prenote.at("controller_positions").empty() &&
prenote.contains("controller_positions") && prenote.at("controller_positions").is_array() &&
!prenote.at("controller_positions").empty() &&
hierarchyFactory.CreateFromJsonById(prenote.at("controller_positions")) != nullptr;
}
} // namespace UKControllerPlugin::Prenote
6 changes: 3 additions & 3 deletions test/plugin/prenote/PrenoteServiceTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ namespace UKControllerPluginTest::Prenote {
this->service = std::make_unique<PrenoteService>(
this->mapper, this->airfieldOwnership, this->activeCallsigns, *this->messager);

this->prenotes.Add(std::make_shared<PublishedPrenote>(1, "test", std::move(this->hierarchyPointer)));
this->prenotes.Add(std::make_shared<PublishedPrenote>(1, std::move(this->hierarchyPointer)));

ON_CALL(this->mockFlightplan, GetOrigin).WillByDefault(Return("EGKK"));
ON_CALL(this->mockFlightplan, GetCallsign).WillByDefault(Return("BAW123"));
Expand Down Expand Up @@ -211,7 +211,7 @@ namespace UKControllerPluginTest::Prenote {

auto hierarchyTwo = std::make_unique<ControllerPositionHierarchy>();
hierarchyTwo->AddPosition(this->controllerNoLondon);
this->prenotes.Add(std::make_shared<PublishedPrenote>(2, "test", std::move(hierarchyTwo)));
this->prenotes.Add(std::make_shared<PublishedPrenote>(2, std::move(hierarchyTwo)));

EXPECT_CALL(
mockPlugin,
Expand Down Expand Up @@ -250,7 +250,7 @@ namespace UKControllerPluginTest::Prenote {
auto hierarchyTwo = std::make_unique<ControllerPositionHierarchy>();
hierarchyTwo->AddPosition(this->controllerOther);
hierarchyTwo->AddPosition(this->controllerNoLondon);
this->prenotes.Add(std::make_shared<PublishedPrenote>(2, "test", std::move(hierarchyTwo)));
this->prenotes.Add(std::make_shared<PublishedPrenote>(2, std::move(hierarchyTwo)));

ON_CALL(this->mockFlightplan, GetSidName).WillByDefault(Return("LAM4M"));
ON_CALL(sidMapper, MapFlightplanToSid(testing::Ref(mockFlightplan)))
Expand Down
45 changes: 12 additions & 33 deletions test/plugin/prenote/PublishedPrenoteCollectionFactoryTest.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#include "controller/ControllerPosition.h"
#include "controller/ControllerPositionCollection.h"
#include "controller/ControllerPositionHierarchyFactory.h"
#include "controller/ControllerPositionHierarchy.h"
#include "controller/ControllerPositionHierarchyFactory.h"
#include "prenote/PublishedPrenote.h"
#include "prenote/PublishedPrenoteCollection.h"
#include "prenote/PublishedPrenoteCollectionFactory.h"
Expand Down Expand Up @@ -32,66 +32,49 @@ namespace UKControllerPluginTest::Prenote {

TEST_F(PublishedPrenoteCollectionFactoryTest, APrenoteIsValid)
{
nlohmann::json data = {
{"id", 1}, {"key", "prenote_one"}, {"controller_positions", nlohmann::json::array({1, 2})}};
nlohmann::json data = {{"id", 1}, {"controller_positions", nlohmann::json::array({1, 2})}};

EXPECT_TRUE(PrenoteValid(data, factory));
}

TEST_F(PublishedPrenoteCollectionFactoryTest, APrenoteIsInvalidIfIdMissing)
{
nlohmann::json data = {{"key", "prenote_one"}, {"controller_positions", nlohmann::json::array({1, 2})}};
nlohmann::json data = {{"controller_positions", nlohmann::json::array({1, 2})}};

EXPECT_FALSE(PrenoteValid(data, factory));
}

TEST_F(PublishedPrenoteCollectionFactoryTest, APrenoteIsInvalidIfIdNotInterger)
{
nlohmann::json data = {
{"id", "abc"}, {"key", "prenote_one"}, {"controller_positions", nlohmann::json::array({1, 2})}};

EXPECT_FALSE(PrenoteValid(data, factory));
}

TEST_F(PublishedPrenoteCollectionFactoryTest, APrenoteIsInvalidIfKeyMissing)
{
nlohmann::json data = {{"id", 1}, {"controller_positions", nlohmann::json::array({1, 2})}};

EXPECT_FALSE(PrenoteValid(data, factory));
}

TEST_F(PublishedPrenoteCollectionFactoryTest, APrenoteIsInvalidIfKeyNotString)
{
nlohmann::json data = {{"id", 1}, {"key", 123}, {"controller_positions", nlohmann::json::array({1, 2})}};
nlohmann::json data = {{"id", "abc"}, {"controller_positions", nlohmann::json::array({1, 2})}};

EXPECT_FALSE(PrenoteValid(data, factory));
}

TEST_F(PublishedPrenoteCollectionFactoryTest, APrenoteIsInvalidIfControllersMissing)
{
nlohmann::json data = {{"id", 1}, {"key", "prenote_one"}};
nlohmann::json data = {{"id", 1}};

EXPECT_FALSE(PrenoteValid(data, factory));
}

TEST_F(PublishedPrenoteCollectionFactoryTest, APrenoteIsInvalidIfControllersNotArray)
{
nlohmann::json data = {{"id", 1}, {"key", "prenote_one"}, {"controller_positions", nlohmann::json::object()}};
nlohmann::json data = {{"id", 1}, {"controller_positions", nlohmann::json::object()}};

EXPECT_FALSE(PrenoteValid(data, factory));
}

TEST_F(PublishedPrenoteCollectionFactoryTest, APrenoteIsInvalidIfControllersInvalid)
{
nlohmann::json data = {
{"id", 1}, {"key", "prenote_one"}, {"controller_positions", nlohmann::json::array({"ab", "cd"})}};
nlohmann::json data = {{"id", 1}, {"controller_positions", nlohmann::json::array({"ab", "cd"})}};

EXPECT_FALSE(PrenoteValid(data, factory));
}

TEST_F(PublishedPrenoteCollectionFactoryTest, APrenoteIsInvalidIfControllersEmpty)
{
nlohmann::json data = {{"id", 1}, {"key", "prenote_one"}, {"controller_positions", nlohmann::json::array()}};
nlohmann::json data = {{"id", 1}, {"controller_positions", nlohmann::json::array()}};

EXPECT_FALSE(PrenoteValid(data, factory));
}
Expand All @@ -104,34 +87,30 @@ namespace UKControllerPluginTest::Prenote {
TEST_F(PublishedPrenoteCollectionFactoryTest, ItReturnsACollection)
{
auto data = nlohmann::json::array();
data.push_back({{"id", 1}, {"key", "prenote_one"}, {"controller_positions", nlohmann::json::array({1, 2})}});
data.push_back({{"id", 2}, {"key", "prenote_two"}, {"controller_positions", nlohmann::json::array({1})}});
data.push_back({{"id", 1}, {"controller_positions", nlohmann::json::array({1, 2})}});
data.push_back({{"id", 2}, {"controller_positions", nlohmann::json::array({1})}});

auto collection = CreatePublishedPrenoteCollection(data, factory);
EXPECT_EQ(2, collection->Count());
auto prenoteOne = collection->Get(1);
EXPECT_EQ(1, prenoteOne->id);
EXPECT_EQ("prenote_one", prenoteOne->key);
EXPECT_EQ(2, prenoteOne->controllers->CountPositions());

auto prenoteTwo = collection->Get(2);
EXPECT_EQ(2, prenoteTwo->id);
EXPECT_EQ("prenote_two", prenoteTwo->key);
EXPECT_EQ(1, prenoteTwo->controllers->CountPositions());
}

TEST_F(PublishedPrenoteCollectionFactoryTest, ItIgnoresInvalidPrenotes)
{
auto data = nlohmann::json::array();
data.push_back({{"id", 1}, {"key", "prenote_one"}, {"controller_positions", nlohmann::json::array({1, 2})}});
data.push_back(
{{"id", "invalid"}, {"key", "prenote_two"}, {"controller_positions", nlohmann::json::array({1})}});
data.push_back({{"id", 1}, {"controller_positions", nlohmann::json::array({1, 2})}});
data.push_back({{"id", "invalid"}, {"controller_positions", nlohmann::json::array({1})}});

auto collection = CreatePublishedPrenoteCollection(data, factory);
EXPECT_EQ(1, collection->Count());
auto prenoteOne = collection->Get(1);
EXPECT_EQ(1, prenoteOne->id);
EXPECT_EQ("prenote_one", prenoteOne->key);
EXPECT_EQ(2, prenoteOne->controllers->CountPositions());
}

Expand Down
4 changes: 2 additions & 2 deletions test/plugin/prenote/PublishedPrenoteCollectionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ namespace UKControllerPluginTest::Prenote {
public:
PublishedPrenoteCollectionTest()
{
this->prenote1 = std::make_shared<PublishedPrenote>(1, "One", nullptr);
this->prenote2 = std::make_shared<PublishedPrenote>(2, "Two", nullptr);
this->prenote1 = std::make_shared<PublishedPrenote>(1, nullptr);
this->prenote2 = std::make_shared<PublishedPrenote>(2, nullptr);
}

std::shared_ptr<PublishedPrenote> prenote1;
Expand Down
6 changes: 3 additions & 3 deletions test/plugin/prenote/PublishedPrenoteMapperTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ namespace UKControllerPluginTest::Prenote {

this->flightRules.Add(std::make_shared<FlightRule>(1, "I", "IFR"));
this->flightRules.Add(std::make_shared<FlightRule>(2, "V", "VFR"));
this->prenotes.Add(std::make_shared<PublishedPrenote>(1, "TEST1", nullptr));
this->prenotes.Add(std::make_shared<PublishedPrenote>(2, "TEST2", nullptr));
this->prenotes.Add(std::make_shared<PublishedPrenote>(3, "TEST3", nullptr));
this->prenotes.Add(std::make_shared<PublishedPrenote>(1, nullptr));
this->prenotes.Add(std::make_shared<PublishedPrenote>(2, nullptr));
this->prenotes.Add(std::make_shared<PublishedPrenote>(3, nullptr));
}

testing::NiceMock<Sid::MockSidMapperInterface> sidMapper;
Expand Down

0 comments on commit 1d17631

Please sign in to comment.