Rename mongo flag for enabling locking. Add validation around parsing…

… mongo params.

database/mongodb/README.md

@@ -13,8 +13,8 @@
 |------------|---------------------|-------------|
 | `x-migrations-collection` | `MigrationsCollection` | Name of the migrations collection |
 | `x-transaction-mode` | `TransactionMode` | If set to `true` wrap commands in [transaction](https://docs.mongodb.com/manual/core/transactions). Available only for replica set. Driver is using [strconv.ParseBool](https://golang.org/pkg/strconv/#ParseBool) for parsing|
-| `x-advisory-locking` | `true` | Feature flag for advisory locking, if set to false, disable advisory locking |
-| `x-advisory-lock-collection` | `migrate_advisory_lock` | The name of the collection to use for advisory locking |
+| `x-advisory-locking` | `true` | Feature flag for advisory locking, if set to false, disable advisory locking. |
+| `x-advisory-lock-collection` | `migrate_advisory_lock` | The name of the collection to use for advisory locking.|
 | `x-advisory-lock-timout` | `15` | The max time in seconds that the advisory lock will wait if the db is already locked. |
 | `x-advisory-lock-timout-interval` | `10` | The max timeout in seconds interval that the advisory lock will wait if the db is already locked. |
 | `dbname` | `DatabaseName` | The name of the database to connect to |

database/mongodb/mongodb.go

@@ -46,10 +46,10 @@ type Mongo struct {
 }
 
 type Locking struct {
-	CollectionName     string
-	Timeout            int
-	UseAdvisoryLocking bool
-	Interval           int
+	CollectionName string
+	Timeout        int
+	Enabled        bool
+	Interval       int
 }
 type Config struct {
 	DatabaseName         string
@@ -98,7 +98,7 @@ func WithInstance(instance *mongo.Client, config *Config) (database.Driver, erro
 		config: config,
 	}
 
-	if mc.config.Locking.UseAdvisoryLocking {
+	if mc.config.Locking.Enabled {
 		if err := mc.ensureLockTable(); err != nil {
 			return nil, err
 		}
@@ -123,14 +123,22 @@ func (m *Mongo) Open(dsn string) (database.Driver, error) {
 
 	migrationsCollection := unknown.Get("x-migrations-collection")
 	lockCollection := unknown.Get("x-advisory-lock-collection")
-	transactionMode, _ := strconv.ParseBool(unknown.Get("x-transaction-mode"))
-	advisoryLockingFlag, err := strconv.ParseBool(unknown.Get("x-advisory-locking"))
+	transactionMode, err := parseBoolean(unknown.Get("x-transaction-mode"), false)
 	if err != nil {
-		advisoryLockingFlag = DefaultAdvisoryLockingFlag
+		return nil, err
+	}
+	advisoryLockingFlag, err := parseBoolean(unknown.Get("x-advisory-locking"), DefaultAdvisoryLockingFlag)
+	if err != nil {
+		return nil, err
+	}
+	lockingTimout, err := parseInt(unknown.Get("x-advisory-lock-timeout"), DefaultLockTimeout)
+	if err != nil {
+		return nil, err
+	}
+	maxLockingIntervals, err := parseInt(unknown.Get("x-advisory-lock-timout-interval"), DefaultLockTimeoutInterval)
+	if err != nil {
+		return nil, err
 	}
-	lockingTimout, _ := strconv.Atoi(unknown.Get("x-advisory-lock-timeout"))
-	maxLockingIntervals, _ := strconv.Atoi(unknown.Get("x-advisory-lock-timout-interval"))
-
 	client, err := mongo.Connect(context.TODO(), options.Client().ApplyURI(dsn))
 	if err != nil {
 		return nil, err
@@ -144,10 +152,10 @@ func (m *Mongo) Open(dsn string) (database.Driver, error) {
 		MigrationsCollection: migrationsCollection,
 		TransactionMode:      transactionMode,
 		Locking: Locking{
-			CollectionName:     lockCollection,
-			Timeout:            lockingTimout,
-			UseAdvisoryLocking: advisoryLockingFlag,
-			Interval:           maxLockingIntervals,
+			CollectionName: lockCollection,
+			Timeout:        lockingTimout,
+			Enabled:        advisoryLockingFlag,
+			Interval:       maxLockingIntervals,
 		},
 	})
 	if err != nil {
@@ -156,6 +164,39 @@ func (m *Mongo) Open(dsn string) (database.Driver, error) {
 	return mc, nil
 }
 
+//Parse the url param, convert it to boolean
+// returns error if param invalid. returns defaultValue if param not present
+func parseBoolean(urlParam string, defaultValue bool) (bool, error) {
+
+	// if parameter passed, parse it (otherwise return default value)
+	if urlParam != "" {
+		result, err := strconv.ParseBool(urlParam)
+		if err != nil {
+			return false, err
+		}
+		return result, nil
+	}
+
+	// if no url Param passed, return default value
+	return defaultValue, nil
+}
+
+//Parse the url param, convert it to int
+// returns error if param invalid. returns defaultValue if param not present
+func parseInt(urlParam string, defaultValue int) (int, error) {
+
+	// if parameter passed, parse it (otherwise return default value)
+	if urlParam != "" {
+		result, err := strconv.Atoi(urlParam)
+		if err != nil {
+			return -1, err
+		}
+		return result, nil
+	}
+
+	// if no url Param passed, return default value
+	return defaultValue, nil
+}
 func (m *Mongo) SetVersion(version int, dirty bool) error {
 	migrationsCollection := m.db.Collection(m.config.MigrationsCollection)
 	if err := migrationsCollection.Drop(context.TODO()); err != nil {
@@ -286,7 +327,7 @@ func (m *Mongo) ensureVersionTable() (err error) {
 // Utilizes advisory locking on the config.LockingCollection collection
 // This uses a unique index on the `locking_key` field.
 func (m *Mongo) Lock() error {
-	if !m.config.Locking.UseAdvisoryLocking {
+	if !m.config.Locking.Enabled {
 		return nil
 	}
 	pid := os.Getpid()
@@ -321,7 +362,7 @@ func (m *Mongo) Lock() error {
 
 }
 func (m *Mongo) Unlock() error {
-	if !m.config.Locking.UseAdvisoryLocking {
+	if !m.config.Locking.Enabled {
 		return nil
 	}
 

database/mongodb/mongodb_test.go

@@ -222,7 +222,7 @@ func TestLockWorks(t *testing.T) {
 		}
 
 		// disable locking, validate wer can lock twice
-		mc.config.Locking.UseAdvisoryLocking = false
+		mc.config.Locking.Enabled = false
 		err = mc.Lock()
 		if err != nil {
 			t.Fatal(err)
@@ -234,7 +234,7 @@ func TestLockWorks(t *testing.T) {
 
 		// re-enable locking,
 		//try to hit a lock conflict
-		mc.config.Locking.UseAdvisoryLocking = true
+		mc.config.Locking.Enabled = true
 		mc.config.Locking.Timeout = 1
 		err = mc.Lock()
 		if err != nil {