Move RequiresBinlogFormatChange to inspector + add test

Author: timvaillancourt

go/base/context.go

@@ -354,14 +354,6 @@ func (this *MigrationContext) GetVoluntaryLockName() string {
 	return fmt.Sprintf("%s.%s.lock", this.DatabaseName, this.OriginalTableName)
 }
 
-// RequiresBinlogFormatChange is `true` when the original binlog format isn't `ROW`
-func (this *MigrationContext) RequiresBinlogFormatChange() bool {
-	if this.InspectorServerInfo == nil {
-		return true
-	}
-	return this.InspectorServerInfo.BinlogFormat != "ROW"
-}
-
 // GetApplierHostname is a safe access method to the applier hostname
 func (this *MigrationContext) GetApplierHostname() string {
 	if this.ApplierConnectionConfig == nil {

go/base/utils_test.go

@@ -37,7 +37,8 @@ func TestValidateConnection(t *testing.T) {
 			Port:     mysql.DefaultInstancePort,
 		},
 	}
-	// check valid port matches connectionConfig
+
+	// check valid port matching connectionConfig validates
 	{
 		migrationContext := &MigrationContext{Log: NewDefaultLogger()}
 		serverInfo := &mysql.ServerInfo{
@@ -81,6 +82,15 @@ func TestValidateConnection(t *testing.T) {
 		}
 		test.S(t).ExpectNil(ValidateConnection(serverInfo, connectionConfig, migrationContext, "test"))
 	}
+	// check extra_port validates when port does not match but extra_port does
+	{
+		migrationContext := &MigrationContext{Log: NewDefaultLogger()}
+		serverInfo := &mysql.ServerInfo{
+			Port:      gosql.NullInt64{Int64: 12345, Valid: true},
+			ExtraPort: gosql.NullInt64{Int64: mysql.DefaultInstancePort, Valid: true},
+		}
+		test.S(t).ExpectNil(ValidateConnection(serverInfo, connectionConfig, migrationContext, "test"))
+	}
 	// check validation fails when valid port does not match connectionConfig
 	{
 		migrationContext := &MigrationContext{Log: NewDefaultLogger()}

go/logic/inspect.go

@@ -81,6 +81,14 @@ func (this *Inspector) InitDBConnections() (err error) {
 	return nil
 }
 
+// RequiresBinlogFormatChange is `true` when the original binlog format isn't `ROW`
+func (this *Inspector) RequiresBinlogFormatChange() bool {
+	if this.ServerInfo() == nil {
+		return true
+	}
+	return this.ServerInfo().BinlogFormat != "ROW"
+}
+
 func (this *Inspector) ValidateOriginalTable() (err error) {
 	if err := this.validateTable(); err != nil {
 		return err
@@ -318,7 +326,7 @@ func (this *Inspector) restartReplication() error {
 // applyBinlogFormat sets ROW binlog format and restarts replication to make
 // the replication thread apply it.
 func (this *Inspector) applyBinlogFormat() error {
-	if this.migrationContext.RequiresBinlogFormatChange() {
+	if this.RequiresBinlogFormatChange() {
 		if !this.migrationContext.SwitchToRowBinlogFormat {
 			return fmt.Errorf("Existing binlog_format is %s. Am not switching it to ROW unless you specify --switch-to-rbr",
 				this.ServerInfo().BinlogFormat)
@@ -350,7 +358,7 @@ func (this *Inspector) validateBinlogs() error {
 		return fmt.Errorf("%s must have binary logs enabled", this.connectionConfig.Key.String())
 	}
 
-	if this.migrationContext.RequiresBinlogFormatChange() {
+	if this.RequiresBinlogFormatChange() {
 		if !this.migrationContext.SwitchToRowBinlogFormat {
 			return fmt.Errorf("You must be using ROW binlog format. I can switch it for you, provided --switch-to-rbr and that %s doesn't have replicas", this.connectionConfig.Key.String())
 		}

go/logic/inspect_test.go

@@ -10,6 +10,8 @@ import (
 
 	test "github.com/openark/golib/tests"
 
+	"github.com/github/gh-ost/go/base"
+	"github.com/github/gh-ost/go/mysql"
 	"github.com/github/gh-ost/go/sql"
 )
 
@@ -29,3 +31,25 @@ func TestInspectGetSharedUniqueKeys(t *testing.T) {
 	test.S(t).ExpectEquals(sharedUniqKeys[0].Columns.String(), "id,item_id")
 	test.S(t).ExpectEquals(sharedUniqKeys[1].Columns.String(), "id,org_id")
 }
+
+func TestRequiresBinlogFormatChange(t *testing.T) {
+	migrationContext := &base.MigrationContext{
+		InspectorServerInfo: &mysql.ServerInfo{},
+	}
+
+	{
+		migrationContext.InspectorServerInfo.BinlogFormat = "MINIMAL"
+		inspector := &Inspector{migrationContext: migrationContext}
+		test.S(t).ExpectTrue(inspector.RequiresBinlogFormatChange())
+	}
+	{
+		migrationContext.InspectorServerInfo.BinlogFormat = "MIXED"
+		inspector := &Inspector{migrationContext: migrationContext}
+		test.S(t).ExpectTrue(inspector.RequiresBinlogFormatChange())
+	}
+	{
+		migrationContext.InspectorServerInfo.BinlogFormat = "ROW"
+		inspector := &Inspector{migrationContext: migrationContext}
+		test.S(t).ExpectFalse(inspector.RequiresBinlogFormatChange())
+	}
+}