DebitNote keep alive property naming (#990)

* DebitNote keep alive property naming * Fix unused property name which is actually used in tests
golemfactory · Jan 29, 2021 · ed42183 · ed42183
1 parent 86bdee2
commit ed42183
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 21 deletions.
diff --git a/agent/provider/src/market/mod.rs b/agent/provider/src/market/mod.rs
@@ -1,5 +1,5 @@
 pub mod config;
-mod negotiator;
+pub mod negotiator;
 pub mod presets;
 pub mod provider_market;
 pub mod termination_reason;

diff --git a/agent/provider/src/market/negotiator.rs b/agent/provider/src/market/negotiator.rs
@@ -1,5 +1,5 @@
 mod accept_all;
-mod builtin;
+pub mod builtin;
 mod common;
 mod component;
 mod composite;

diff --git a/agent/provider/src/market/negotiator/builtin.rs b/agent/provider/src/market/negotiator/builtin.rs
@@ -1,5 +1,5 @@
-mod expiration;
-mod max_agreements;
+pub mod expiration;
+pub mod max_agreements;
 
 pub use expiration::LimitExpiration;
 pub use max_agreements::MaxAgreements;
diff --git a/agent/provider/src/market/negotiator/builtin/expiration.rs b/agent/provider/src/market/negotiator/builtin/expiration.rs
@@ -25,6 +25,18 @@ pub struct LimitExpiration {
     min_deadline: i64,
 }
 
+pub static DEBIT_NOTE_ACCEPT_TIMEOUT_PROPERTY: &'static str =
+    "/golem/com/payment/debit-notes/accept-timeout?";
+pub static AGREEMENT_EXPIRATION_PROPERTY: &'static str = "/golem/srv/comp/expiration";
+
+// TODO: We should unify properties access in agreement-utils, because it is annoying to use both forms.
+pub static DEBIT_NOTE_ACCEPT_TIMEOUT_PROPERTY_FLAT: &'static str =
+    "golem.com.payment.debit-notes.accept-timeout?";
+
+// Note: Tests are using this.
+#[allow(dead_code)]
+pub static AGREEMENT_EXPIRATION_PROPERTY_FLAT: &'static str = "golem.srv.comp.expiration";
+
 impl LimitExpiration {
     pub fn new(config: &AgreementExpirationNegotiatorConfig) -> anyhow::Result<LimitExpiration> {
         let component = LimitExpiration {
@@ -49,17 +61,16 @@ impl LimitExpiration {
 }
 
 fn proposal_expiration_from(proposal: &ProposalView) -> Result<DateTime<Utc>> {
-    let expiration_key_str = "/golem/srv/comp/expiration";
     let value = proposal
-        .pointer(expiration_key_str)
+        .pointer(AGREEMENT_EXPIRATION_PROPERTY)
         .ok_or_else(|| anyhow::anyhow!("Missing expiration key in Proposal"))?
         .clone();
     let timestamp: i64 = serde_json::from_value(value)?;
     Ok(Utc.timestamp_millis(timestamp))
 }
 
 fn debit_deadline_from(proposal: &ProposalView) -> Result<Option<Duration>> {
-    match proposal.pointer_typed::<i64>("/golem/com/payment/debit-notes/acceptance-timeout") {
+    match proposal.pointer_typed::<i64>(DEBIT_NOTE_ACCEPT_TIMEOUT_PROPERTY) {
         // Requestor is able to accept DebitNotes, because he set this property.
         Ok(deadline) => Ok(Some(Duration::seconds(deadline))),
         // If he didn't set this property, he is unable to accept DebitNotes.
@@ -134,7 +145,7 @@ impl NegotiatorComponent for LimitExpiration {
                     // Requestor proposed better deadline, than we required.
                     // We are expected to set property to the same value if we agree.
                     let deadline_prop = offer
-                        .pointer_mut("/golem/com/payment/debit-notes/acceptance-timeout")
+                        .pointer_mut(DEBIT_NOTE_ACCEPT_TIMEOUT_PROPERTY)
                         .unwrap();
                     *deadline_prop = serde_json::Value::Number(req_deadline.num_seconds().into());
 
@@ -145,7 +156,7 @@ impl NegotiatorComponent for LimitExpiration {
             // Requestor doesn't support DebitNotes acceptance, so we should
             // remove our property from Proposal to match with his.
             (None, Some(_)) => {
-                offer.remove_property("/golem/com/payment/debit-notes/acceptance-timeout")?;
+                offer.remove_property(DEBIT_NOTE_ACCEPT_TIMEOUT_PROPERTY)?;
                 NegotiationResult::Negotiating { offer }
             }
             // We agree with Requestor, that he won't accept DebitNotes.
@@ -156,7 +167,7 @@ impl NegotiatorComponent for LimitExpiration {
 
     fn fill_template(&mut self, mut template: OfferDefinition) -> anyhow::Result<OfferDefinition> {
         template.offer.set_property(
-            "golem.com.payment.debit-notes.acceptance-timeout",
+            DEBIT_NOTE_ACCEPT_TIMEOUT_PROPERTY_FLAT,
             serde_json::Value::Number(self.payment_deadline.num_seconds().into()),
         );
         Ok(template)
@@ -233,8 +244,8 @@ mod test_expiration_negotiator {
             .to_proposal();
 
         let proposal = properties_to_proposal(serde_json::json!({
-            "golem.srv.comp.expiration": (Utc::now() + Duration::minutes(15)).timestamp_millis(),
-            "golem.com.payment.debit-notes.acceptance-timeout": 50,
+            AGREEMENT_EXPIRATION_PROPERTY_FLAT: (Utc::now() + Duration::minutes(15)).timestamp_millis(),
+            DEBIT_NOTE_ACCEPT_TIMEOUT_PROPERTY_FLAT: 50,
         }));
 
         match negotiator
@@ -264,8 +275,8 @@ mod test_expiration_negotiator {
             .to_proposal();
 
         let proposal = properties_to_proposal(serde_json::json!({
-            "golem.srv.comp.expiration": (Utc::now() + Duration::minutes(7)).timestamp_millis(),
-            "golem.com.payment.debit-notes.acceptance-timeout": 130,
+            AGREEMENT_EXPIRATION_PROPERTY_FLAT: (Utc::now() + Duration::minutes(7)).timestamp_millis(),
+            DEBIT_NOTE_ACCEPT_TIMEOUT_PROPERTY_FLAT: 130,
         }));
 
         match negotiator
@@ -295,8 +306,8 @@ mod test_expiration_negotiator {
             .to_proposal();
 
         let proposal = properties_to_proposal(serde_json::json!({
-            "golem.srv.comp.expiration": (Utc::now() + Duration::minutes(7)).timestamp_millis(),
-            "golem.com.payment.debit-notes.acceptance-timeout": 120,
+            AGREEMENT_EXPIRATION_PROPERTY_FLAT: (Utc::now() + Duration::minutes(7)).timestamp_millis(),
+            DEBIT_NOTE_ACCEPT_TIMEOUT_PROPERTY_FLAT: 120,
         }));
 
         match negotiator
@@ -328,7 +339,7 @@ mod test_expiration_negotiator {
             .to_proposal();
 
         let proposal = properties_to_proposal(serde_json::json!({
-            "golem.srv.comp.expiration": (Utc::now() + Duration::minutes(15)).timestamp_millis(),
+            AGREEMENT_EXPIRATION_PROPERTY_FLAT: (Utc::now() + Duration::minutes(15)).timestamp_millis(),
         }));
 
         match negotiator
@@ -357,7 +368,7 @@ mod test_expiration_negotiator {
             .to_proposal();
 
         let proposal = properties_to_proposal(serde_json::json!({
-            "golem.srv.comp.expiration": (Utc::now() + Duration::minutes(7)).timestamp_millis(),
+            AGREEMENT_EXPIRATION_PROPERTY_FLAT: (Utc::now() + Duration::minutes(7)).timestamp_millis(),
         }));
 
         match negotiator

diff --git a/agent/provider/src/payments/model.rs b/agent/provider/src/payments/model.rs
@@ -4,6 +4,8 @@ use std::time::Duration;
 
 use ya_agreement_utils::{AgreementView, Error};
 
+use crate::market::negotiator::builtin::expiration::DEBIT_NOTE_ACCEPT_TIMEOUT_PROPERTY;
+
 /// Implementation of payment model which knows, how to compute amount
 /// of money, that requestor should pay for computations.
 pub trait PaymentModel {
@@ -33,9 +35,10 @@ impl<'a> PaymentDescription<'a> {
     }
 
     pub fn get_debit_note_deadline(&self) -> Result<Option<chrono::Duration>> {
-        match self.agreement.pointer_typed::<i64>(
-            "/demand/properties/golem/com/payment/debit-notes/acceptance-timeout",
-        ) {
+        match self
+            .agreement
+            .pointer_typed::<i64>(DEBIT_NOTE_ACCEPT_TIMEOUT_PROPERTY)
+        {
             Ok(deadline) => Ok(Some(chrono::Duration::seconds(deadline))),
             Err(Error::NoKey(_)) => Ok(None),
             Err(e) => Err(e.into()),