From 1f2073c0f162325475ff9ea0bc5aac8614fd1871 Mon Sep 17 00:00:00 2001
From: ALI OMAR Ismail <73675505+Ismail731404@users.noreply.github.com>
Date: Wed, 25 Oct 2023 16:14:56 +0200
Subject: [PATCH] Fixed improperly merged sections and added unit tests (#134)

* Fixed improperly merged sections, added unit tests for testable functions, and translated French comments to English.

* Add error handling for last evaluation is unknown scenario.

* Update situation_history_querier.go

remove spaces

* Update situation_history_querier.go

* Update fact_history_builder_test.go

remove \n
---
 internals/history/fact_history_builder.go     |  3 -
 .../history/fact_history_builder_test.go      | 99 +++++++++++++------
 internals/history/fact_history_querier.go     | 58 ++++++-----
 internals/history/service.go                  |  4 +-
 .../history/situation_history_builder_test.go | 25 +++++
 .../history/situation_history_querier.go      | 19 ++--
 internals/history/utils.go                    |  1 +
 internals/history/utils_test.go               | 27 +++++
 internals/scheduler/compact_history_job.go    |  4 +-
 internals/scheduler/fact_calculation_job.go   |  2 +-
 internals/scheduler/fact_recalculation_job.go |  2 +-
 internals/scheduler/job.go                    |  2 +-
 internals/scheduler/purge_history_job.go      |  1 -
 internals/scheduler/util_test.go              | 35 ++++++-
 14 files changed, 200 insertions(+), 82 deletions(-)

diff --git a/internals/history/fact_history_builder.go b/internals/history/fact_history_builder.go
index 7a0b3d25..db92f582 100644
--- a/internals/history/fact_history_builder.go
+++ b/internals/history/fact_history_builder.go
@@ -1,8 +1,6 @@
 package history
 
 import (
-	"time"
-
 	sq "github.com/Masterminds/squirrel"
 )
 
@@ -78,4 +76,3 @@ func (builder HistoryFactsBuilder) GetTodaysFactResultByParameters(param ParamGe
 		Where(sq.Expr("ts >= ?::timestamptz", todayStart)).
 		Where(sq.Expr("ts < ?::timestamptz", tomorrowStart))
 }
-
diff --git a/internals/history/fact_history_builder_test.go b/internals/history/fact_history_builder_test.go
index 22a607b3..bec4bc71 100644
--- a/internals/history/fact_history_builder_test.go
+++ b/internals/history/fact_history_builder_test.go
@@ -1,6 +1,7 @@
 package history
 
 import (
+	"reflect"
 	"testing"
 	"time"
 
@@ -31,7 +32,7 @@ func TestGetHistoryFacts(t *testing.T) {
 
 func initialiseDB() (*sqlx.DB, error) {
 
-    credentials := postgres.Credentials{
+	credentials := postgres.Credentials{
 		URL:      "localhost",
 		Port:     "5432",
 		DbName:   "postgres",
@@ -46,12 +47,12 @@ func initialiseDB() (*sqlx.DB, error) {
 	return db, nil
 }
 func TestGetByCriteria(t *testing.T) {
-    
+
 	// Initiate DB connection
 	db, err := initialiseDB()
 
 	if err != nil {
-		t.Fatalf("Error initializing DB: %s",err)
+		t.Fatalf("Error initializing DB: %s", err)
 	}
 
 	defer db.Close()
@@ -61,41 +62,41 @@ func TestGetByCriteria(t *testing.T) {
 		conn:    db,
 	}
 
-	param := ParamGetFactHistory {
-		FactID: 10000000,
-		SituationID: 10000000,
+	param := ParamGetFactHistory{
+		FactID:              10000000,
+		SituationID:         10000000,
 		SituationInstanceID: 10000000,
 	}
 
 	value := 44
-    historyItem := HistoryFactsV4{
-        FactID: param.FactID,
-        SituationID: param.SituationID,
-        SituationInstanceID: param.SituationInstanceID,
-        Ts: time.Now(),
+	historyItem := HistoryFactsV4{
+		FactID:              param.FactID,
+		SituationID:         param.SituationID,
+		SituationInstanceID: param.SituationInstanceID,
+		Ts:                  time.Now(),
 		Result: reader.Item{
 			Aggs: map[string]*reader.ItemAgg{
 				"count": {
-					Value: value,  
+					Value: value,
 				},
 			},
 		},
-    }
-
-    insertedID, err := querier.Insert(historyItem)
-    if err != nil {
-        t.Fatalf("Error inserting: %s", err)
-    }
-    if insertedID <= 0 {
-        t.Fatalf("Invalid ID returned after insert")
-    }
-	
+	}
+
+	insertedID, err := querier.Insert(historyItem)
+	if err != nil {
+		t.Fatalf("Error inserting: %s", err)
+	}
+	if insertedID <= 0 {
+		t.Fatalf("Invalid ID returned after insert")
+	}
+
 	defer func() {
-        err := querier.Delete(insertedID)
-        if err != nil {
-            t.Fatalf("Error cleaning up: %s", err)
-        }
-    }()
+		err := querier.Delete(insertedID)
+		if err != nil {
+			t.Fatalf("Error cleaning up: %s", err)
+		}
+	}()
 
 	results, err := querier.GetTodaysFactResultByParameters(param)
 	if err != nil {
@@ -103,11 +104,45 @@ func TestGetByCriteria(t *testing.T) {
 	}
 
 	if len(results.Results) != 1 {
-        t.Fatalf("Expected 1 result, got %d and resulat : %v ", len(results.Results),results)
-    }
+		t.Fatalf("Expected 1 result, got %d and resulat : %v ", len(results.Results), results)
+	}
 
-    if results.Results[0].Value != int64(value) {
-        t.Fatalf("Retrieved ID does not match inserted ID")
-    }
+	if results.Results[0].Value != int64(value) {
+		t.Fatalf("Retrieved ID does not match inserted ID")
+	}
 }
 
+func TestGetTodaysFactResultByParameters(t *testing.T) {
+	builder := HistoryFactsBuilder{}
+
+	param := ParamGetFactHistory{
+		FactID:              123,
+		SituationID:         456,
+		SituationInstanceID: 789,
+	}
+
+	expectedSQL := `SELECT result, ts FROM fact_history_v5 WHERE fact_id = $1 AND situation_id = $2 AND situation_instance_id = $3 AND ts >= $4::timestamptz AND ts < $5::timestamptz`
+
+	todayStart, tomorrowStart := getTodayTimeRange()
+
+	expectedArgs := []interface{}{
+		param.FactID,
+		param.SituationID,
+		param.SituationInstanceID,
+		todayStart,
+		tomorrowStart,
+	}
+
+	sql, args, err := builder.GetTodaysFactResultByParameters(param).ToSql()
+	if err != nil {
+		t.Fatalf("Failed to build SQL: %v", err)
+	}
+
+	if expectedSQL != sql {
+		t.Errorf("Expected SQL to be %s but got %s", expectedSQL, sql)
+	}
+
+	if !reflect.DeepEqual(expectedArgs, args) {
+		t.Errorf("Expected args to be %v, but got %v", expectedArgs, args)
+	}
+}
diff --git a/internals/history/fact_history_querier.go b/internals/history/fact_history_querier.go
index 5f99108a..e57f3723 100644
--- a/internals/history/fact_history_querier.go
+++ b/internals/history/fact_history_querier.go
@@ -33,8 +33,8 @@ type GetFactHistory struct {
 }
 
 type FactResult struct {
-	Value int64  `json:"value"`
-	Time  time.Duration  `json:"time"`
+	Value int64         `json:"value"`
+	Time  time.Duration `json:"time"`
 }
 
 type ParamGetFactHistory struct {
@@ -117,7 +117,6 @@ func (querier HistoryFactsQuerier) ExecDelete(builder sq.DeleteBuilder) error {
 	return nil
 }
 
-
 func (querier HistoryFactsQuerier) QueryReturning(builder sq.InsertBuilder) (int64, error) {
 	rows, err := builder.RunWith(querier.conn.DB).Query()
 	if err != nil {
@@ -201,30 +200,30 @@ func (querier HistoryFactsQuerier) scanFirst(rows *sql.Rows) (HistoryFactsV4, er
 }
 
 func (querier *HistoryFactsQuerier) QueryGetSpecificFields(builder sq.SelectBuilder) (GetFactHistory, error) {
-    rows, err := builder.RunWith(querier.conn).Query()
-    if err != nil {
-        return GetFactHistory{}, err
-    }
-    defer rows.Close()
-
-    var results []FactResult
-
-    for rows.Next() {
-        var resultBytes []byte
-        var ts time.Time
-        err = rows.Scan(&resultBytes, &ts)
-        if err != nil {
-            return GetFactHistory{}, err
-        }
-
-        var parsedResult map[string]map[string]map[string]int64
-        err = json.Unmarshal(resultBytes, &parsedResult)
-        if err != nil {
-            return GetFactHistory{}, err
-        }
-
-        duration := time.Duration(ts.Hour())*time.Hour + time.Duration(ts.Minute())*time.Minute + time.Duration(ts.Second())*time.Second
-        factRes := FactResult{Time: duration}
+	rows, err := builder.RunWith(querier.conn).Query()
+	if err != nil {
+		return GetFactHistory{}, err
+	}
+	defer rows.Close()
+
+	var results []FactResult
+
+	for rows.Next() {
+		var resultBytes []byte
+		var ts time.Time
+		err = rows.Scan(&resultBytes, &ts)
+		if err != nil {
+			return GetFactHistory{}, err
+		}
+
+		var parsedResult map[string]map[string]map[string]int64
+		err = json.Unmarshal(resultBytes, &parsedResult)
+		if err != nil {
+			return GetFactHistory{}, err
+		}
+
+		duration := time.Duration(ts.Hour())*time.Hour + time.Duration(ts.Minute())*time.Minute + time.Duration(ts.Second())*time.Second
+		factRes := FactResult{Time: duration}
 
 		if aggs, ok := parsedResult["aggs"]; ok {
 			for key := range aggs {
@@ -239,12 +238,11 @@ func (querier *HistoryFactsQuerier) QueryGetSpecificFields(builder sq.SelectBuil
 			}
 		}
 		results = append(results, factRes)
-    }
+	}
 
-    return GetFactHistory{Results: results}, nil
+	return GetFactHistory{Results: results}, nil
 }
 
-
 func (querier HistoryFactsQuerier) GetTodaysFactResultByParameters(param ParamGetFactHistory) (GetFactHistory, error) {
 	builder := querier.Builder.GetTodaysFactResultByParameters(param)
 	return querier.QueryGetSpecificFields(builder)
diff --git a/internals/history/service.go b/internals/history/service.go
index c7bbe4bb..6cc37154 100644
--- a/internals/history/service.go
+++ b/internals/history/service.go
@@ -6,8 +6,8 @@ import (
 
 	sq "github.com/Masterminds/squirrel"
 	"github.com/jmoiron/sqlx"
-	"github.com/myrteametrics/myrtea-engine-api/v5/internals/explainer/issues"
 	"github.com/myrteametrics/myrtea-engine-api/v5/internals/explainer/draft"
+	"github.com/myrteametrics/myrtea-engine-api/v5/internals/explainer/issues"
 )
 
 var (
@@ -123,7 +123,7 @@ func (service HistoryService) GetHistoryFactsFromSituationIds(historySituationsI
 
 func (service HistoryService) PurgeHistory(options GetHistorySituationsOptions) error {
 	return service.deleteHistoryPurge(
-		service.HistorySituationsQuerier.Builder.GetHistorySituationsIdsBase(options), options ,
+		service.HistorySituationsQuerier.Builder.GetHistorySituationsIdsBase(options), options,
 	)
 }
 
diff --git a/internals/history/situation_history_builder_test.go b/internals/history/situation_history_builder_test.go
index 7f5496ac..d1b9c458 100644
--- a/internals/history/situation_history_builder_test.go
+++ b/internals/history/situation_history_builder_test.go
@@ -1,6 +1,7 @@
 package history
 
 import (
+	"reflect"
 	"testing"
 	"time"
 )
@@ -78,3 +79,27 @@ func TestGetHistorySituationsDetails(t *testing.T) {
 	t.Fail()
 	t.Log(builder.ToSql())
 }
+
+func TestGetLatestHistorySituation(t *testing.T) {
+	builder := HistorySituationsBuilder{}
+
+	expectedSQL := "SELECT ts, metadatas FROM situation_history_v5 WHERE situation_id = $1 AND situation_instance_id = $2 AND ts >= $3::timestamptz ORDER BY ts DESC LIMIT 1"
+	expectedArgs := []interface{}{
+		int64(1),
+		int64(12345678),
+		getStartDate30DaysAgo(),
+	}
+
+	sql, args, err := builder.GetLatestHistorySituation(1, 12345678).ToSql()
+	if err != nil {
+		t.Fatalf("Failed to build SQL: %v", err)
+	}
+
+	if expectedSQL != sql {
+		t.Errorf("Expected SQL to be \n%s\n but got \n%s", expectedSQL, sql)
+	}
+
+	if !reflect.DeepEqual(expectedArgs, args) {
+		t.Errorf("Expected args to be %v, but got %v", expectedArgs, args)
+	}
+}
diff --git a/internals/history/situation_history_querier.go b/internals/history/situation_history_querier.go
index 9e6c4e8a..1f281752 100644
--- a/internals/history/situation_history_querier.go
+++ b/internals/history/situation_history_querier.go
@@ -275,16 +275,21 @@ func (querier *HistorySituationsQuerier) QueryGetFieldsTsMetadatas(ctx context.C
 		var metadatas []models.MetaData
 		var ts time.Time
 		var metadataBytes []byte
-		err = rows.Scan(&ts, &metadataBytes)
 
+		err = rows.Scan(&ts, &metadataBytes)
 		if err != nil {
 			return HistorySituationsV4{}, fmt.Errorf("row scanning failed: %w", err)
-		} else {
-			err = json.Unmarshal(metadataBytes, &metadatas)
-			if err != nil {
-				return HistorySituationsV4{}, fmt.Errorf("Warning: unable to unmarshal metadatas JSON: %v\n", err)
-			}
 		}
+
+		err = json.Unmarshal(metadataBytes, &metadatas)
+		if err != nil {
+			return HistorySituationsV4{}, fmt.Errorf("Warning: unable to unmarshal metadatas JSON: %v", err)
+		}
+
+		if len(metadatas) == 0 {
+			return HistorySituationsV4{}, fmt.Errorf("The latest evaluation of the situation is unknown.")
+		}
+
 		result = HistorySituationsV4{Metadatas: metadatas, Ts: ts}
 		break //LIMIT 1
 	}
@@ -303,7 +308,7 @@ func (querier HistorySituationsQuerier) GetLatestHistory(situationID int64, situ
 	results, err := querier.QueryGetFieldsTsMetadatas(ctx, selectBuilder)
 	if err != nil {
 		if errors.Is(err, context.DeadlineExceeded) {
-			return HistorySituationsV4{}, errors.New("Timeout Error: The request targeting the 'situation_history_v5' table timed out after 1 minute.")
+			return HistorySituationsV4{}, errors.New("Timeout Error: The request targeting the 'situation_history_v5' table timed out after 10 seconds.")
 		}
 		return HistorySituationsV4{}, err
 	}
diff --git a/internals/history/utils.go b/internals/history/utils.go
index d7ac95ee..782d19b1 100644
--- a/internals/history/utils.go
+++ b/internals/history/utils.go
@@ -187,6 +187,7 @@ func getTodayTimeRange() (string, string) {
 	todayStart := todayStartDate.Format("2006-01-02 15:04:05")
 	tomorrowStart := todayStartDate.Add(24 * time.Hour).Format("2006-01-02 15:04:05")
 	return todayStart, tomorrowStart
+}
 
 func getStartDate30DaysAgo() string {
 	now := time.Now().UTC()
diff --git a/internals/history/utils_test.go b/internals/history/utils_test.go
index e3f3c2c8..d0b48975 100644
--- a/internals/history/utils_test.go
+++ b/internals/history/utils_test.go
@@ -2,6 +2,7 @@ package history
 
 import (
 	"testing"
+	"time"
 
 	"github.com/myrteametrics/myrtea-engine-api/v5/internals/situation"
 	"github.com/myrteametrics/myrtea-sdk/v4/expression"
@@ -39,3 +40,29 @@ func TestEvaluateExpressionFactsChain(t *testing.T) {
 		t.Log(expressionFactsEvaluated)
 	}
 }
+
+func TestGetTodayTimeRange(t *testing.T) {
+	todayStart, tomorrowStart := getTodayTimeRange()
+
+	now := time.Now().UTC().Truncate(24 * time.Hour)
+	expectedTodayStart := now.Format("2006-01-02 15:04:05")
+	expectedTomorrowStart := now.Add(24 * time.Hour).Format("2006-01-02 15:04:05")
+
+	if todayStart != expectedTodayStart {
+		t.Errorf("Expected today's start to be %s, but got %s", expectedTodayStart, todayStart)
+	}
+
+	if tomorrowStart != expectedTomorrowStart {
+		t.Errorf("Expected tomorrow's start to be %s, but got %s", expectedTomorrowStart, tomorrowStart)
+	}
+}
+
+func TestGetStartDate30DaysAgo(t *testing.T) {
+	date30DaysAgo := getStartDate30DaysAgo()
+
+	expectedDate := time.Now().UTC().AddDate(0, 0, -30).Format("2006-01-02 15:04:05")
+
+	if date30DaysAgo != expectedDate {
+		t.Errorf("Expected date 30 days ago to be %s, but got %s", expectedDate, date30DaysAgo)
+	}
+}
diff --git a/internals/scheduler/compact_history_job.go b/internals/scheduler/compact_history_job.go
index 1d317435..6091fba3 100644
--- a/internals/scheduler/compact_history_job.go
+++ b/internals/scheduler/compact_history_job.go
@@ -67,14 +67,14 @@ func (job CompactHistoryJob) Run() {
 		S().RemoveRunningJob(job.ScheduleID)
 		return
 	}
-	
+
 	toOffsetDuration, err := parseDuration(job.ToOffset)
 	if err != nil {
 		zap.L().Info("Error parsing the Compact's FromOffset ", zap.Error(err), zap.Int64("idSchedule", job.ScheduleID))
 		S().RemoveRunningJob(job.ScheduleID)
 		return
 	}
-	
+
 	if toOffsetDuration < fromOffsetDuration {
 		zap.L().Info("the Compact's FromOffset Duration must be less than ToOffset duration ", zap.Error(err), zap.Int64("idSchedule", job.ScheduleID))
 		S().RemoveRunningJob(job.ScheduleID)
diff --git a/internals/scheduler/fact_calculation_job.go b/internals/scheduler/fact_calculation_job.go
index 555f77f7..c773caa7 100644
--- a/internals/scheduler/fact_calculation_job.go
+++ b/internals/scheduler/fact_calculation_job.go
@@ -599,7 +599,7 @@ func filterTaskBatch(situationsToUpdate map[string]history.HistoryRecordV4, situ
 						}
 					}
 				} else {
-					// rechercher parent in database
+					// search for the parent in the database
 					Parent, err := history.S().HistorySituationsQuerier.GetLatestHistory(int64(idSituationDependsOn), int64(idInstanceDependsOn))
 					if err != nil {
 						logDataRetrieval(false, idSituationDependsOn, idInstanceDependsOn, situation.SituationID, situation.SituationInstanceID, err, "")
diff --git a/internals/scheduler/fact_recalculation_job.go b/internals/scheduler/fact_recalculation_job.go
index 8107c854..824eed19 100644
--- a/internals/scheduler/fact_recalculation_job.go
+++ b/internals/scheduler/fact_recalculation_job.go
@@ -44,7 +44,7 @@ type FactRecalculationJob struct {
 	ScheduleID     int64   `json:"-"`
 }
 
-//ResolveFromAndTo resolves the expressions in parameters From and To
+// ResolveFromAndTo resolves the expressions in parameters From and To
 func (job *FactRecalculationJob) ResolveFromAndTo(t time.Time) (time.Time, time.Time, error) {
 
 	var from time.Time
diff --git a/internals/scheduler/job.go b/internals/scheduler/job.go
index baf32eb1..643eac0c 100644
--- a/internals/scheduler/job.go
+++ b/internals/scheduler/job.go
@@ -45,7 +45,7 @@ func (schedule *InternalSchedule) IsValid() (bool, error) {
 	if schedule.JobType == "" {
 		return false, errors.New("missing JobType")
 	}
-	if _, ok := jobTypes[schedule.JobType]; !ok{
+	if _, ok := jobTypes[schedule.JobType]; !ok {
 		return false, errors.New("invalid JobType")
 	}
 	if schedule.Job == nil {
diff --git a/internals/scheduler/purge_history_job.go b/internals/scheduler/purge_history_job.go
index e0f9ae35..f00511fe 100644
--- a/internals/scheduler/purge_history_job.go
+++ b/internals/scheduler/purge_history_job.go
@@ -45,7 +45,6 @@ func (job PurgeHistoryJob) Run() {
 		return
 	}
 
-
 	options := history.GetHistorySituationsOptions{
 		SituationID:         -1,
 		SituationInstanceID: -1,
diff --git a/internals/scheduler/util_test.go b/internals/scheduler/util_test.go
index 3c42f489..884b5c06 100644
--- a/internals/scheduler/util_test.go
+++ b/internals/scheduler/util_test.go
@@ -1,7 +1,5 @@
 package scheduler
 
-
-
 import (
 	"testing"
 	"time"
@@ -34,3 +32,36 @@ func TestParseDuration(t *testing.T) {
 		}
 	}
 }
+
+func TestGenerateKeyAndValues(t *testing.T) {
+
+	situation1 := map[string]string{
+		IDSituationDependsOn: "123",
+		IDInstanceDependsOn:  "456",
+	}
+	key, id1, id2, err := generateKeyAndValues(situation1)
+	if err != nil {
+		t.Fatalf("Expected no error, but got: %v", err)
+	}
+	if key != "123-456" {
+		t.Errorf("Expected key to be 123-456, but got: %s", key)
+	}
+	if id1 != 123 || id2 != 456 {
+		t.Errorf("Expected ids to be 123 and 456, but got: %d and %d", id1, id2)
+	}
+
+	situation2 := map[string]string{}
+	_, _, _, err = generateKeyAndValues(situation2)
+	if err == nil {
+		t.Error("Expected an error due to missing keys, but got none")
+	}
+
+	situation3 := map[string]string{
+		IDSituationDependsOn: "abc",
+		IDInstanceDependsOn:  "456",
+	}
+	_, _, _, err = generateKeyAndValues(situation3)
+	if err == nil {
+		t.Error("Expected an error due to non-int value, but got none")
+	}
+}