rules: Properly handle conflicts on PUT and POST

Previously, PUT and POST did not result in errors (409) when non-existing
resources were modified, or existing resources were created.
This patch resolves that.

Closes #38

cmd/rules_import.go

@@ -71,9 +71,10 @@ Usage example:
 		for _, r := range rules {
 			fmt.Printf("Importing rule %s...\n", r.ID)
 			client := oathkeeper.NewSDK(endpoint)
-			out, response, err := client.GetRule(r.ID)
-			if err == nil {
-				response.Body.Close()
+
+			shouldUpdate := false
+			if _, response, _ := client.GetRule(r.ID); err == nil && response.StatusCode == http.StatusOK {
+				shouldUpdate = true
 			}
 
 			rh := make([]swagger.RuleHandler, len(r.Authenticators))
@@ -104,7 +105,7 @@ Usage example:
 				},
 			}
 
-			if out != nil {
+			if shouldUpdate {
 				out, response, err := client.UpdateRule(r.ID, sr)
 				checkResponse(response, err, http.StatusOK)
 				fmt.Printf("Successfully imported rule %s...\n", out.Id)

helper/errors.go

@@ -52,4 +52,9 @@ var (
 		CodeField:   http.StatusNotFound,
 		StatusField: http.StatusText(http.StatusNotFound),
 	}
+	ErrResourceConflict = &herodot.DefaultError{
+		ErrorField:  "The request could not be completed due to a conflict with the current state of the target resource",
+		CodeField:   http.StatusConflict,
+		StatusField: http.StatusText(http.StatusConflict),
+	}
 )

rule/manager_memory.go

@@ -54,10 +54,19 @@ func (m *MemoryManager) GetRule(id string) (*Rule, error) {
 }
 
 func (m *MemoryManager) CreateRule(rule *Rule) error {
-	return m.UpdateRule(rule)
+	if _, ok := m.Rules[rule.ID]; ok {
+		return errors.WithStack(helper.ErrResourceConflict)
+	}
+
+	m.Rules[rule.ID] = *rule
+	return nil
 }
 
 func (m *MemoryManager) UpdateRule(rule *Rule) error {
+	if _, ok := m.Rules[rule.ID]; !ok {
+		return errors.WithStack(helper.ErrResourceConflict)
+	}
+
 	m.Rules[rule.ID] = *rule
 	return nil
 }

rule/manager_sql.go

@@ -107,6 +107,14 @@ func (s *SQLManager) CreateRule(rule *Rule) error {
 		return errors.WithStack(err)
 	}
 
+	if err := s.createRule(tx, rule); err != nil {
+		return err
+	}
+
+	return commit(tx)
+}
+
+func (s *SQLManager) createRule(tx *sqlx.Tx, rule *Rule) error {
 	sr := toSqlRule(rule)
 	if err := s.create(tx, "oathkeeper_rule", sqlParams, sr, "surrogate_id"); err != nil {
 		return err
@@ -120,13 +128,6 @@ func (s *SQLManager) CreateRule(rule *Rule) error {
 	if err := s.createMany(tx, "oathkeeper_rule_credentials_issuer", sqlParamsHandler, sr.CredentialsIssuers, rule.ID); err != nil {
 		return err
 	}
-
-	if err := tx.Commit(); err != nil {
-		if rErr := tx.Rollback(); rErr != nil {
-			return errors.Wrap(rErr, err.Error())
-		}
-		return errors.WithStack(err)
-	}
 	return nil
 }
 
@@ -169,11 +170,57 @@ func (s *SQLManager) create(tx *sqlx.Tx, table string, params []string, value in
 }
 
 func (s *SQLManager) UpdateRule(rule *Rule) error {
-	return s.CreateRule(rule)
+	_, err := s.GetRule(rule.ID)
+	if errors.Cause(err) == helper.ErrResourceNotFound {
+		return errors.WithStack(helper.ErrResourceConflict)
+	} else if err != nil {
+		return err
+	}
+
+	tx, err := s.db.Beginx()
+	if err != nil {
+		return errors.WithStack(err)
+	}
+
+	if err := s.deleteRule(tx, rule.ID); err != nil {
+		return err
+	}
+
+	if err := s.createRule(tx, rule); err != nil {
+		return err
+	}
+
+	return commit(tx)
+}
+
+func commit(tx *sqlx.Tx) error {
+	if err := tx.Commit(); err != nil {
+		if rErr := tx.Rollback(); rErr != nil {
+			return errors.Wrap(rErr, err.Error())
+		}
+		return errors.WithStack(err)
+	}
+	return nil
 }
 
 func (s *SQLManager) DeleteRule(id string) error {
-	if _, err := s.db.Exec(s.db.Rebind(`DELETE FROM oathkeeper_rule WHERE surrogate_id=?`), id); err != nil {
+	tx, err := s.db.Beginx()
+	if err != nil {
+		return errors.WithStack(err)
+	}
+
+	if err := s.deleteRule(tx, id); err != nil {
+		return err
+	}
+
+	return commit(tx)
+}
+
+func (s *SQLManager) deleteRule(tx *sqlx.Tx, id string) error {
+	if _, err := tx.Exec(s.db.Rebind(`DELETE FROM oathkeeper_rule WHERE surrogate_id=?`), id); err != nil {
+		if rErr := tx.Rollback(); rErr != nil {
+			return errors.Wrap(rErr, err.Error())
+		}
 		return errors.WithStack(err)
 	}
 	return nil

rule/manager_test.go

@@ -25,6 +25,7 @@ import (
 
 	_ "github.com/go-sql-driver/mysql"
 	_ "github.com/lib/pq"
+	"github.com/ory/oathkeeper/helper"
 	"github.com/ory/oathkeeper/pkg"
 	"github.com/ory/sqlcon/dockertest"
 	"github.com/stretchr/testify/assert"
@@ -56,6 +57,9 @@ func TestManagers(t *testing.T) {
 			_, err := manager.GetRule("1")
 			require.Error(t, err)
 
+			// Updating of a non-existent rule should throw 409
+			require.EqualError(t, manager.UpdateRule(&r3), helper.ErrResourceConflict.Error())
+
 			require.NoError(t, manager.CreateRule(&r1))
 			require.NoError(t, manager.CreateRule(&r2))
 			require.NoError(t, manager.CreateRule(&r3))