refactor type checks on dynamics to better handle errors

README.md

@@ -146,6 +146,4 @@ resource "jenkins_job_datadog_job_property" "main" {
 ## TODO
 
 1. import resources
-1. Refactor unmarshall to use map
-1. Refactor types = reflect checks
 1. Fragile TestAccJobBuildDiscarderPropertyStrategyLogRotatorBasic test

provider/job_build_discarder_property_strategy.go

@@ -6,7 +6,7 @@ import (
 )
 
 type jobBuildDiscarderPropertyStrategy interface {
-	fromClientStrategy(client.JobBuildDiscarderPropertyStrategy) jobBuildDiscarderPropertyStrategy
+	fromClientStrategy(client.JobBuildDiscarderPropertyStrategy) (jobBuildDiscarderPropertyStrategy, error)
 	toClientStrategy(id string) client.JobBuildDiscarderPropertyStrategy
 	setResourceData(*schema.ResourceData) error
 }

provider/job_build_discarder_property_strategy_log_rotator.go

@@ -1,6 +1,9 @@
 package provider
 
 import (
+	"fmt"
+	"reflect"
+
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/jgramoll/terraform-provider-jenkins/client"
 )
@@ -31,14 +34,18 @@ func (strategy *jobBuildDiscarderPropertyStrategyLogRotator) toClientStrategy(id
 	return clientStrategy
 }
 
-func (s *jobBuildDiscarderPropertyStrategyLogRotator) fromClientStrategy(cs client.JobBuildDiscarderPropertyStrategy) jobBuildDiscarderPropertyStrategy {
-	clientStrategy := cs.(*client.JobBuildDiscarderPropertyStrategyLogRotator)
+func (s *jobBuildDiscarderPropertyStrategyLogRotator) fromClientStrategy(clientStrategyInterface client.JobBuildDiscarderPropertyStrategy) (jobBuildDiscarderPropertyStrategy, error) {
+	clientStrategy, ok := clientStrategyInterface.(*client.JobBuildDiscarderPropertyStrategyLogRotator)
+	if !ok {
+		return nil, fmt.Errorf("Strategy is not of expected type, expected *client.JobBuildDiscarderPropertyStrategyLogRotator, actually %s",
+			reflect.TypeOf(clientStrategyInterface).String())
+	}
 	strategy := newJobBuildDiscarderPropertyStrategyLogRotator()
 	strategy.DaysToKeep = clientStrategy.DaysToKeep
 	strategy.NumToKeep = clientStrategy.NumToKeep
 	strategy.ArtifactDaysToKeep = clientStrategy.ArtifactDaysToKeep
 	strategy.ArtifactNumToKeep = clientStrategy.ArtifactNumToKeep
-	return strategy
+	return strategy, nil
 }
 
 func (strategy *jobBuildDiscarderPropertyStrategyLogRotator) setResourceData(d *schema.ResourceData) error {

provider/job_build_discarder_property_strategy_log_rotator_test.go

@@ -2,7 +2,6 @@ package provider
 
 import (
 	"fmt"
-	"reflect"
 	"testing"
 
 	"github.com/hashicorp/terraform/helper/acctest"
@@ -11,10 +10,6 @@ import (
 	"github.com/jgramoll/terraform-provider-jenkins/client"
 )
 
-func init() {
-	jobBuildDiscarderPropertyStrategyTypes["jenkins_job_build_discarder_property_log_rotator_strategy"] = reflect.TypeOf((*client.JobBuildDiscarderPropertyStrategyLogRotator)(nil))
-}
-
 func TestAccJobBuildDiscarderPropertyStrategyLogRotatorBasic(t *testing.T) {
 	var jobRef client.Job
 	var strategyRefs []client.JobBuildDiscarderPropertyStrategy
@@ -72,20 +67,13 @@ resource "jenkins_job_build_discarder_property_log_rotator_strategy" "main" {
 }`, daysToKeep)
 }
 
-func ensureJobBuildDiscarderPropertyStrategyLogRotator(strategyInterface client.JobBuildDiscarderPropertyStrategy, rs *terraform.ResourceState) error {
-	strategy, ok := strategyInterface.(*client.JobBuildDiscarderPropertyStrategyLogRotator)
-	if !ok {
-		return fmt.Errorf("Strategy is not of expected type, expected *client.JobBuildDiscarderPropertyStrategyLogRotator, actually %s",
-			reflect.TypeOf(strategyInterface).String())
-	}
-
-	_, _, strategyId, err := resourceJobPropertyStrategyId(rs.Primary.Attributes["id"])
+func ensureJobBuildDiscarderPropertyStrategyLogRotator(clientStrategyInterface client.JobBuildDiscarderPropertyStrategy, rs *terraform.ResourceState) error {
+	strategyInterface, err := newJobBuildDiscarderPropertyStrategyLogRotator().fromClientStrategy(clientStrategyInterface)
 	if err != nil {
 		return err
 	}
-	if strategyId != strategy.Id {
-		return fmt.Errorf("JobBuildDiscarderPropertyStrategyLogRotator id should be %v, was %v", strategyId, strategy.Id)
-	}
+	strategy := strategyInterface.(*jobBuildDiscarderPropertyStrategyLogRotator)
+
 	err = testCompareResourceInt("JobBuildDiscarderPropertyStrategyLogRotator", "DaysToKeep", rs.Primary.Attributes["days_to_keep"], strategy.DaysToKeep)
 	if err != nil {
 		return err

provider/job_build_discarder_property_strategy_resource.go

@@ -91,8 +91,10 @@ func resourceJobBuildDiscarderPropertyStrategyRead(d *schema.ResourceData, m int
 		d.SetId("")
 		return nil
 	}
-	strategy := createJobBuildDiscarderPropertyStrategy().fromClientStrategy(discardProperty.Strategy.Item)
-
+	strategy, err := createJobBuildDiscarderPropertyStrategy().fromClientStrategy(discardProperty.Strategy.Item)
+	if err != nil {
+		return err
+	}
 	log.Println("[INFO] Reading build discarder propety strategy", d.Id())
 	return strategy.setResourceData(d)
 }

provider/job_build_discarder_property_strategy_test.go

@@ -3,15 +3,12 @@ package provider
 import (
 	"errors"
 	"fmt"
-	"reflect"
 
 	"github.com/hashicorp/terraform/helper/resource"
 	"github.com/hashicorp/terraform/terraform"
 	"github.com/jgramoll/terraform-provider-jenkins/client"
 )
 
-var jobBuildDiscarderPropertyStrategyTypes = map[string]reflect.Type{}
-
 func testAccCheckBuildDiscarderPropertyStrategies(
 	jobRef *client.Job,
 	expectedStrategyResourceNames []string,
@@ -56,7 +53,7 @@ func ensureJobBuildDiscarderPropertyStrategy(
 	resource *terraform.ResourceState,
 	ensureStrategyFunc func(client.JobBuildDiscarderPropertyStrategy, *terraform.ResourceState) error,
 ) (client.JobBuildDiscarderPropertyStrategy, error) {
-	_, propertyId, strategyId, err := resourceJobPropertyStrategyId(resource.Primary.Attributes["id"])
+	_, propertyId, _, err := resourceJobPropertyStrategyId(resource.Primary.Attributes["id"])
 	if err != nil {
 		return nil, err
 	}
@@ -73,13 +70,6 @@ func ensureJobBuildDiscarderPropertyStrategy(
 		return nil, errors.New("Property does not have strategy")
 	}
 
-	expectedType := jobBuildDiscarderPropertyStrategyTypes[resource.Type]
-	strategyType := reflect.TypeOf(strategy)
-	if expectedType != strategyType {
-		return nil, fmt.Errorf("Job Property Strategy %s was type \"%s\", expected type \"%s\"",
-			strategyId, strategyType, expectedType)
-	}
-
 	err = ensureStrategyFunc(strategy, resource)
 	if err != nil {
 		return nil, err

provider/job_datadog_job_property_test.go

@@ -66,8 +66,5 @@ resource "jenkins_job_datadog_job_property" "prop_%v" {
 
 func ensureJobDatadogJobProperty(propertyInterface client.JobProperty, resource *terraform.ResourceState) error {
 	_, err := newJobDatadogJobProperty().fromClientJobProperty(propertyInterface)
-	if err != nil {
-		return err
-	}
-	return nil
+	return err
 }

provider/job_declarative_job_action_test.go

@@ -2,7 +2,6 @@ package provider
 
 import (
 	"fmt"
-	"reflect"
 	"testing"
 
 	"github.com/hashicorp/terraform/helper/acctest"
@@ -51,11 +50,6 @@ resource "jenkins_job_declarative_job_action" "main" {
 }
 
 func ensureJobDeclarativeJobAction(actionInterface client.JobAction, rs *terraform.ResourceState) error {
-	_, ok := actionInterface.(*client.JobDeclarativeJobAction)
-	if !ok {
-		return fmt.Errorf("Action is not of expected type, expected *client.JobDeclarativeJobAction, actually %s",
-			reflect.TypeOf(actionInterface).String())
-	}
-
-	return nil
+	_, err := newJobDeclarativeJobAction().fromClientAction(actionInterface)
+	return err
 }

provider/job_declarative_job_property_tracker_action_test.go

@@ -2,7 +2,6 @@ package provider
 
 import (
 	"fmt"
-	"reflect"
 	"testing"
 
 	"github.com/hashicorp/terraform/helper/acctest"
@@ -51,11 +50,6 @@ resource "jenkins_job_declarative_job_property_tracker_action" "main" {
 }
 
 func ensureJobDeclarativeJobPropertyTrackerAction(actionInterface client.JobAction, rs *terraform.ResourceState) error {
-	_, ok := actionInterface.(*client.JobDeclarativeJobPropertyTrackerAction)
-	if !ok {
-		return fmt.Errorf("Action is not of expected type, expected *client.JobDeclarativeJobPropertyTrackerAction, actually %s",
-			reflect.TypeOf(actionInterface).String())
-	}
-
-	return nil
+	_, err := newJobDeclarativeJobPropertyTrackerAction().fromClientAction(actionInterface)
+	return err
 }

provider/job_definition.go

@@ -6,7 +6,7 @@ import (
 )
 
 type jobDefinition interface {
-	fromClientJobDefintion(client.JobDefinition) jobDefinition
+	fromClientJobDefintion(client.JobDefinition) (jobDefinition, error)
 	toClientDefinition(definitionId string) client.JobDefinition
 	setResourceData(*schema.ResourceData) error
 }

provider/job_definition_resource.go

@@ -73,8 +73,10 @@ func resourceJobDefinitionRead(d *schema.ResourceData, m interface{}, createJobD
 		return nil
 	}
 
-	definition := createJobDefinition().fromClientJobDefintion(j.Definition)
-
+	definition, err := createJobDefinition().fromClientJobDefintion(j.Definition)
+	if err != nil {
+		return err
+	}
 	log.Println("[INFO] Reading from job definition", d.Id())
 	return definition.setResourceData(d)
 }

provider/job_definition_test.go

@@ -3,49 +3,21 @@ package provider
 import (
 	"errors"
 	"fmt"
-	"reflect"
 
 	"github.com/hashicorp/terraform/helper/resource"
 	"github.com/hashicorp/terraform/terraform"
 	"github.com/jgramoll/terraform-provider-jenkins/client"
 )
 
-var jobDefinitionTypes = map[string]reflect.Type{}
-
 func testAccCheckJobDefinition(jobRef *client.Job, expectedDefinitionResourceName string, ensureJobDefinitionFunc func(client.JobDefinition, *terraform.ResourceState) error) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		definitionResource, ok := s.RootModule().Resources[expectedDefinitionResourceName]
 		if !ok {
 			return fmt.Errorf("Job Defintion Resource not found: %s", expectedDefinitionResourceName)
 		}
 
-		_, err := ensureJobDefinition(jobRef, definitionResource, ensureJobDefinitionFunc)
-		if err != nil {
-			return err
-		}
-		return nil
-	}
-}
-
-func ensureJobDefinition(jobRef *client.Job, rs *terraform.ResourceState, ensureJobDefinitionFunc func(client.JobDefinition, *terraform.ResourceState) error) (client.JobDefinition, error) {
-	_, definitionId, err := resourceJobDefinitionId(rs.Primary.Attributes["id"])
-	if err != nil {
-		return nil, err
+		return ensureJobDefinitionFunc(jobRef.Definition, definitionResource)
 	}
-
-	expectedType := jobDefinitionTypes[rs.Type]
-	definitionType := reflect.TypeOf(jobRef.Definition)
-	if expectedType != definitionType {
-		return nil, fmt.Errorf("Job Defintion %s was type \"%s\", expected type \"%s\"",
-			definitionId, definitionType, expectedType)
-	}
-
-	err = ensureJobDefinitionFunc(jobRef.Definition, rs)
-	if err != nil {
-		return nil, err
-	}
-
-	return jobRef.Definition, nil
 }
 
 func testAccCheckNoJobDefinition(jobRef *client.Job) resource.TestCheckFunc {

provider/job_gerrit_trigger.go

@@ -1,36 +1,46 @@
 package provider
 
 import (
+	"fmt"
+	"reflect"
+
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/jgramoll/terraform-provider-jenkins/client"
 )
 
 type jobGerritTrigger struct {
-	Plugin                     string       `mapstructure:"plugin"`
-	ServerName                 string       `mapstructure:"server_name"`
-	SilentMode                 bool         `mapstructure:"silent_mode"`
-	SilentStartMode            bool         `mapstructure:"silent_start_mode"`
-	EscapeQuotes               bool         `mapstructure:"escape_quotes"`
-	NameAndEmailParameterMode  string       `mapstructure:"name_and_email_parameter_mode"`
-	CommitMessageParameterMode string       `mapstructure:"commit_message_parameter_mode"`
-	ChangeSubjectParameterMode string       `mapstructure:"change_subject_parameter_mode"`
-	CommentTextParameterMode   string       `mapstructure:"comment_text_parameter_mode"`
-	SkipVote                   *[]*skipVote `mapstructure:"skip_vote"`
+	Plugin                      string       `mapstructure:"plugin"`
+	ServerName                  string       `mapstructure:"server_name"`
+	SilentMode                  bool         `mapstructure:"silent_mode"`
+	SilentStartMode             bool         `mapstructure:"silent_start_mode"`
+	EscapeQuotes                bool         `mapstructure:"escape_quotes"`
+	NameAndEmailParameterMode   string       `mapstructure:"name_and_email_parameter_mode"`
+	CommitMessageParameterMode  string       `mapstructure:"commit_message_parameter_mode"`
+	ChangeSubjectParameterMode  string       `mapstructure:"change_subject_parameter_mode"`
+	CommentTextParameterMode    string       `mapstructure:"comment_text_parameter_mode"`
+	DynamicTriggerConfiguration bool         `mapstructure:"dynamic_trigger_configuration"`
+	SkipVote                    *[]*skipVote `mapstructure:"skip_vote"`
 }
 
 func newJobGerritTrigger() *jobGerritTrigger {
 	return &jobGerritTrigger{
-		EscapeQuotes:               true,
-		NameAndEmailParameterMode:  "PLAIN",
-		CommitMessageParameterMode: "BASE64",
-		ChangeSubjectParameterMode: "PLAIN",
-		CommentTextParameterMode:   "BASE64",
-		SkipVote:                   &[]*skipVote{},
+		EscapeQuotes:                true,
+		NameAndEmailParameterMode:   "PLAIN",
+		CommitMessageParameterMode:  "BASE64",
+		ChangeSubjectParameterMode:  "PLAIN",
+		CommentTextParameterMode:    "BASE64",
+		DynamicTriggerConfiguration: false,
+		SkipVote:                    &[]*skipVote{},
 	}
 }
 
-func (t *jobGerritTrigger) fromClientJobTrigger(clientTriggerInterface client.JobTrigger) jobTrigger {
-	clientTrigger := clientTriggerInterface.(*client.JobGerritTrigger)
+func (t *jobGerritTrigger) fromClientJobTrigger(clientTriggerInterface client.JobTrigger) (jobTrigger, error) {
+	clientTrigger, ok := clientTriggerInterface.(*client.JobGerritTrigger)
+	if !ok {
+		return nil, fmt.Errorf("Strategy is not of expected type, expected *client.JobGerritTrigger, actually %s",
+			reflect.TypeOf(clientTriggerInterface).String())
+	}
+
 	trigger := newJobGerritTrigger()
 	trigger.Plugin = clientTrigger.Plugin
 	trigger.ServerName = clientTrigger.ServerName
@@ -41,8 +51,9 @@ func (t *jobGerritTrigger) fromClientJobTrigger(clientTriggerInterface client.Jo
 	trigger.CommitMessageParameterMode = clientTrigger.CommitMessageParameterMode.String()
 	trigger.ChangeSubjectParameterMode = clientTrigger.ChangeSubjectParameterMode.String()
 	trigger.CommentTextParameterMode = clientTrigger.CommentTextParameterMode.String()
+	trigger.DynamicTriggerConfiguration = clientTrigger.DynamicTriggerConfiguration
 	*trigger.SkipVote = []*skipVote{newSkipVotefromClient(clientTrigger.SkipVote)}
-	return trigger
+	return trigger, nil
 }
 
 func (t *jobGerritTrigger) toClientJobTrigger(id string) (client.JobTrigger, error) {
@@ -57,6 +68,7 @@ func (t *jobGerritTrigger) toClientJobTrigger(id string) (client.JobTrigger, err
 	if err != nil {
 		return nil, err
 	}
+	clientTrigger.DynamicTriggerConfiguration = t.DynamicTriggerConfiguration
 	clientTrigger.SkipVote = newClientSkipVote(t.SkipVote)
 	return clientTrigger, nil
 }
@@ -113,5 +125,8 @@ func (t *jobGerritTrigger) setResourceData(d *schema.ResourceData) error {
 	if err := d.Set("comment_text_parameter_mode", t.CommentTextParameterMode); err != nil {
 		return err
 	}
+	if err := d.Set("dynamic_trigger_configuration", t.DynamicTriggerConfiguration); err != nil {
+		return err
+	}
 	return d.Set("skip_vote", t.SkipVote)
 }

provider/job_gerrit_trigger_draft_published_event.go

@@ -1,6 +1,9 @@
 package provider
 
 import (
+	"fmt"
+	"reflect"
+
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/jgramoll/terraform-provider-jenkins/client"
 )
@@ -12,8 +15,13 @@ func newJobGerritTriggerDraftPublishedEvent() *jobGerritTriggerDraftPublishedEve
 	return &jobGerritTriggerDraftPublishedEvent{}
 }
 
-func (event *jobGerritTriggerDraftPublishedEvent) fromClientJobTriggerEvent(clientEvent client.JobGerritTriggerOnEvent) jobGerritTriggerEvent {
-	return newJobGerritTriggerDraftPublishedEvent()
+func (event *jobGerritTriggerDraftPublishedEvent) fromClientJobTriggerEvent(clientEventInterface client.JobGerritTriggerOnEvent) (jobGerritTriggerEvent, error) {
+	_, ok := clientEventInterface.(*client.JobGerritTriggerPluginDraftPublishedEvent)
+	if !ok {
+		return nil, fmt.Errorf("Failed to parse client action, expected *client.JobGerritTriggerPluginDraftPublishedEvent, got %s",
+			reflect.TypeOf(clientEventInterface).String())
+	}
+	return newJobGerritTriggerDraftPublishedEvent(), nil
 }
 
 func (event *jobGerritTriggerDraftPublishedEvent) toClientJobTriggerEvent(eventId string) (client.JobGerritTriggerOnEvent, error) {