Refactor API versioning, replace not update the settings.

jangocheng · Jan 30, 2019 · 2808990 · 2808990
1 parent 5e24035
commit 2808990
Show file tree

Hide file tree

Showing 12 changed files with 194 additions and 257 deletions.
diff --git a/apiserver/allfacades.go b/apiserver/allfacades.go
@@ -267,9 +267,9 @@ func AllFacades() *facade.Registry {
 
 	reg("StatusHistory", 2, statushistory.NewAPI)
 
-	reg("Storage", 3, storage.NewFacadeV3)
-	reg("Storage", 4, storage.NewFacadeV4) // changes Destroy() method signature.
-	reg("Storage", 5, storage.NewFacadeV5) // adds Update() and Delete()
+	reg("Storage", 3, storage.NewStorageAPIV3)
+	reg("Storage", 4, storage.NewStorageAPIV4) // changes Destroy() method signature.
+	reg("Storage", 5, storage.NewStorageAPI)   // Update and Delete storage pools
 
 	reg("StorageProvisioner", 3, storageprovisioner.NewFacadeV3)
 	reg("StorageProvisioner", 4, storageprovisioner.NewFacadeV4)

diff --git a/apiserver/facades/client/storage/base_test.go b/apiserver/facades/client/storage/base_test.go
@@ -26,9 +26,9 @@ type baseStorageSuite struct {
 	resources  *common.Resources
 	authorizer apiservertesting.FakeAuthorizer
 
-	api             *storage.APIv5
-	apiCaas         *storage.APIv5
-	apiv3           *storage.APIv3
+	api             *storage.StorageAPI
+	apiCaas         *storage.StorageAPI
+	apiv3           *storage.StorageAPIv3
 	storageAccessor *mockStorageAccessor
 	state           *mockState
 
@@ -67,12 +67,14 @@ func (s *baseStorageSuite) SetUpTest(c *gc.C) {
 	s.poolManager = s.constructPoolManager()
 
 	s.callContext = context.NewCloudCallContext()
-	var err error
-	s.api, err = storage.NewAPIv5(s.state, state.ModelTypeIAAS, s.storageAccessor, s.registry, s.poolManager, s.resources, s.authorizer, s.callContext)
-	s.apiCaas, err = storage.NewAPIv5(s.state, state.ModelTypeCAAS, s.storageAccessor, s.registry, s.poolManager, s.resources, s.authorizer, s.callContext)
-	c.Assert(err, jc.ErrorIsNil)
-	s.apiv3, err = storage.NewAPIv3(s.state, state.ModelTypeIAAS, s.storageAccessor, s.registry, s.poolManager, s.resources, s.authorizer, s.callContext)
-	c.Assert(err, jc.ErrorIsNil)
+	s.api = storage.NewStorageAPIForTest(s.state, state.ModelTypeIAAS, s.storageAccessor, s.registry, s.poolManager, s.authorizer, s.callContext)
+	s.apiCaas = storage.NewStorageAPIForTest(s.state, state.ModelTypeCAAS, s.storageAccessor, s.registry, s.poolManager, s.authorizer, s.callContext)
+	newAPI := storage.NewStorageAPIForTest(s.state, state.ModelTypeIAAS, s.storageAccessor, s.registry, s.poolManager, s.authorizer, s.callContext)
+	s.apiv3 = &storage.StorageAPIv3{
+		StorageAPIv4: storage.StorageAPIv4{
+			StorageAPI: *newAPI,
+		},
+	}
 }
 
 // TODO(axw) get rid of assertCalls, use stub directly everywhere.
@@ -357,13 +359,9 @@ func (s *baseStorageSuite) constructPoolManager() *mockPoolManager {
 			}
 			return result, nil
 		},
-		updatePool: func(name string, attrs map[string]interface{}) error {
+		replacePool: func(name string, attrs map[string]interface{}) error {
 			if p, ok := s.pools[name]; ok {
-				updatedAttr := p.Attrs()
-				for k, v := range attrs {
-					updatedAttr[k] = v
-				}
-				newPool, err := jujustorage.NewConfig(name, p.Provider(), updatedAttr)
+				newPool, err := jujustorage.NewConfig(name, p.Provider(), attrs)
 				s.pools[name] = newPool
 				return err
 			}

diff --git a/apiserver/facades/client/storage/export_test.go b/apiserver/facades/client/storage/export_test.go
@@ -1,13 +1,14 @@
-// Copyright 2015, 2019 Canonical Ltd.
+// Copyright 2015 Canonical Ltd.
 // Licensed under the AGPLv3, see LICENCE file for details.
 
 package storage
 
 var (
-	ValidatePoolListFilter   = (*APIv5).validatePoolListFilter
-	ValidateNameCriteria     = (*APIv5).validateNameCriteria
-	ValidateProviderCriteria = (*APIv5).validateProviderCriteria
-	EnsureStoragePoolFilter  = (*APIv5).ensureStoragePoolFilter
+	ValidatePoolListFilter   = (*StorageAPI).validatePoolListFilter
+	ValidateNameCriteria     = (*StorageAPI).validateNameCriteria
+	ValidateProviderCriteria = (*StorageAPI).validateProviderCriteria
+	EnsureStoragePoolFilter  = (*StorageAPI).ensureStoragePoolFilter
+	NewStorageAPIForTest     = newStorageAPI
 )
 
 type (

diff --git a/apiserver/facades/client/storage/mock_test.go b/apiserver/facades/client/storage/mock_test.go
@@ -16,11 +16,11 @@ import (
 )
 
 type mockPoolManager struct {
-	getPool    func(name string) (*jujustorage.Config, error)
-	createPool func(name string, providerType jujustorage.ProviderType, attrs map[string]interface{}) (*jujustorage.Config, error)
-	deletePool func(name string) error
-	listPools  func() ([]*jujustorage.Config, error)
-	updatePool func(name string, attrs map[string]interface{}) error
+	getPool     func(name string) (*jujustorage.Config, error)
+	createPool  func(name string, providerType jujustorage.ProviderType, attrs map[string]interface{}) (*jujustorage.Config, error)
+	deletePool  func(name string) error
+	listPools   func() ([]*jujustorage.Config, error)
+	replacePool func(name string, attrs map[string]interface{}) error
 }
 
 func (m *mockPoolManager) Get(name string) (*jujustorage.Config, error) {
@@ -39,8 +39,8 @@ func (m *mockPoolManager) List() ([]*jujustorage.Config, error) {
 	return m.listPools()
 }
 
-func (m *mockPoolManager) Update(name string, attrs map[string]interface{}) error {
-	return m.updatePool(name, attrs)
+func (m *mockPoolManager) Replace(name string, attrs map[string]interface{}) error {
+	return m.replacePool(name, attrs)
 }
 
 type mockStorageAccessor struct {

diff --git a/apiserver/facades/client/storage/pooldelete_test.go b/apiserver/facades/client/storage/pooldelete_test.go
@@ -9,7 +9,7 @@ import (
 	jc "github.com/juju/testing/checkers"
 	gc "gopkg.in/check.v1"
 
-	storage "github.com/juju/juju/storage"
+	"github.com/juju/juju/storage"
 	"github.com/juju/juju/storage/provider"
 )
 
@@ -41,7 +41,7 @@ func (s *poolDeleteSuite) TestDeletePool(c *gc.C) {
 	c.Assert(pools, gc.HasLen, 0)
 }
 
-func (s *poolDeleteSuite) TestDeleteErrorNotExists(c *gc.C) {
+func (s *poolDeleteSuite) TestDeleteNotExists(c *gc.C) {
 	poolName := fmt.Sprintf("%v%v", tstName, 0)
 
 	err := s.api.DeletePool(poolName)

diff --git a/apiserver/facades/client/storage/poolupdate_test.go b/apiserver/facades/client/storage/poolupdate_test.go
@@ -11,7 +11,7 @@ import (
 	gc "gopkg.in/check.v1"
 
 	"github.com/juju/juju/apiserver/params"
-	storage "github.com/juju/juju/storage"
+	"github.com/juju/juju/storage"
 	"github.com/juju/juju/storage/provider"
 )
 
@@ -26,7 +26,10 @@ func (s *poolUpdateSuite) createPools(c *gc.C, num int) {
 	for i := 0; i < num; i++ {
 		poolName := fmt.Sprintf("%v%v", tstName, i)
 		s.baseStorageSuite.pools[poolName], err =
-			storage.NewConfig(poolName, provider.LoopProviderType, map[string]interface{}{"zip": "zap"})
+			storage.NewConfig(poolName, provider.LoopProviderType, map[string]interface{}{
+				"zip":  "zap",
+				"beep": "boop",
+			})
 		c.Assert(err, jc.ErrorIsNil)
 	}
 }

diff --git a/apiserver/facades/client/storage/shim.go b/apiserver/facades/client/storage/shim.go
@@ -8,104 +8,14 @@ import (
 	"gopkg.in/juju/names.v2"
 
 	"github.com/juju/juju/apiserver/common/storagecommon"
-	"github.com/juju/juju/apiserver/facade"
-	"github.com/juju/juju/caas"
-	"github.com/juju/juju/environs"
 	"github.com/juju/juju/state"
-	"github.com/juju/juju/state/stateenvirons"
-	"github.com/juju/juju/storage/poolmanager"
 )
 
 // This file contains untested shims to let us wrap state in a sensible
 // interface and avoid writing tests that depend on mongodb. If you were
 // to change any part of it so that it were no longer *obviously* and
 // *trivially* correct, you would be Doing It Wrong.
 
-// NewFacadeV5 provides the signature required for facade registration.
-func NewFacadeV5(
-	st *state.State,
-	resources facade.Resources,
-	authorizer facade.Authorizer,
-) (*APIv5, error) {
-	model, err := st.Model()
-	if err != nil {
-		return nil, errors.Trace(err)
-	}
-	registry, err := stateenvirons.NewStorageProviderRegistryForModel(
-		model,
-		stateenvirons.GetNewEnvironFunc(environs.New),
-		stateenvirons.GetNewCAASBrokerFunc(caas.New))
-	pm := poolmanager.New(state.NewStateSettings(st), registry)
-
-	storageAccessor, err := getStorageAccessor(st)
-	if err != nil {
-		return nil, errors.Annotate(err, "getting backend")
-	}
-	return NewAPIv5(
-		stateShim{st},
-		model.Type(),
-		storageAccessor,
-		registry, pm, resources, authorizer,
-		state.CallContext(st))
-}
-
-// NewFacadeV4 provides the signature required for facade registration.
-func NewFacadeV4(
-	st *state.State,
-	resources facade.Resources,
-	authorizer facade.Authorizer,
-) (*APIv4, error) {
-	model, err := st.Model()
-	if err != nil {
-		return nil, errors.Trace(err)
-	}
-	registry, err := stateenvirons.NewStorageProviderRegistryForModel(
-		model,
-		stateenvirons.GetNewEnvironFunc(environs.New),
-		stateenvirons.GetNewCAASBrokerFunc(caas.New))
-	pm := poolmanager.New(state.NewStateSettings(st), registry)
-
-	storageAccessor, err := getStorageAccessor(st)
-	if err != nil {
-		return nil, errors.Annotate(err, "getting backend")
-	}
-	return NewAPIv4(
-		stateShim{st},
-		model.Type(),
-		storageAccessor,
-		registry, pm, resources, authorizer,
-		state.CallContext(st))
-}
-
-// NewFacadeV3 provides the signature required for facade registration.
-func NewFacadeV3(
-	st *state.State,
-	resources facade.Resources,
-	authorizer facade.Authorizer,
-) (*APIv3, error) {
-	model, err := st.Model()
-	if err != nil {
-		return nil, errors.Trace(err)
-	}
-	env, err := stateenvirons.GetNewEnvironFunc(environs.New)(st)
-	if err != nil {
-		return nil, errors.Annotate(err, "getting environ")
-	}
-	registry := stateenvirons.NewStorageProviderRegistry(env)
-	pm := poolmanager.New(state.NewStateSettings(st), registry)
-
-	storageAccessor, err := getStorageAccessor(st)
-	if err != nil {
-		return nil, errors.Annotate(err, "getting backend")
-	}
-	return NewAPIv3(
-		stateShim{st},
-		model.Type(),
-		storageAccessor,
-		registry, pm, resources, authorizer,
-		state.CallContext(st))
-}
-
 type storageAccess interface {
 	storageInterface