[review fix]

1. fix the issues commented in review.

Author: brickzzhang

examples/tencentcloud-mongodb/main.tf

@@ -18,7 +18,7 @@ resource "tencentcloud_mongodb_instance" "mongodb" {
   password       = "test1234"
 
   tags = {
-    "test" = "test"
+    test = "test"
   }
 }
 
@@ -35,15 +35,15 @@ resource "tencentcloud_mongodb_sharding_instance" "mongodb_sharding" {
   password        = "test1234"
 
   tags = {
-    "test" = "test"
+    test = "test"
   }
 }
 
 data "tencentcloud_mongodb_instances" "mongodb_instances" {
   instance_id = tencentcloud_mongodb_instance.mongodb.id
 
   tags = {
-    "test" = "test"
+    test = "test"
   }
 }
 
@@ -60,6 +60,6 @@ resource "tencentcloud_mongodb_standby_instance" "mongodb" {
   prepaid_period         = 1
 
   tags = {
-    "test" = "test"
+    test = "test"
   }
 }

tencentcloud/extension_mongodb.go

@@ -41,8 +41,8 @@ const (
 )
 
 const (
-	MONGODB_CHARGE_TYPE_POSTPAID = "POSTPAID"
-	MONGODB_CHARGE_TYPE_PREPAID  = "PREPAID"
+	MONGODB_CHARGE_TYPE_POSTPAID = COMMON_PAYTYPE_POSTPAID
+	MONGODB_CHARGE_TYPE_PREPAID  = COMMON_PAYTYPE_PREPAID
 )
 
 var MONGODB_CHARGE_TYPE = map[uint64]string{
@@ -124,7 +124,7 @@ func TencentMongodbBasicInfo() map[string]*schema.Schema {
 			Type:        schema.TypeString,
 			Optional:    true,
 			ForceNew:    true,
-			Description: "ID of the subnet within this VPC. The vaule is required if VpcId is set.",
+			Description: "ID of the subnet within this VPC. The value is required if `vpc_id` is set.",
 		},
 		"project_id": {
 			Type:        schema.TypeInt,
@@ -162,7 +162,7 @@ func TencentMongodbBasicInfo() map[string]*schema.Schema {
 			ForceNew:     true,
 			Default:      MONGODB_CHARGE_TYPE_POSTPAID,
 			ValidateFunc: validateAllowedStringValue([]string{MONGODB_CHARGE_TYPE_POSTPAID, MONGODB_CHARGE_TYPE_PREPAID}),
-			Description:  "The charge type of instance. Valid values are `PREPAID` and `POSTPAID`. Default value is `POSTPAID`. Note: TencentCloud International only supports `POSTPAID`. Caution that update operation on this field will delete old instances and create new with new charge type.",
+			Description:  "The charge type of instance. Valid values are `PREPAID` and `POSTPAID_BY_HOUR`. Default value is `POSTPAID_BY_HOUR`. Note: TencentCloud International only supports `POSTPAID_BY_HOUR`. Caution that update operation on this field will delete old instances and create new one with new charge type.",
 		},
 		"prepaid_period": {
 			Type:         schema.TypeInt,
@@ -173,7 +173,8 @@ func TencentMongodbBasicInfo() map[string]*schema.Schema {
 		"auto_renew_flag": {
 			Type:        schema.TypeInt,
 			Optional:    true,
-			Description: "Auto renew flag. Valid values are `0`(NOTIFY_AND_MANUAL_RENEW), `1`(NOTIFY_AND_AUTO_RENEW) and `2`(DISABLE_NOTIFY_AND_MANUAL_RENEW). Note: only works for PREPAID instance. Only supports`0` and `1` for creation.",
+			Default:     0,
+			Description: "Auto renew flag. Valid values are `0`(NOTIFY_AND_MANUAL_RENEW), `1`(NOTIFY_AND_AUTO_RENEW) and `2`(DISABLE_NOTIFY_AND_MANUAL_RENEW). Default value is `0`. Note: only works for PREPAID instance. Only supports`0` and `1` for creation.",
 		},
 		// Computed
 		"status": {

tencentcloud/internal/helper/transform.go

@@ -1,7 +1,5 @@
 package helper
 
-import "github.com/terraform-providers/terraform-provider-tencentcloud/tencentcloud/connectivity"
-
 func Bool(i bool) *bool { return &i }
 
 func String(i string) *string { return &i }
@@ -111,8 +109,3 @@ func BoolToInt64Ptr(s bool) (i *int64) {
 	i = &result
 	return
 }
-
-func CopySelf(client *connectivity.TencentCloudClient) *connectivity.TencentCloudClient {
-	tmpClient := *client
-	return &tmpClient
-}

tencentcloud/resource_tc_mongodb_instance.go

@@ -33,13 +33,14 @@ import (
 	"errors"
 	"fmt"
 	"log"
+	"reflect"
+	"strings"
 
 	"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
-	"github.com/terraform-providers/terraform-provider-tencentcloud/tencentcloud/ratelimit"
-
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
 	mongodb "github.com/tencentcloud/tencentcloud-sdk-go/tencentcloud/mongodb/v20190725"
 	"github.com/terraform-providers/terraform-provider-tencentcloud/tencentcloud/internal/helper"
+	"github.com/terraform-providers/terraform-provider-tencentcloud/tencentcloud/ratelimit"
 )
 
 func resourceTencentCloudMongodbInstance() *schema.Resource {
@@ -83,9 +84,6 @@ func resourceTencentCloudMongodbInstance() *schema.Resource {
 }
 
 func mongodbAllInstanceReqSet(requestInter interface{}, d *schema.ResourceData) error {
-	requestByMonth, okByMonth := requestInter.(*mongodb.CreateDBInstanceRequest)
-	requestByUse, _ := requestInter.(*mongodb.CreateDBInstanceHourRequest)
-
 	var (
 		replicateSetNum       = 1
 		nodeNum               = 3
@@ -98,6 +96,7 @@ func mongodbAllInstanceReqSet(requestInter interface{}, d *schema.ResourceData)
 		machine               = d.Get("machine_type").(string)
 		password              = d.Get("password").(string)
 		instanceType          = MONGO_INSTANCE_TYPE_FORMAL
+		projectId             = d.Get("project_id").(int)
 	)
 
 	if machine == MONGODB_MACHINE_TYPE_GIO {
@@ -106,79 +105,50 @@ func mongodbAllInstanceReqSet(requestInter interface{}, d *schema.ResourceData)
 		machine = MONGODB_MACHINE_TYPE_HIO10G
 	}
 
-	if okByMonth {
+	getType := reflect.TypeOf(requestInter)
+	value := reflect.ValueOf(requestInter).Elem()
+
+	for k, v := range map[string]interface{}{
+		"ReplicateSetNum": helper.IntUint64(replicateSetNum),
+		"NodeNum":         helper.IntUint64(nodeNum),
+		"GoodsNum":        helper.IntUint64(goodsNum),
+		"ClusterType":     &clusterType,
+		"Memory":          helper.IntUint64(memoryInterface),
+		"Volume":          helper.IntUint64(volumeInterface),
+		"MongoVersion":    &mongoVersionInterface,
+		"Zone":            &zoneInterface,
+		"MachineCode":     &machine,
+		"Password":        &password,
+		"Clone":           helper.IntInt64(instanceType),
+		"ProjectId":       helper.IntInt64(projectId),
+	} {
+		value.FieldByName(k).Set(reflect.ValueOf(v))
+	}
+
+	var okVpc, okSubnet bool
+	if v, ok := d.GetOk("vpc_id"); ok {
+		okVpc = ok
+		value.FieldByName("VpcId").Set(reflect.ValueOf(helper.String(v.(string))))
+	}
+	if v, ok := d.GetOk("subnet_id"); ok {
+		okSubnet = ok
+		value.FieldByName("SubnetId").Set(reflect.ValueOf(helper.String(v.(string))))
+	}
+	if (okVpc && !okSubnet) || (!okVpc && okSubnet) {
+		return fmt.Errorf("you have to set vpc_id and subnet_id both")
+	}
+	if v, ok := d.GetOk("security_groups"); ok {
+		sliceReflect := helper.InterfacesStringsPoint(v.(*schema.Set).List())
+		value.FieldByName("SecurityGroup").Set(reflect.ValueOf(sliceReflect))
+	}
+
+	if strings.Contains(getType.String(), "CreateDBInstanceRequest") {
 		if v, ok := d.GetOk("prepaid_period"); ok {
-			requestByMonth.Period = helper.IntUint64(v.(int))
+			value.FieldByName("Period").Set(reflect.ValueOf(helper.IntUint64(v.(int))))
 		} else {
 			return fmt.Errorf("prepaid_period must be specified for a PREPAID instance")
 		}
-		requestByMonth.AutoRenewFlag = helper.IntUint64(d.Get("auto_renew_flag").(int))
-
-		requestByMonth.ReplicateSetNum = helper.IntUint64(replicateSetNum)
-		requestByMonth.NodeNum = helper.IntUint64(nodeNum)
-		requestByMonth.GoodsNum = helper.IntUint64(goodsNum)
-		requestByMonth.ClusterType = &clusterType
-		requestByMonth.Memory = helper.IntUint64(memoryInterface)
-		requestByMonth.Volume = helper.IntUint64(volumeInterface)
-		requestByMonth.MongoVersion = &mongoVersionInterface
-		requestByMonth.Zone = &zoneInterface
-		requestByMonth.MachineCode = &machine
-		requestByMonth.Password = &password
-		requestByMonth.Clone = helper.IntInt64(instanceType)
-
-		if v, ok := d.GetOk("vpc_id"); ok {
-			requestByMonth.VpcId = helper.String(v.(string))
-		}
-		if v, ok := d.GetOk("subnet_id"); ok {
-			requestByMonth.SubnetId = helper.String(v.(string))
-		}
-		err := fmt.Errorf("you have to set vpc_id and subnet_id both")
-		if (requestByMonth.VpcId != nil && requestByMonth.SubnetId == nil) || (requestByMonth.VpcId == nil && requestByMonth.SubnetId != nil) {
-			return err
-		}
-		if v, ok := d.GetOk("project_id"); ok {
-			requestByMonth.ProjectId = helper.IntInt64(v.(int))
-		}
-		if v, ok := d.GetOk("security_groups"); ok {
-			securityGroups := v.(*schema.Set).List()
-			requestByMonth.SecurityGroup = make([]*string, 0, len(securityGroups))
-			for _, v := range securityGroups {
-				requestByMonth.SecurityGroup = append(requestByMonth.SecurityGroup, helper.String(v.(string)))
-			}
-		}
-	} else {
-		requestByUse.ReplicateSetNum = helper.IntUint64(replicateSetNum)
-		requestByUse.NodeNum = helper.IntUint64(nodeNum)
-		requestByUse.GoodsNum = helper.IntUint64(goodsNum)
-		requestByUse.ClusterType = &clusterType
-		requestByUse.Memory = helper.IntUint64(memoryInterface)
-		requestByUse.Volume = helper.IntUint64(volumeInterface)
-		requestByUse.MongoVersion = &mongoVersionInterface
-		requestByUse.Zone = &zoneInterface
-		requestByUse.MachineCode = &machine
-		requestByUse.Password = &password
-		requestByUse.Clone = helper.IntInt64(instanceType)
-
-		if v, ok := d.GetOk("vpc_id"); ok {
-			requestByUse.VpcId = helper.String(v.(string))
-		}
-		if v, ok := d.GetOk("subnet_id"); ok {
-			requestByUse.SubnetId = helper.String(v.(string))
-		}
-		err := fmt.Errorf("you have to set vpc_id and subnet_id both")
-		if (requestByUse.VpcId != nil && requestByUse.SubnetId == nil) || (requestByUse.VpcId == nil && requestByUse.SubnetId != nil) {
-			return err
-		}
-		if v, ok := d.GetOk("project_id"); ok {
-			requestByUse.ProjectId = helper.IntInt64(v.(int))
-		}
-		if v, ok := d.GetOk("security_groups"); ok {
-			securityGroups := v.(*schema.Set).List()
-			requestByUse.SecurityGroup = make([]*string, 0, len(securityGroups))
-			for _, v := range securityGroups {
-				requestByUse.SecurityGroup = append(requestByUse.SecurityGroup, helper.String(v.(string)))
-			}
-		}
+		value.FieldByName("AutoRenewFlag").Set(reflect.ValueOf(helper.IntUint64(d.Get("auto_renew_flag").(int))))
 	}
 
 	return nil
@@ -193,21 +163,19 @@ func mongodbCreateInstanceByUse(ctx context.Context, d *schema.ResourceData, met
 	}
 
 	var response *mongodb.CreateDBInstanceHourResponse
-	err := resource.Retry(6*writeRetryTimeout, func() *resource.RetryError {
+	var err error
+	err = resource.Retry(writeRetryTimeout, func() *resource.RetryError {
 		ratelimit.Check(request.GetAction())
-		result, e := meta.(*TencentCloudClient).apiV3Conn.UseMongodbClient().CreateDBInstanceHour(request)
-		if e != nil {
-			log.Printf("[CRITAL]%s api[%s] fail, reason:%s\n", logId, request.GetAction(), e.Error())
-			return retryError(e)
+		response, err = meta.(*TencentCloudClient).apiV3Conn.UseMongodbClient().CreateDBInstanceHour(request)
+		if err != nil {
+			log.Printf("[CRITAL]%s api[%s] fail, reason:%s", logId, request.GetAction(), err.Error())
+			return retryError(err)
 		}
-		response = result
 		return nil
 	})
 	if err != nil {
 		return err
 	}
-	log.Printf("[DEBUG]%s api[%s] success, request body [%s], response body [%s]\n",
-		logId, request.GetAction(), request.ToJsonString(), response.ToJsonString())
 
 	if len(response.Response.InstanceIds) < 1 {
 		return fmt.Errorf("mongodb instance id is nil")
@@ -226,21 +194,19 @@ func mongodbCreateInstanceByMonth(ctx context.Context, d *schema.ResourceData, m
 	}
 
 	var response *mongodb.CreateDBInstanceResponse
-	err := resource.Retry(6*writeRetryTimeout, func() *resource.RetryError {
+	var err error
+	err = resource.Retry(writeRetryTimeout, func() *resource.RetryError {
 		ratelimit.Check(request.GetAction())
-		result, e := meta.(*TencentCloudClient).apiV3Conn.UseMongodbClient().CreateDBInstance(request)
-		if e != nil {
-			log.Printf("[CRITAL]%s api[%s] fail, reason:%s\n", logId, request.GetAction(), e.Error())
-			return retryError(e)
+		response, err = meta.(*TencentCloudClient).apiV3Conn.UseMongodbClient().CreateDBInstance(request)
+		if err != nil {
+			log.Printf("[CRITAL]%s api[%s] fail, reason:%s", logId, request.GetAction(), err.Error())
+			return retryError(err)
 		}
-		response = result
 		return nil
 	})
 	if err != nil {
 		return err
 	}
-	log.Printf("[DEBUG]%s api[%s] success, request body [%s], response body [%s]\n",
-		logId, request.GetAction(), request.ToJsonString(), response.ToJsonString())
 
 	if len(response.Response.InstanceIds) < 1 {
 		return fmt.Errorf("mongodb instance id is nil")
@@ -273,7 +239,7 @@ func resourceTencentCloudMongodbInstanceCreate(d *schema.ResourceData, meta inte
 		_, ok := d.GetOk("prepaid_period")
 		_, ok1 := d.GetOk("auto_renew_flag")
 		if ok || ok1 {
-			return fmt.Errorf("prepaid_period and auto_renew_flag don't make sense for POSTPAID instance, please remove them from your template")
+			return fmt.Errorf("prepaid_period and auto_renew_flag don't make sense for POSTPAID_BY_HOUR instance, please remove them from your template")
 		}
 		if err := mongodbCreateInstanceByUse(ctx, d, meta); err != nil {
 			return err
@@ -308,7 +274,6 @@ func resourceTencentCloudMongodbInstanceCreate(d *schema.ResourceData, meta inte
 	if !has {
 		return fmt.Errorf("[CRITAL]%s creating mongodb instance failed, instance doesn't exist\n", logId)
 	}
-	d.SetId(instanceId)
 
 	if tags := helper.GetTags(d, "tags"); len(tags) > 0 {
 		resourceName := BuildTagResourceName("mongodb", "instance", region, instanceId)
@@ -426,12 +391,9 @@ func resourceTencentCloudMongodbInstanceUpdate(d *schema.ResourceData, meta inte
 
 	if d.HasChange("memory") || d.HasChange("volume") {
 		// precheck
-		if !(d.HasChange("memory") && d.HasChange("volume")) {
-			return fmt.Errorf("[CRITAL] updating memory and volume of mongodb instance failed, memory and volume must upgrade/downgrade at same time")
-		}
 		oldMemory, newMemory := d.GetChange("memory")
 		oldVolume, newVolume := d.GetChange("volume")
-		if (newMemory.(int)-oldMemory.(int))^(newVolume.(int)-oldVolume.(int)) <= 0 {
+		if (newMemory.(int) >= oldMemory.(int) || newVolume.(int) >= oldVolume.(int)) && (newMemory.(int) <= oldMemory.(int) || newVolume.(int) <= oldVolume.(int)) {
 			return fmt.Errorf("[CRITAL] updating memory and volume of mongodb instance failed, memory and volume must upgrade/downgrade at same time")
 		}
 		memory := d.Get("memory").(int)
@@ -453,13 +415,13 @@ func resourceTencentCloudMongodbInstanceUpdate(d *schema.ResourceData, meta inte
 
 			memoryDes := *infos.Memory / 1024 / (*infos.ReplicationSetNum)
 			volumeDes := *infos.Volume / 1024 / (*infos.ReplicationSetNum)
-			if d.Get("memory").(int) != int(memoryDes) || d.Get("volume").(int) != int(volumeDes) {
+			if memory != int(memoryDes) || volume != int(volumeDes) {
 				return resource.RetryableError(fmt.Errorf("[CRITAL] updating mongodb instance, current memory and volume values: %d, %d, waiting for them becoming new value: %d, %d", memoryDes, volumeDes, d.Get("memory").(int), d.Get("volume").(int)))
 			}
 			return nil
 		})
 		if errUpdate != nil {
-			return fmt.Errorf("[CRITAL] updating mongodb instance failed, memory and volume values don't change")
+			return errUpdate
 		}
 
 		d.SetPartial("memory")