CHORE: Eliminate SetTargetTXTString() (#2409)

models/t_parse.go

@@ -13,25 +13,29 @@ import (
 // seems to quote them differently.
 //
 // Recommended calling convention:  Process the exceptions first, then use the
-// function for everything else.
+// PopulateFromString function for everything else.
 //
-//		  var err error
-//		  switch rType {
-//		  case "MX":
-//	       // MX priority in a separate field.
-//	       if err := rc.SetTargetMX(cr.Priority, target); err != nil {
-//	         return nil, fmt.Errorf("unparsable MX record received from cloudflare: %w", err)
-//	       }
-//		  case "TXT":
-//	       // TXT records are stored verbatim; no quoting/escaping to parse.
-//			  err = rc.SetTargetTXT(target)
-//	       // ProTip: Use rc.SetTargetTXTs(manystrings) if the API or parser returns a list of substrings.
-//		  default:
-//			  err = rec.PopulateFromString(rType, target, origin)
-//		  }
-//		  if err != nil {
-//			  return nil, fmt.Errorf("unparsable record received from CHANGE_TO_PROVDER_NAME: %w", err)
-//		  }
+//      rtype := FILL_IN_TYPE
+//     	var err error
+//      rc := &models.RecordConfig{Type: rtype}
+//      rc.SetLabelFromFQDN(FILL_IN_NAME, origin)
+//      rc.TTL = uint32(FILL_IN_TTL)
+//      rc.Original = FILL_IN_ORIGINAL // The raw data received from provider (if needed later)
+//     	switch rtype {
+//     	case "MX":
+//     		// MX priority in a separate field.
+//     		err = rc.SetTargetMX(cr.Priority, target)
+//     	case "TXT":
+//     		// TXT records are stored verbatim; no quoting/escaping to parse.
+//     		err = rc.SetTargetTXT(target)
+//     	default:
+//     		err = rc.PopulateFromString(rtype, target, origin)
+//     	}
+//     	if err != nil {
+//     		return nil, fmt.Errorf("unparsable record type=%q received from PROVDER_NAME: %w", rtype, err)
+//     	}
+//     	return rc, nil
+
 func (rc *RecordConfig) PopulateFromString(rtype, contents, origin string) error {
 	if rc.Type != "" && rc.Type != rtype {
 		panic(fmt.Errorf("assertion failed: rtype already set (%s) (%s)", rtype, rc.Type))
@@ -64,7 +68,10 @@ func (rc *RecordConfig) PopulateFromString(rtype, contents, origin string) error
 	case "SOA":
 		return rc.SetTargetSOAString(contents)
 	case "SPF", "TXT":
-		return rc.SetTargetTXTString(contents)
+		// Parsing the contents may be unexpected. If your provider gives you a
+		// string that needs no further parsing, special case TXT and use
+		// rc.SetTargetTXT(target) like in the example above.
+		return rc.SetTargetTXTs(ParseQuotedTxt(contents))
 	case "SRV":
 		return rc.SetTargetSRVString(contents)
 	case "SSHFP":

models/t_txt.go

@@ -152,34 +152,6 @@ func (rc *RecordConfig) GetTargetTXTJoined() string {
 	return strings.Join(rc.TxtStrings, "")
 }
 
-// SetTargetTXTString is like SetTargetTXTs but accepts one big string,
-// which is parsed into individual strings.
-// Ex:
-//
-//	foo             << 1 string
-//	foo bar         << 1 string
-//	"foo bar"       << 1 string
-//	"foo" "bar"     << 2 strings
-//	"f"oo" "bar"    << 2 strings, one has a quote in it
-//
-// BUG: This function doesn't handle escaped quotes ("like \" this").
-//
-// FIXME(tlim): This function is badly named. It obscures the fact
-// that the string is parsed for quotes and stores a list of strings.
-//
-// Deprecated: This function has a confusing name. Most providers API
-// return a single string, in which case you should use
-// SetTargetTXT(). If your provider returns multiple strings, use
-// SetTargetTXTs().  If your provider returns a single string that
-// must be parsed to extract the individual strings, use
-// SetTargetTXTfromRFC1035Quoted().  Sadly we have not figured out
-// an integration test that will fail if you chose the wrong function.
-// As a result, we recommend trying SetTargetTXT() before you try
-// SetTargetTXTfromRFC1035Quoted().
-func (rc *RecordConfig) SetTargetTXTString(s string) error {
-	return rc.SetTargetTXTs(ParseQuotedTxt(s))
-}
-
 // SetTargetTXTfromRFC1035Quoted parses a series of quoted strings
 // and sets .TxtStrings based on the result.
 // Note: Most APIs do notThis is rarely used. Try using SetTargetTXT() first.

providers/gcloud/gcloudProvider.go

@@ -300,8 +300,16 @@ func nativeToRecord(set *gdns.ResourceRecordSet, rec, origin string) (*models.Re
 	r := &models.RecordConfig{}
 	r.SetLabelFromFQDN(set.Name, origin)
 	r.TTL = uint32(set.Ttl)
-	if err := r.PopulateFromString(set.Type, rec, origin); err != nil {
-		return nil, fmt.Errorf("unparsable record received from GCLOUD: %w", err)
+	rtype := set.Type
+	var err error
+	switch rtype {
+	case "TXT":
+		err = r.SetTargetTXTs(models.ParseQuotedTxt(rec))
+	default:
+		err = r.PopulateFromString(rtype, rec, origin)
+	}
+	if err != nil {
+		return nil, fmt.Errorf("unparsable record %q received from GCLOUD: %w", rtype, err)
 	}
 	return r, nil
 }

providers/hedns/hednsProvider.go

@@ -367,6 +367,8 @@ func (c *hednsProvider) GetZoneRecords(domain string, meta map[string]string) (m
 			// Convert to TXT record as SPF is deprecated
 			rc.Type = "TXT"
 			fallthrough
+		case "TXT":
+			err = rc.SetTargetTXTs(models.ParseQuotedTxt(data))
 		default:
 			err = rc.PopulateFromString(rc.Type, data, domain)
 		}

providers/route53/route53Provider.go

@@ -651,7 +651,7 @@ func nativeToRecords(set r53Types.ResourceRecordSet, origin string) ([]*models.R
 				rtype = "TXT"
 				fallthrough
 			default:
-				ty := string(rtype)
+				rtypeString := string(rtype)
 				val := *rec.Value
 
 				// AWS Route53 has a bug.  Sometimes it returns a target
@@ -673,18 +673,26 @@ func nativeToRecords(set r53Types.ResourceRecordSet, origin string) ([]*models.R
 				// first n pushes. It will seem odd but this is AWS's bug.
 				// The UPSERT command only fixes the first record, even if
 				// the UPSET received a list of corrections.
-				if ty == "CNAME" || ty == "MX" {
+				if rtypeString == "CNAME" || rtypeString == "MX" {
 					if !strings.HasSuffix(val, ".") {
 						val = val + "."
 					}
 				}
 
+				var err error
 				rc := &models.RecordConfig{TTL: uint32(aws.ToInt64(set.TTL))}
 				rc.SetLabelFromFQDN(unescape(set.Name), origin)
-				if err := rc.PopulateFromString(string(rtype), val, origin); err != nil {
-					return nil, fmt.Errorf("unparsable record received from R53: %w", err)
-				}
 				rc.Original = set
+				switch rtypeString {
+				case "TXT":
+					err = rc.SetTargetTXTs(models.ParseQuotedTxt(val))
+				default:
+					err = rc.PopulateFromString(rtypeString, val, origin)
+				}
+				if err != nil {
+					return nil, fmt.Errorf("unparsable record type=%q received from ROUTE53: %w", rtypeString, err)
+				}
+
 				results = append(results, rc)
 			}
 		}