Unit tests for Reservation

CMakeLists.txt

@@ -125,6 +125,7 @@ set(MOCPP_SRC_UNIT
     tests/Api.cpp
     tests/Metering.cpp
     tests/Configuration.cpp
+    tests/Reservation.cpp
 )
 
 add_executable(mo_unit_tests

src/MicroOcpp/Model/ConnectorBase/Connector.cpp

@@ -524,47 +524,62 @@ std::shared_ptr<Transaction> Connector::beginTransaction(const char *idTag) {
         return nullptr;
     }
 
-    auto rService = model.getReservationService();
+    MOCPP_DBG_DEBUG("Begin transaction process (%s), prepare", idTag != nullptr ? idTag : "");
+
     AuthorizationData *localAuth = nullptr;
-    bool expiredLocalAuth = false;
+    bool offlineBlockedAuth = false; //if offline authorization will be blocked by local auth list entry
 
     //check local OCPP whitelist
-    if (auto authService = model.getAuthorizationService()) {
+    if (model.getAuthorizationService()) {
+        localAuth = model.getAuthorizationService()->getLocalAuthorization(idTag);
 
-        localAuth = authService->getLocalAuthorization(idTag);
-
-        //check if blocked by reservation
-        Reservation *reservation = nullptr;
-        if (localAuth && rService) {
-            reservation = rService->getReservation(
-                        connectorId,
-                        idTag,
-                        localAuth->getParentIdTag() ? localAuth->getParentIdTag() : nullptr);
-            if (reservation && !reservation->matches(
-                        idTag,
-                        localAuth->getParentIdTag() ? localAuth->getParentIdTag() : nullptr)) {
-                //reservation found for connector but does not match idTag or parentIdTag
-                MOCPP_DBG_DEBUG("reservation blocks local auth at connector %u", connectorId);
-                localAuth = nullptr;;
-            }
-        }
-
+        //check authorization status
         if (localAuth && localAuth->getAuthorizationStatus() != AuthorizationStatus::Accepted) {
             MOCPP_DBG_DEBUG("local auth denied (%s)", idTag);
+            offlineBlockedAuth = true;
             localAuth = nullptr;
         }
 
         //check expiry
-        if (localAuth && localAuth->getExpiryDate()) {
-            auto& tnow = model.getClock().now();
-            if (tnow >= *localAuth->getExpiryDate()) {
-                MOCPP_DBG_DEBUG("idTag %s local auth entry expired", idTag);
+        if (localAuth && localAuth->getExpiryDate() && *localAuth->getExpiryDate() < model.getClock().now()) {
+            MOCPP_DBG_DEBUG("idTag %s local auth entry expired", idTag);
+            offlineBlockedAuth = true;
+            localAuth = nullptr;
+        }
+    }
+
+    Reservation *reservation = nullptr;
+    bool offlineBlockedResv = false; //if offline authorization will be blocked by reservation
+
+    //check if blocked by reservation
+    if (model.getReservationService()) {
+        const char *parentIdTag = localAuth ? localAuth->getParentIdTag() : nullptr;
+
+        reservation = model.getReservationService()->getReservation(
+                connectorId,
+                idTag,
+                parentIdTag);
+
+        if (reservation && !reservation->matches(
+                    idTag,
+                    parentIdTag)) {
+            //reservation blocks connector
+            offlineBlockedResv = true; //when offline, tx is always blocked
+
+            //if parentIdTag is known, abort this tx immediately, otherwise wait for Authorize.conf to decide
+            if (parentIdTag) {
+                //parentIdTag known
+                MOCPP_DBG_INFO("connector %u reserved - abort transaction", connectorId);
+                updateTxNotification(TxNotification::ReservationConflict);
+                return nullptr;
+            } else {
+                //parentIdTag unkown but local authorization failed in any case
+                MOCPP_DBG_INFO("connector %u reserved - no local auth", connectorId);
                 localAuth = nullptr;
-                expiredLocalAuth = true;
             }
         }
     }
-
+    
     transaction = allocateTransaction();
 
     if (!transaction) {
@@ -581,17 +596,16 @@ std::shared_ptr<Transaction> Connector::beginTransaction(const char *idTag) {
 
     transaction->setBeginTimestamp(model.getClock().now());
 
-    MOCPP_DBG_DEBUG("Begin transaction process (%s), prepare", idTag != nullptr ? idTag : "");
-
     //check for local preauthorization
-    if (localPreAuthorizeBool && localPreAuthorizeBool->getBool()) {
-        if (localAuth && localAuth->getAuthorizationStatus() == AuthorizationStatus::Accepted) {
-            MOCPP_DBG_DEBUG("Begin transaction process (%s), preauthorized locally", idTag != nullptr ? idTag : "");
-
-            transaction->setAuthorized();
+    if (localAuth && localPreAuthorizeBool && localPreAuthorizeBool->getBool()) {
+        MOCPP_DBG_DEBUG("Begin transaction process (%s), preauthorized locally", idTag != nullptr ? idTag : "");
 
-            updateTxNotification(TxNotification::Authorized);
+        if (reservation) {
+            transaction->setReservationId(reservation->getReservationId());
         }
+        transaction->setAuthorized();
+
+        updateTxNotification(TxNotification::Authorized);
     }
 
     transaction->commit();
@@ -616,15 +630,21 @@ std::shared_ptr<Transaction> Connector::beginTransaction(const char *idTag) {
                         connectorId,
                         tx->getIdTag(),
                         idTagInfo["parentIdTag"] | (const char*) nullptr);
-            if (reservation && !reservation->matches(
-                        tx->getIdTag(),
-                        idTagInfo["parentIdTag"] | (const char*) nullptr)) {
-                //reservation found for connector but does not match idTag or parentIdTag
-                MOCPP_DBG_INFO("connector %u reserved - abort transaction", connectorId);
-                tx->setInactive();
-                tx->commit();
-                updateTxNotification(TxNotification::ReservationConflict);
-                return;
+            if (reservation) {
+                //reservation found for connector
+                if (reservation->matches(
+                            tx->getIdTag(),
+                            idTagInfo["parentIdTag"] | (const char*) nullptr)) {
+                    MOCPP_DBG_INFO("connector %u matches reservationId %i", connectorId, reservation->getReservationId());
+                    tx->setReservationId(reservation->getReservationId());
+                } else {
+                    //reservation found for connector but does not match idTag or parentIdTag
+                    MOCPP_DBG_INFO("connector %u reserved - abort transaction", connectorId);
+                    tx->setInactive();
+                    tx->commit();
+                    updateTxNotification(TxNotification::ReservationConflict);
+                    return;
+                }
             }
         }
 
@@ -634,49 +654,59 @@ std::shared_ptr<Transaction> Connector::beginTransaction(const char *idTag) {
 
         updateTxNotification(TxNotification::Authorized);
     });
-    authorize->setOnTimeoutListener([this, tx, localAuth, expiredLocalAuth] () {
 
-        if (expiredLocalAuth) {
+    //capture local auth and reservation check in for timeout handler
+    bool localAuthFound = localAuth;
+    bool reservationFound = reservation;
+    int reservationId = reservation ? reservation->getReservationId() : -1;
+    authorize->setOnTimeoutListener([this, tx,
+                offlineBlockedAuth, 
+                offlineBlockedResv, 
+                localAuthFound,
+                reservationFound,
+                reservationId] () {
+
+        if (offlineBlockedAuth) {
             //local auth entry exists, but is expired -> avoid offline tx
             MOCPP_DBG_DEBUG("Abort transaction process (%s), timeout, expired local auth", tx->getIdTag());
             tx->setInactive();
             tx->commit();
             updateTxNotification(TxNotification::AuthorizationTimeout);
             return;
         }
-
-        if (localAuth) {
+
+        if (offlineBlockedResv) {
+            //reservation found for connector but does not match idTag or parentIdTag
+            MOCPP_DBG_INFO("connector %u reserved (offline) - abort transaction", connectorId);
+            tx->setInactive();
+            tx->commit();
+            updateTxNotification(TxNotification::ReservationConflict);
+            return;
+        }
+
+        if (localAuthFound) {
             MOCPP_DBG_DEBUG("Offline transaction process (%s), locally authorized", tx->getIdTag());
+            if (reservationFound) {
+                tx->setReservationId(reservationId);
+            }
             tx->setAuthorized();
             tx->commit();
 
             updateTxNotification(TxNotification::Authorized);
             return;
         }
 
-        if (model.getReservationService()) {
-            auto reservation = model.getReservationService()->getReservation(
-                        connectorId,
-                        tx->getIdTag());
-            if (reservation && !reservation->matches(
-                        tx->getIdTag())) {
-                //reservation found for connector but does not match idTag or parentIdTag
-                MOCPP_DBG_INFO("connector %u reserved (offline) - abort transaction", connectorId);
-                tx->setInactive();
-                tx->commit();
-                updateTxNotification(TxNotification::ReservationConflict);
-                return;
-            }
-        }
-
         if (allowOfflineTxForUnknownIdBool && allowOfflineTxForUnknownIdBool->getBool()) {
             MOCPP_DBG_DEBUG("Offline transaction process (%s), allow unknown ID", tx->getIdTag());
+            if (reservationFound) {
+                tx->setReservationId(reservationId);
+            }
             tx->setAuthorized();
             tx->commit();
             updateTxNotification(TxNotification::Authorized);
             return;
         }
-        
+
         MOCPP_DBG_DEBUG("Abort transaction process (%s): timeout", tx->getIdTag());
         tx->setInactive();
         tx->commit();

src/MicroOcpp/Model/Reservation/Reservation.cpp

@@ -6,10 +6,6 @@
 #include <MicroOcpp/Model/Model.h>
 #include <MicroOcpp/Debug.h>
 
-#ifndef RESERVATION_FN
-#define RESERVATION_FN (MOCPP_FILENAME_PREFIX "reservations.jsn")
-#endif
-
 using namespace MicroOcpp;
 
 Reservation::Reservation(Model& model, unsigned int slot) : model(model), slot(slot) {
@@ -19,7 +15,6 @@ Reservation::Reservation(Model& model, unsigned int slot) : model(model), slot(s
 
     snprintf(expiryDateRawKey, sizeof(expiryDateRawKey), MOCPP_RESERVATION_EXPDATE_KEY "%u", slot);
     expiryDateRawString = declareConfiguration<const char*>(expiryDateRawKey, "", RESERVATION_FN, false, false, false);
-    expiryDate.setTime(expiryDateRawString->getString());
 
     snprintf(idTagKey, sizeof(idTagKey), MOCPP_RESERVATION_IDTAG_KEY "%u", slot);
     idTagString = declareConfiguration<const char*>(idTagKey, "", RESERVATION_FN, false, false, false);
@@ -60,8 +55,7 @@ bool Reservation::isActive() {
         return false;
     }
 
-    auto& t_now = model.getClock().now();
-    if (t_now > expiryDate) {
+    if (model.getClock().now() > getExpiryDate()) {
         //reservation expired
         return false;
     }
@@ -94,6 +88,9 @@ int Reservation::getConnectorId() {
 }
 
 Timestamp& Reservation::getExpiryDate() {
+    if (expiryDate == MIN_TIME && *expiryDateRawString->getString()) {
+        expiryDate.setTime(expiryDateRawString->getString());
+    }
     return expiryDate;
 }
 
@@ -109,13 +106,16 @@ const char *Reservation::getParentIdTag() {
     return parentIdTagString->getString();
 }
 
-void Reservation::update(JsonObject payload) {
-    connectorIdInt->setInt(payload["connectorId"] | -1);
-    expiryDate.setTime(payload["expiryDate"]);
-    expiryDateRawString->setString(payload["expiryDate"] | "");
-    idTagString->setString(payload["idTag"] | "");
-    reservationIdInt->setInt(payload["reservationId"] | -1);
-    parentIdTagString->setString(payload["parentIdTag"] | "");
+void Reservation::update(int reservationId, unsigned int connectorId, Timestamp expiryDate, const char *idTag, const char *parentIdTag) {
+    reservationIdInt->setInt(reservationId);
+    connectorIdInt->setInt((int) connectorId);
+    this->expiryDate = expiryDate;
+    char expiryDate_cstr [JSONDATE_LENGTH + 1];
+    if (this->expiryDate.toJsonString(expiryDate_cstr, JSONDATE_LENGTH + 1)) {
+        expiryDateRawString->setString(expiryDate_cstr);
+    }
+    idTagString->setString(idTag);
+    parentIdTagString->setString(parentIdTag);
 
     configuration_save();
 }

src/MicroOcpp/Model/Reservation/Reservation.h

@@ -8,6 +8,10 @@
 #include <MicroOcpp/Core/Configuration.h>
 #include <MicroOcpp/Core/Time.h>
 
+#ifndef RESERVATION_FN
+#define RESERVATION_FN (MOCPP_FILENAME_PREFIX "reservations.jsn")
+#endif
+
 #define MOCPP_RESERVATION_CID_KEY "cid_"
 #define MOCPP_RESERVATION_EXPDATE_KEY "expdt_"
 #define MOCPP_RESERVATION_IDTAG_KEY "idt_"
@@ -28,7 +32,7 @@ class Reservation {
     std::shared_ptr<Configuration> expiryDateRawString;
     char expiryDateRawKey [sizeof(MOCPP_RESERVATION_EXPDATE_KEY "xxx") + 1];
 
-    Timestamp expiryDate;
+    Timestamp expiryDate = MIN_TIME;
     std::shared_ptr<Configuration> idTagString;
     char idTagKey [sizeof(MOCPP_RESERVATION_IDTAG_KEY "xxx") + 1];
     std::shared_ptr<Configuration> reservationIdInt;
@@ -55,7 +59,7 @@ class Reservation {
     int getReservationId();
     const char *getParentIdTag();
 
-    void update(JsonObject payload);
+    void update(int reservationId, unsigned int connectorId, Timestamp expiryDate, const char *idTag, const char *parentIdTag = nullptr);
     void clear();
 };
 

src/MicroOcpp/Model/Reservation/ReservationService.cpp

@@ -18,7 +18,7 @@ ReservationService::ReservationService(Context& context, unsigned int numConnect
     if (maxReservations > 0) {
         reservations.reserve((size_t) maxReservations);
         for (int i = 0; i < maxReservations; i++) {
-            reservations.push_back(std::unique_ptr<Reservation>(new Reservation(context.getModel(), i)));
+            reservations.emplace_back(new Reservation(context.getModel(), i));
         }
     }
 
@@ -48,7 +48,7 @@ void ReservationService::loop() {
             }
 
             //check if other tx started at this connector (e.g. due to RemoteStartTransaction)
-            if (connector->getTransaction() && connector->getTransaction()->isActive()) {
+            if (connector->getTransaction() && connector->getTransaction()->isAuthorized()) {
                 reservation->clear();
                 continue;
             }
@@ -57,11 +57,11 @@ void ReservationService::loop() {
         //check if tx with same idTag or reservationId has started
         for (unsigned int cId = 1; cId < context.getModel().getNumConnectors(); cId++) {
             auto& transaction = context.getModel().getConnector(cId)->getTransaction();
-            if (transaction) {
+            if (transaction && transaction->isAuthorized()) {
                 const char *cIdTag = transaction->getIdTag();
                 if (transaction->getReservationId() == reservation->getReservationId() || 
                         (cIdTag && !strcmp(cIdTag, reservation->getIdTag()))) {
-                    
+
                     reservation->clear();
                     break;
                 }
@@ -178,15 +178,19 @@ Reservation *ReservationService::getReservationById(int reservationId) {
     return nullptr;
 }
 
-bool ReservationService::updateReservation(JsonObject payload) {
-    if (auto reservation = getReservationById(payload["reservationId"])) {
-        reservation->update(payload);
+bool ReservationService::updateReservation(int reservationId, unsigned int connectorId, Timestamp expiryDate, const char *idTag, const char *parentIdTag) {
+    if (auto reservation = getReservationById(reservationId)) {
+        if (getReservation(connectorId) && getReservation(connectorId) != reservation && getReservation(connectorId)->isActive()) {
+            MOCPP_DBG_DEBUG("found blocking reservation at connectorId %u", connectorId);
+            return false; //cannot transfer reservation to other connector with existing reservation
+        }
+        reservation->update(reservationId, connectorId, expiryDate, idTag, parentIdTag);
         return true;
     }
 
 // Alternative condition: avoids that one idTag can make two reservations at a time. The specification doesn't
 // mention that double-reservations should be possible but it seems to mean it. 
-    if (auto reservation = getReservation(payload["connectorId"], nullptr, nullptr)) {
+    if (auto reservation = getReservation(connectorId, nullptr, nullptr)) {
 //                payload["idTag"],
 //                payload.containsKey("parentIdTag") ? payload["parentIdTag"] : nullptr)) {
 //    if (auto reservation = getReservation(payload["connectorId"].as<int>())) {
@@ -198,7 +202,7 @@ bool ReservationService::updateReservation(JsonObject payload) {
     //update free reservation slot
     for (auto& reservation : reservations) {
         if (!reservation->isActive()) {
-            reservation->update(payload);
+            reservation->update(reservationId, connectorId, expiryDate, idTag, parentIdTag);
             return true;
         }
     }

src/MicroOcpp/Model/Reservation/ReservationService.h

@@ -39,7 +39,7 @@ class ReservationService {
 
     Reservation *getReservationById(int reservationId);
 
-    bool updateReservation(JsonObject payload);
+    bool updateReservation(int reservationId, unsigned int connectorId, Timestamp expiryDate, const char *idTag, const char *parentIdTag = nullptr);
 };
 
 }