From a02c0bfac885f8b8373a9d39e782305c2ccafc90 Mon Sep 17 00:00:00 2001
From: Evgenii Stratonikov <e.stratonikov@yadro.com>
Date: Thu, 26 Oct 2023 13:15:40 +0300
Subject: [PATCH] [#186] netmap: Marshal policy with brackets

Brackets can be semantically important and must not be omitted,
otherwise the output is plain wrong.
We do not take the responsibility to preserve every bracket, though,
because parser does some optimizations related to grouping long chains
of filters combined with the same operation.

Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
---
 netmap/policy.go      | 21 +++++++++---
 netmap/policy_test.go | 74 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 4 deletions(-)

diff --git a/netmap/policy.go b/netmap/policy.go
index 8826512..52cee8f 100644
--- a/netmap/policy.go
+++ b/netmap/policy.go
@@ -455,7 +455,7 @@ func (p PlacementPolicy) WriteStringTo(w io.StringWriter) (err error) {
 			return err
 		}
 
-		err = writeFilterStringTo(w, p.filters[i])
+		err = writeFilterStringTo(w, p.filters[i], false)
 		if err != nil {
 			return err
 		}
@@ -464,7 +464,7 @@ func (p PlacementPolicy) WriteStringTo(w io.StringWriter) (err error) {
 	return nil
 }
 
-func writeFilterStringTo(w io.StringWriter, f netmap.Filter) error {
+func writeFilterStringTo(w io.StringWriter, f netmap.Filter, mayNeedOuterBrackets bool) error {
 	var err error
 	var s string
 	op := f.GetOp()
@@ -489,7 +489,7 @@ func writeFilterStringTo(w io.StringWriter, f netmap.Filter) error {
 		if err != nil {
 			return err
 		}
-		err = writeFilterStringTo(w, inner[0])
+		err = writeFilterStringTo(w, inner[0], false)
 		if err != nil {
 			return err
 		}
@@ -498,6 +498,13 @@ func writeFilterStringTo(w io.StringWriter, f netmap.Filter) error {
 			return err
 		}
 	} else {
+		useBrackets := mayNeedOuterBrackets && op == netmap.OR && len(inner) > 1
+		if useBrackets {
+			_, err = w.WriteString("(")
+			if err != nil {
+				return err
+			}
+		}
 		for i := range inner {
 			if i != 0 {
 				_, err = w.WriteString(" " + op.String() + " ")
@@ -505,7 +512,13 @@ func writeFilterStringTo(w io.StringWriter, f netmap.Filter) error {
 					return err
 				}
 			}
-			err = writeFilterStringTo(w, inner[i])
+			err = writeFilterStringTo(w, inner[i], true)
+			if err != nil {
+				return err
+			}
+		}
+		if useBrackets {
+			_, err = w.WriteString(")")
 			if err != nil {
 				return err
 			}
diff --git a/netmap/policy_test.go b/netmap/policy_test.go
index 05b0c98..81b7b48 100644
--- a/netmap/policy_test.go
+++ b/netmap/policy_test.go
@@ -1,6 +1,7 @@
 package netmap_test
 
 import (
+	"strings"
 	"testing"
 
 	. "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/netmap"
@@ -29,6 +30,79 @@ func TestPlacementPolicyEncoding(t *testing.T) {
 	})
 }
 
+func TestPlacementPolicyWriteString(t *testing.T) {
+	var testCases = []struct {
+		name   string
+		input  string
+		output string // If the output is empty, make it equal to input.
+	}{
+		{
+			name: "no compound operators",
+			input: `REP 1
+CBF 1
+SELECT 1 FROM Color
+FILTER Color EQ Red AS Color`,
+		},
+		{
+			name: "no brackets in single level same-operator chain",
+			input: `REP 1
+CBF 1
+SELECT 1 FROM Color
+FILTER Color EQ Red OR Color EQ Blue OR Color EQ Green AS Color`,
+		},
+		{
+			name: "no brackets aroung higher precedence op",
+			input: `REP 1
+CBF 1
+SELECT 1 FROM Color
+FILTER Color EQ Red OR Color EQ Blue AND Color NE Green AS Color`,
+		},
+		{
+			name: "no brackets aroung higher precedence op, even if present in the input",
+			input: `REP 1
+CBF 1
+SELECT 1 FROM Color
+FILTER Color EQ Red OR (Color EQ Blue AND Color NE Green) AS Color`,
+			output: `REP 1
+CBF 1
+SELECT 1 FROM Color
+FILTER Color EQ Red OR Color EQ Blue AND Color NE Green AS Color`,
+		},
+		{
+			name: "brackets aroung lower precedence op",
+			input: `REP 1
+CBF 1
+SELECT 1 FROM Color
+FILTER (Color EQ Red OR Color EQ Blue) AND Color NE Green AS Color`,
+		},
+		{
+			name: "no extra brackets for bracketed same-operator chain",
+			input: `REP 1
+CBF 1
+SELECT 1 FROM Color
+FILTER (Color EQ Red OR Color EQ Blue OR Color EQ Yellow) AND Color NE Green AS Color`,
+		},
+	}
+
+	for _, tc := range testCases {
+		var p PlacementPolicy
+		require.NoError(t, p.DecodeString(tc.input))
+
+		var sb strings.Builder
+		require.NoError(t, p.WriteStringTo(&sb))
+
+		if tc.output == "" {
+			require.Equal(t, tc.input, sb.String())
+		} else {
+			require.Equal(t, tc.output, sb.String())
+
+			var p1 PlacementPolicy
+			require.NoError(t, p1.DecodeString(tc.output))
+			require.Equal(t, p, p1)
+		}
+	}
+}
+
 func TestDecodeSelectFilterExpr(t *testing.T) {
 	for _, s := range []string{
 		"SELECT 1 FROM *",