Merge pull request #279 from zhang-accounting/278-app-panic-on-txn-wi…

…th-one-or-more-posting-without-amount

fix: skip the process for those invalid txn

.github/workflows/build-latest.yml

@@ -111,4 +111,4 @@ jobs:
         run: mkdir -p build
         working-directory: ./frontend
       - name: Clippy
-        run: cargo clippy --all-features --all-targets -- -D warnings -A clippy::empty_docs
+        run: cargo clippy --all-features --all-targets -- -D warnings -D clippy::dbg_macro -A clippy::empty_docs

bindings/python/src/domain.rs

@@ -57,7 +57,7 @@ impl CommodityDomain {
     }
     #[getter]
     pub fn rounding(&self) -> Option<String> {
-        self.0.rounding.clone()
+        Some(self.0.rounding.to_string())
     }
 
     pub fn __repr__(&self) -> String {

integration-tests/implicit-posting-test/main.zhang

@@ -0,0 +1,10 @@
+option "title" "My Accounting"
+option "operating_currency" "CNY"
+
+
+2023-12-02 "KFC" "VME50 Package"
+  Assets:BankCard
+
+2023-12-02 "KFC" "VME50 Package"
+  Assets:BankCard
+  Assets:BankCard

integration-tests/implicit-posting-test/validations.json

@@ -0,0 +1,23 @@
+[
+  {
+    "uri": "/api/store",
+    "validations": [
+      [
+        "$.data.errors.length()",
+        2
+      ],
+      [
+        "$.data.errors[0].error_type",
+        "TransactionCannotInferTradeAmount"
+      ],
+      [
+        "$.data.errors[1].error_type",
+        "TransactionHasMultipleImplicitPosting"
+      ],
+      [
+        "$.data.transactions",
+        {}
+      ]
+    ]
+  }
+]

zhang-ast/src/data.rs

@@ -1,6 +1,7 @@
 use std::collections::HashSet;
 use std::ops::{Div, Mul, Neg};
 
+use bigdecimal::{BigDecimal, One};
 use chrono::{DateTime, Datelike, NaiveDate, NaiveDateTime, TimeZone, Utc};
 use chrono_tz::Tz;
 use indexmap::IndexSet;
@@ -31,7 +32,7 @@ impl Date {
     }
     pub(crate) fn naive_datetime(&self) -> NaiveDateTime {
         match self {
-            Date::Date(date) => date.and_hms_opt(0, 0, 0).unwrap(),
+            Date::Date(date) => date.and_hms_opt(0, 0, 0).expect("cannot construct naive datetime from naive date"),
             Date::DateHour(date_hour) => *date_hour,
             Date::Datetime(datetime) => *datetime,
         }
@@ -152,19 +153,27 @@ impl<'a> TxnPosting<'a> {
     pub fn units(&self) -> Option<Amount> {
         self.posting.units.clone()
     }
+
     /// if cost is not specified, and it can be indicated from price. e.g.
     /// `Assets:Card 1 CNY @ 10 AAA` then cost `10 AAA` can be indicated from single price`@ 10 AAA`
     pub fn costs(&self) -> Option<Amount> {
         self.posting.cost.clone().or_else(|| {
             self.posting.price.as_ref().map(|price| match price {
                 SingleTotalPrice::Single(single_price) => single_price.clone(),
                 SingleTotalPrice::Total(total_price) => Amount::new(
-                    (&total_price.number).div(&self.posting.units.as_ref().unwrap().number),
+                    (&total_price.number).div(self.posting.units.as_ref().map(|it| &it.number).unwrap_or(&BigDecimal::one())),
                     total_price.currency.clone(),
                 ),
             })
         })
     }
+    /// trade amount means the amount used for other postings to calculate balance
+    /// 1. if `unit` is null, return null
+    /// 2. if `unit` is present,
+    /// 2.1 return `unit * cost`, if cost is present
+    /// 2.2 return `unit * single_price`, if single price is present
+    /// 2.3 return `total_price`, if total price is present
+    /// 2.4 return `unit`, if both cost and price are not present.
     pub fn trade_amount(&self) -> Option<Amount> {
         self.posting
             .units
@@ -179,15 +188,16 @@ impl<'a> TxnPosting<'a> {
             })
     }
 
+    /// infer the trade amount based on other postings, if it's trade amount is null
     pub fn infer_trade_amount(&self) -> Result<Amount, ErrorKind> {
         self.trade_amount().map(Ok).unwrap_or_else(|| {
+            // get other postings' trade amount
             let (trade_amount_postings, non_trade_amount_postings): (Vec<AmountLotPair>, Vec<AmountLotPair>) = self
                 .txn
                 .txn_postings()
                 .iter()
                 .map(|it| (it.trade_amount(), it.lots()))
                 .partition(|it| it.0.is_some());
-
             match non_trade_amount_postings.len() {
                 0 => unreachable!("txn should not have zero posting"),
                 1 => {
@@ -200,10 +210,16 @@ impl<'a> TxnPosting<'a> {
                             inventory.add_lot(trade_amount, info);
                         }
                     }
-                    if inventory.size() > 1 {
-                        Err(ErrorKind::TransactionCannotInferTradeAmount)
-                    } else {
-                        Ok(inventory.pop().unwrap().neg())
+                    match inventory.size() {
+                        0 => Err(ErrorKind::TransactionCannotInferTradeAmount),
+                        1 => {
+                            if let Some(inventory_balance) = inventory.pop() {
+                                Ok(inventory_balance.neg())
+                            } else {
+                                Err(ErrorKind::TransactionCannotInferTradeAmount)
+                            }
+                        }
+                        _ => Err(ErrorKind::TransactionExplicitPostingHaveMultipleCommodity),
                     }
                 }
                 _ => Err(ErrorKind::TransactionHasMultipleImplicitPosting),

zhang-ast/src/error.rs

@@ -1,6 +1,20 @@
-#[derive(Debug)]
+use serde::Serialize;
+use strum::Display;
+
+#[derive(Debug, Display, Clone, PartialEq, Eq, Serialize)]
 pub enum ErrorKind {
-    TransactionHasMultipleImplicitPosting,
+    UnbalancedTransaction,
     TransactionCannotInferTradeAmount,
+    TransactionHasMultipleImplicitPosting,
+    TransactionExplicitPostingHaveMultipleCommodity,
     InvalidFlag,
+
+    AccountBalanceCheckError,
+    AccountDoesNotExist,
+    AccountClosed,
+    TransactionDoesNotBalance,
+    CommodityDoesNotDefine,
+    CloseNonZeroAccount,
+
+    BudgetDoesNotExist,
 }

zhang-cli/src/github.rs

@@ -119,8 +119,8 @@ impl Accessor for GithubAccessor {
         .map_err(new_request_build_error)?;
 
         let resp = self.core.client.send(req).await?;
-        let status = dbg!(resp.status());
-        match dbg!(status) {
+        let status = resp.status();
+        match status {
             StatusCode::OK => parse_into_metadata(&path, resp.headers()).map(RpStat::new),
             StatusCode::NOT_FOUND => Err(opendal::Error::new(opendal::ErrorKind::NotFound, "not found")),
             StatusCode::FORBIDDEN => Err(opendal::Error::new(opendal::ErrorKind::PermissionDenied, "Forbidden")),
@@ -131,10 +131,10 @@ impl Accessor for GithubAccessor {
     async fn read(&self, path: &str, _args: OpRead) -> opendal::Result<(RpRead, Self::Reader)> {
         info!("read");
         let path = urlencoding::encode(path);
-        let req = Request::get(dbg!(format!(
+        let req = Request::get(format!(
             "https://api.github.com/repos/{}/{}/contents/{}",
             &self.core.user, &self.core.repo, &path
-        )))
+        ))
         .header("accept", "application/vnd.github.raw")
         .header("Authorization", &self.core.token)
         .header("X-GitHub-Api-Version", "2022-11-28")
@@ -143,7 +143,7 @@ impl Accessor for GithubAccessor {
         .map_err(new_request_build_error)?;
 
         let resp = self.core.client.send(req).await?;
-        let status = dbg!(resp.status());
+        let status = resp.status();
         match status {
             StatusCode::OK => {
                 let size = parse_content_length(resp.headers())?;
@@ -201,7 +201,7 @@ impl oio::OneShotWrite for GithubWriter {
 
         let resp = self.core.client.send(req).await?;
 
-        let status = dbg!(resp.status());
+        let status = resp.status();
 
         match status {
             StatusCode::CREATED | StatusCode::OK | StatusCode::NO_CONTENT => {
@@ -220,7 +220,7 @@ impl oio::OneShotWrite for GithubWriter {
 impl GithubCore {
     pub async fn stat(&self, path: &str) -> opendal::Result<Response<IncomingAsyncBody>> {
         let path = urlencoding::encode(path);
-        let req = Request::get(dbg!(format!("https://api.github.com/repos/{}/{}/contents/{}", &self.user, &self.repo, &path)))
+        let req = Request::get(format!("https://api.github.com/repos/{}/{}/contents/{}", &self.user, &self.repo, &path))
             .header("accept", "application/vnd.github.raw")
             .header("Authorization", &self.token)
             .header("X-GitHub-Api-Version", "2022-11-28")

zhang-core/src/data_source.rs

@@ -137,10 +137,9 @@ impl DataSource for LocalFileSystemDataSource {
                 continue;
             }
             let file_content = self.get(pathbuf.to_string_lossy().to_string())?;
-            //todo: remove utf8 string unwrap
             let entity_directives = self
                 .data_type
-                .transform(String::from_utf8(file_content).unwrap(), Some(pathbuf.to_string_lossy().to_string()))?;
+                .transform(String::from_utf8_lossy(&file_content).to_string(), Some(pathbuf.to_string_lossy().to_string()))?;
 
             entity_directives.iter().filter_map(|directive| self.go_next(directive)).for_each(|buf| {
                 let fullpath = if buf.starts_with('/') {

zhang-core/src/data_type/text/parser.rs

@@ -69,12 +69,12 @@ impl ZhangParser {
 
     fn number(input: Node) -> Result<BigDecimal> {
         let pure_number = input.as_str().replace([',', '_'], "");
-        Ok(BigDecimal::from_str(&pure_number).unwrap())
+        Ok(BigDecimal::from_str(&pure_number).expect("invalid number detect"))
     }
 
     fn quote_string(input: Node) -> Result<ZhangString> {
         let string = input.as_str();
-        Ok(ZhangString::QuoteString(unescape(string).unwrap()))
+        Ok(ZhangString::QuoteString(unescape(string).expect("string contains invalid escape char")))
     }
 
     fn unquote_string(input: Node) -> Result<ZhangString> {
@@ -103,7 +103,7 @@ impl ZhangParser {
 
         );
         Ok(Account {
-            account_type: AccountType::from_str(&r.0).unwrap(),
+            account_type: AccountType::from_str(&r.0).expect("invalid account type"),
             content: format!("{}:{}", &r.0, r.1.join(":")),
             components: r.1,
         })
@@ -118,14 +118,18 @@ impl ZhangParser {
     }
 
     fn date_only(input: Node) -> Result<Date> {
-        let date = NaiveDate::parse_from_str(input.as_str(), "%Y-%m-%d").unwrap();
+        let date = NaiveDate::parse_from_str(input.as_str(), "%Y-%m-%d").expect("cannot construct naive date");
         Ok(Date::Date(date))
     }
     fn datetime(input: Node) -> Result<Date> {
-        Ok(Date::Datetime(NaiveDateTime::parse_from_str(input.as_str(), "%Y-%m-%d %H:%M:%S").unwrap()))
+        Ok(Date::Datetime(
+            NaiveDateTime::parse_from_str(input.as_str(), "%Y-%m-%d %H:%M:%S").expect("cannot construct naive datetime"),
+        ))
     }
     fn date_hour(input: Node) -> Result<Date> {
-        Ok(Date::DateHour(NaiveDateTime::parse_from_str(input.as_str(), "%Y-%m-%d %H:%M").unwrap()))
+        Ok(Date::DateHour(
+            NaiveDateTime::parse_from_str(input.as_str(), "%Y-%m-%d %H:%M").expect("cannot construct naive date hour"),
+        ))
     }
 
     fn plugin(input: Node) -> Result<Directive> {
@@ -241,7 +245,7 @@ impl ZhangParser {
     }
 
     fn transaction_flag(input: Node) -> Result<Option<Flag>> {
-        Ok(Some(Flag::from_str(input.as_str().trim()).unwrap()))
+        Ok(Some(Flag::from_str(input.as_str().trim()).expect("cannot read node as str")))
     }
 
     fn posting_price(input: Node) -> Result<SingleTotalPrice> {