Repository: amp-labs/connectors
Pull request #43 (2024-01-02 21:19:16)

[ENG-518] refactor: Replace XMLData struct to xmldom package. 
Replacing custom created xmlData struct to xmldom library. 

There is issue/bug with the library which is when parsing xml string into the struct, it omits namespaces in attributes. However, the only place we use that is in test runs to create xmldom.Node object, just for test run purpose. 

```
<?xml version="1.0" encoding="UTF-8"?><soapenv:Envelope xmlns:soapenv="http://schemas.xmlsoap.org/soap/envelope/" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"><soapenv:Header xmlns="http://soap.sforce.com/2006/04/metadata"><AllOrNoneHeader><allOrNone>true</allOrNone></AllOrNoneHeader><SessionHeader><sessionId>00DDp000000JQ4L!ASAAQOszeyj5XanytfgLLuflNdcTWzijpNIV4bezmW9e0Nnm07UrduwdpiU7MXa6BqOXRP9s0Pq1L.2BE__U.7.F7PpPA37Q</sessionId></SessionHeader></soapenv:Header><soapenv:Body xmlns="http://soap.sforce.com/2006/04/metadata"><createMetadata><metadata xsi:type="CustomObject"><fullName>TestObject15__c</fullName><label>Test Object 15</label><pluralLabel>Test Objects 15</pluralLabel><nameField><type>Text</type><label>Test Object Name</label></nameField><deploymentStatus>Deployed</deploymentStatus><sharingModel>ReadWrite</sharingModel></metadata><metadata xsi:type="CustomField"><fullName>TestObject13__c.Comments__c</fullName><label>Comments</label><type>LongTextArea</type><length>500</length><inlineHelpText>This field contains help text for this object</inlineHelpText><description>Add your comments about this object here</description><visibleLines>30</visibleLines><required>false</required><trackFeedHistory>false</trackFeedHistory><trackHistory>false</trackHistory></metadata></createMetadata></soapenv:Body></soapenv:Envelope>
```

```
Field Operation Result <?xml version="1.0" encoding="UTF-8"?><soapenv:Envelope xmlns:soapenv="http://schemas.xmlsoap.org/soap/envelope/" xmlns="http://soap.sforce.com/2006/04/metadata"><soapenv:Body><createMetadataResponse><result><errors><fields>DeveloperName</fields><message>That object name is already in use.</message><statusCode>DUPLICATE_DEVELOPER_NAME</statusCode></errors><fullName>TestObject15__c</fullName><success>false</success></result><result><errors><fields>DeveloperName</fields><message>There is already a field named Comments on Test Object 13.</message><statusCode>DUPLICATE_DEVELOPER_NAME</statusCode></errors><fullName>TestObject13__c.Comments__c</fullName><success>false</success></result></createMetadataResponse></soapenv:Body></soapenv:Envelope>
```


jlimatampersand: Current dependencies on/for this PR:
* `main`
  * **PR #43** <a href="https://app.graphite.dev/github/pr/amp-labs/connectors/43?utm_source=stack-comment-icon" target="_blank"><img src="https://static.graphite.dev/graphite-32x32-black.png" alt="Graphite" width="10px" height="10px"/></a>  👈

This [stack of pull requests](https://stacking.dev/?utm_source=stack-comment) is managed by [Graphite](https://app.graphite.dev/github/pr/amp-labs/connectors/43?utm_source=stack-comment).
codesee-maps[bot]: <details open>
<summary><b>👇 <a href="https://app.codesee.io/r/reviews?pr=43&src=https%3A%2F%2Fgithub.com%2Famp-labs%2Fconnectors">Click on the image</a> for a new way to code review</b>
</summary>

####

[![Review these changes using an interactive CodeSee Map](https://s3.us-east-2.amazonaws.com/maps.codesee.io/images/github/amp-labs/connectors/43/67dd7a30/d1ddb2b5bd79d32c252b3403fca36e0d7a451475.svg)](https://app.codesee.io/r/reviews?pr=43&src=https%3A%2F%2Fgithub.com%2Famp-labs%2Fconnectors)

#### Legend
<img src="https://app.codesee.io/Legend_v3.png" alt="CodeSee Map legend" width="220" height="156">
</details>

Patch:
diff --git a/go.mod b/go.mod
index 113c3857..ddb6ab96 100644
--- a/go.mod
+++ b/go.mod
@@ -8,10 +8,12 @@ require (
 	github.com/joho/godotenv v1.5.1
 	github.com/spyzhov/ajson v0.9.0
 	github.com/stretchr/testify v1.8.4
+	github.com/subchen/go-xmldom v1.1.2
 	golang.org/x/oauth2 v0.15.0
 )
 
 require (
+	github.com/antchfx/xpath v0.0.0-20170515025933-1f3266e77307 // indirect
 	github.com/go-playground/locales v0.14.1 // indirect
 	github.com/go-playground/universal-translator v0.18.1 // indirect
 	github.com/google/go-cmp v0.5.9 // indirect
diff --git a/go.sum b/go.sum
index f7e958e7..3ad9224c 100644
--- a/go.sum
+++ b/go.sum
@@ -1,3 +1,5 @@
+github.com/antchfx/xpath v0.0.0-20170515025933-1f3266e77307 h1:C735MoY/X+UOx6SECmHk5pVOj51h839Ph13pEoY8UmU=
+github.com/antchfx/xpath v0.0.0-20170515025933-1f3266e77307/go.mod h1:Yee4kTMuNiPYJ7nSNorELQMr1J33uOpXDMByNYhvtNk=
 github.com/brianvoe/gofakeit/v6 v6.26.3 h1:3ljYrjPwsUNAUFdUIr2jVg5EhKdcke/ZLop7uVg1Er8=
 github.com/brianvoe/gofakeit/v6 v6.26.3/go.mod h1:Xj58BMSnFqcn/fAQeSK+/PLtC5kSb7FJIq4JyGa8vEs=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
@@ -32,6 +34,8 @@ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO
 github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
 github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
 github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
+github.com/subchen/go-xmldom v1.1.2 h1:7evI2YqfYYOnuj+PBwyaOZZYjl3iWq35P6KfBUw9jeU=
+github.com/subchen/go-xmldom v1.1.2/go.mod h1:6Pg/HuX5/T4Jlj0IPJF1sRxKVoI/rrKP6LIMge9d5/8=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks=
 golang.org/x/net v0.19.0 h1:zTwKpTd2XuCqf8huc7Fo2iSy+4RHPd10s4KzeTnVr1c=
diff --git a/hubspot/search.go b/hubspot/search.go
index ae719e75..159aee29 100644
--- a/hubspot/search.go
+++ b/hubspot/search.go
@@ -38,6 +38,8 @@ func (c *Connector) Search(ctx context.Context, config SearchParams) (*common.Re
 // If the time is zero, it returns an empty filter. For contacts, it uses the
 // lastmodifieddate field. For other objects, it uses the hs_lastmodifieddate.
 // Read more: https://community.hubspot.com/t5/APIs-Integrations/CRM-V3-API-Search-issue-with-Contacts-when-using-Filters/m-p/324617
+//
+//nolint:lll
 func BuildLastModifiedFilterGroup(params *common.ReadParams) Filter {
 	if params.Since.IsZero() {
 		return Filter{}
diff --git a/salesforce/metadataApi.go b/salesforce/metadataApi.go
index 992d3603..1cffca2a 100644
--- a/salesforce/metadataApi.go
+++ b/salesforce/metadataApi.go
@@ -12,6 +12,7 @@ import (
 	"strings"
 
 	"github.com/amp-labs/connectors/common"
+	"github.com/subchen/go-xmldom"
 	"golang.org/x/oauth2"
 )
 
@@ -23,14 +24,10 @@ var (
 
 func (c *Connector) CreateMetadata(
 	ctx context.Context,
-	metaDefinition *common.XMLData,
+	metadata *xmldom.Node,
 	tok *oauth2.Token,
 ) (string, error) {
-	if err := metaDefinition.Validate(); err != nil {
-		return "", errors.Join(ErrBadRequest, err)
-	}
-
-	req, err := c.prepareXMLRequest(ctx, metaDefinition, tok)
+	req, err := c.prepareXMLRequest(ctx, metadata, tok)
 	if err != nil {
 		return "", err
 	}
@@ -45,7 +42,7 @@ func (c *Connector) CreateMetadata(
 	// tok object will be updated with new token automatically after failing first call
 	// we simply make another call with updated token.
 	if res.StatusCode == 500 && strings.Contains(string(body), "INVALID_SESSION_ID") {
-		req, err := c.prepareXMLRequest(ctx, metaDefinition, tok)
+		req, err := c.prepareXMLRequest(ctx, metadata, tok)
 		if err != nil {
 			return "", errors.Join(ErrCreateMetadata, err)
 		}
@@ -66,10 +63,10 @@ func (c *Connector) CreateMetadata(
 
 func (c *Connector) prepareXMLRequest(
 	ctx context.Context,
-	operation *common.XMLData,
+	metadata *xmldom.Node,
 	tok *oauth2.Token,
 ) (*http.Request, error) {
-	data := preparePayload(operation, tok.AccessToken)
+	data := preparePayload(metadata, tok.AccessToken)
 
 	endPointURL, err := url.JoinPath(c.Client.Base, "services/Soap/m/"+APIVersionSOAP())
 	if err != nil {
@@ -119,56 +116,90 @@ func addSOAPHeaders(req *http.Request) {
 	req.Header.Set("SOAPAction", "''")
 }
 
-func getEnvelope(header string, body string) string {
-	return fmt.Sprintf(
-		`<?xml version="1.0" encoding="UTF-8"?>
-		<soapenv:Envelope
-			xmlns:soapenv="http://schemas.xmlsoap.org/soap/envelope/"
-			xmlns:xsd="http://www.w3.org/2001/XMLSchema"
-			xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
-			%s
-			%s
-		</soapenv:Envelope>`,
-		header,
-		body,
-	)
+func getEnvelope(header *xmldom.Node, body *xmldom.Node) *xmldom.Document {
+	envelop := xmldom.NewDocument("soapenv:Envelope")
+	envelop.Root.SetAttributeValue("xmlns:soapenv", "http://schemas.xmlsoap.org/soap/envelope/")
+	envelop.Root.SetAttributeValue("xmlns:xsd", "http://www.w3.org/2001/XMLSchema")
+	envelop.Root.SetAttributeValue("xmlns:xsi", "http://www.w3.org/2001/XMLSchema-instance")
+	envelop.Root.Children = []*xmldom.Node{header, body}
+
+	return envelop
 }
 
-func getHeader(headers []string) string {
-	return fmt.Sprintf(
-		`<soapenv:Header xmlns="http://soap.sforce.com/2006/04/metadata">
-			<AllOrNoneHeader>
-				<allOrNone>true</allOrNone>
-			</AllOrNoneHeader>
-			%s
-		</soapenv:Header>`, strings.Join(headers, ""))
+func getHeader(headers ...*xmldom.Node) *xmldom.Node {
+	header := &xmldom.Node{
+		Name:     "soapenv:Header",
+		Children: headers,
+	}
+	header.SetAttributeValue("xmlns", "http://soap.sforce.com/2006/04/metadata")
+
+	return header
 }
 
-func getSessionHeader(token string) string {
-	return fmt.Sprintf(
-		`<SessionHeader>
-		<sessionId>%s</sessionId>
-	</SessionHeader>`, token)
+func getSessionHeader(token string) *xmldom.Node {
+	sessionId := &xmldom.Node{
+		Name: "sessionId",
+		Text: token,
+	}
+	header := &xmldom.Node{
+		Name: "SessionHeader",
+		Children: []*xmldom.Node{
+			sessionId,
+		},
+	}
+
+	return header
 }
 
-func getBody(items []string) string {
-	return fmt.Sprintf(
-		`<soapenv:Body xmlns="http://soap.sforce.com/2006/04/metadata">
-			%s
-		</soapenv:Body>`, strings.Join(items, ""))
+func getBody(items ...*xmldom.Node) *xmldom.Node {
+	body := &xmldom.Node{
+		Name: "soapenv:Body",
+		Attributes: []*xmldom.Attribute{
+			{
+				Name:  "xmlns",
+				Value: "http://soap.sforce.com/2006/04/metadata",
+			},
+		},
+		Children: items,
+	}
+
+	return body
 }
 
-func formOperationXML(oper *common.XMLData) string {
-	return oper.String()
+func preparePayload(metadata *xmldom.Node, accessToken string) string {
+	sessionHeader := getSessionHeader(accessToken)
+	allOrNonHeader := getAllOrNoneHeader(true)
+	header := getHeader(allOrNonHeader, sessionHeader)
+	body := getBody(metadata)
+	envelop := getEnvelope(header, body)
+
+	return envelop.XML()
 }
 
-func preparePayload(oper *common.XMLData, accessToken string) string {
-	metadata := formOperationXML(oper)
-	header := getHeader([]string{getSessionHeader(accessToken)})
-	body := getBody([]string{metadata})
-	data := getEnvelope(header, body)
+type BoolString string
+
+const (
+	True  string = "true"
+	False string = "false"
+)
+
+func getAllOrNoneHeader(allOrNon bool) *xmldom.Node {
+	allOrNonText := False
+	if allOrNon {
+		allOrNonText = True
+	}
+
+	header := &xmldom.Node{
+		Name: "AllOrNoneHeader",
+		Children: []*xmldom.Node{
+			{
+				Name: "allOrNone",
+				Text: allOrNonText,
+			},
+		},
+	}
 
-	return data
+	return header
 }
 
 func GetTokenUpdater(tok *oauth2.Token) common.OAuthOption {


Test patch:
diff --git a/test/metadata/main.go b/test/metadata/main.go
index 1912933c..8fa43b6a 100644
--- a/test/metadata/main.go
+++ b/test/metadata/main.go
@@ -2,7 +2,6 @@ package main
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"log/slog"
 	"net/http"
@@ -13,6 +12,7 @@ import (
 	"github.com/amp-labs/connectors/common"
 	"github.com/amp-labs/connectors/salesforce"
 	"github.com/amp-labs/connectors/test"
+	"github.com/subchen/go-xmldom"
 	"golang.org/x/oauth2"
 )
 
@@ -65,35 +65,32 @@ func main() {
 		_ = sfc.Close()
 	}()
 
-	createObjectData, err := os.ReadFile("./metadata/testCreateCustomObject.json")
-	if err != nil {
-		slog.Error("Error opening testOperation.json", "error", err)
-		os.Exit(1)
-	}
-
-	if string(createObjectData) == "" {
-		slog.Error("Error opening testOperation.json", "error", err)
-		os.Exit(1)
-	}
-
-	var objectOperation *common.XMLData
+	example, err := xmldom.ParseXML(`<createMetadata><metadata xsi:type="CustomObject"><fullName>TestObject15__c</fullName><label>Test Object 15</label><pluralLabel>Test Objects 15</pluralLabel><nameField><type>Text</type><label>Test Object Name</label></nameField><deploymentStatus>Deployed</deploymentStatus><sharingModel>ReadWrite</sharingModel></metadata><metadata xsi:type="CustomField"><fullName>TestObject13__c.Comments__c</fullName><label>Comments</label><type>LongTextArea</type><length>500</length><inlineHelpText>This field contains help text for this object</inlineHelpText><description>Add your comments about this object here</description><visibleLines>30</visibleLines><required>false</required><trackFeedHistory>false</trackFeedHistory><trackHistory>false</trackHistory></metadata></createMetadata>`)
 
-	if err := json.Unmarshal(createObjectData, &objectOperation); err != nil {
-		slog.Error("Error marshalling testOperation.json", "error", err)
+	if err != nil {
+		slog.Error("err parsing", "error", err)
 		os.Exit(1)
 	}
 
-	fieldOperation := getCreateFieldOperation()
-
-	operation := &common.XMLData{
-		XMLName:     "createMetadata",
-		Children:    []common.XMLSchema{objectOperation, fieldOperation},
-		SelfClosing: false,
+	node := example.Root
+
+	// xmldom.ParseXML has known issue that namespace in attribute is not correctly parsed
+	// ex) xsi:type="CustomObject" is parsed as xsi="CustomObject"
+	// so we need to manually change the attribute name
+	// We are using this ParseXML only in this test runner to generate XML
+	// so we can safely implement this package with below modification
+	metadataList := node.FindByName("metadata")
+	for _, metadata := range metadataList {
+		for _, attr := range metadata.Attributes {
+			if attr.Name == "type" {
+				attr.Name = "xsi:type"
+			}
+		}
 	}
 
-	res, err := sfc.CreateMetadata(ctx, operation, tok)
+	res, err := sfc.CreateMetadata(ctx, node, tok)
 	if err != nil {
-		slog.Debug("err", "err", err)
+		slog.Error("err", "err", err)
 	}
 
 	fmt.Println("Field Operation Result", res)