Skip to content

Commit

Permalink
Fix filtering bug and add extra tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@wallyworld
wallyworld committed Mar 21, 2017
1 parent 2c965bd commit d337343
Show file tree
Hide file tree
Showing 9 changed files with 168 additions and 41 deletions.
6 changes: 4 additions & 2 deletions apiserver/crossmodel/base_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ type baseCrossmodelSuite struct {
api *crossmodel.API

mockState *mockState
mockStatePool *mockStatePool
applicationOffers *mockApplicationOffers
}

Expand All @@ -50,9 +51,10 @@ func (s *baseCrossmodelSuite) SetUpTest(c *gc.C) {
}

var err error
s.mockState = &mockState{}
s.mockState = &mockState{modelUUID: "uuid"}
s.mockStatePool = &mockStatePool{map[string]crossmodel.Backend{s.mockState.modelUUID: s.mockState}}
s.api, err = crossmodel.CreateAPI(
getApplicationOffers, s.mockState, &mockStatePool{s.mockState}, s.authorizer,
getApplicationOffers, s.mockState, s.mockStatePool, s.authorizer,
)
c.Assert(err, jc.ErrorIsNil)
}
51 changes: 38 additions & 13 deletions apiserver/crossmodel/crossmodel.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package crossmodel

import (
"fmt"
"sort"

"github.com/juju/errors"
"gopkg.in/juju/names.v2"
Expand Down Expand Up @@ -260,10 +261,15 @@ func (api *API) ListApplicationOffers(filters params.OfferFilters) (params.ListA
return result, nil
}

// getApplicationOffersDetails gets details about remote applications that match given filter.
func (api *API) getApplicationOffersDetails(filters params.OfferFilters) ([]params.ApplicationOfferDetails, error) {
models := make(map[string]Model)
filtersPerModel := make(map[string][]jujucrossmodel.ApplicationOfferFilter)
// getModelFilters splits the specified filters per model and returns
// the model and filter details for each.
func (api *API) getModelFilters(filters params.OfferFilters) (
models map[string]Model,
filtersPerModel map[string][]jujucrossmodel.ApplicationOfferFilter,
_ error,
) {
models = make(map[string]Model)
filtersPerModel = make(map[string][]jujucrossmodel.ApplicationOfferFilter)

// Group the filters per model and then query each model with the relevant filters
// for that model.
Expand All @@ -276,33 +282,44 @@ func (api *API) getApplicationOffersDetails(filters params.OfferFilters) ([]para
var err error
model, err = api.backend.Model()
if err != nil {
return nil, common.ServerError(errors.Trace(err))
return nil, nil, errors.Trace(err)
}
models[modelUUID] = model
}
// If the filter contains a model name, look up the details.
if f.ModelName != "" {
if _, ok := modelUUIDs[f.ModelName]; !ok {
if modelUUID, ok = modelUUIDs[f.ModelName]; !ok {
var err error
model, ok, err = api.modelForName(f.ModelName, "")
model, ok, err := api.modelForName(f.ModelName, "")
if err != nil {
return nil, common.ServerError(errors.Trace(err))
return nil, nil, errors.Trace(err)
}
if !ok {
err := errors.NotFoundf("model %q", f.ModelName)
return nil, common.ServerError(err)
return nil, nil, errors.Trace(err)
}
// Record the UUID for next time.
// Record the UUID and model for next time.
modelUUID = model.UUID()
modelUUIDs[f.ModelName] = modelUUID
models[modelUUID] = model
}
}

// Record the filter and model details against the model UUID.
models[modelUUID] = model
filters := filtersPerModel[modelUUID]
filters = append(filters, makeOfferFilterFromParams(f))
filtersPerModel[modelUUID] = filters
}
return models, filtersPerModel, nil
}

// getApplicationOffersDetails gets details about remote applications that match given filter.
func (api *API) getApplicationOffersDetails(filters params.OfferFilters) ([]params.ApplicationOfferDetails, error) {
// Gather all the filter details for doing a query for each model.
models, filtersPerModel, err := api.getModelFilters(filters)
if err != nil {
return nil, common.ServerError(errors.Trace(err))
}

if len(filtersPerModel) == 0 {
thisModelUUID := api.backend.ModelUUID()
Expand All @@ -314,9 +331,17 @@ func (api *API) getApplicationOffersDetails(filters params.OfferFilters) ([]para
models[thisModelUUID] = model
}

var result []params.ApplicationOfferDetails
// Ensure the result is deterministic.
var allUUIDs []string
for modelUUID := range filtersPerModel {
allUUIDs = append(allUUIDs, modelUUID)
}
sort.Strings(allUUIDs)

// Do the per model queries.
for modelUUID, filters := range filtersPerModel {
var result []params.ApplicationOfferDetails
for _, modelUUID := range allUUIDs {
filters := filtersPerModel[modelUUID]
offers, err := api.applicationOffersFromModel(modelUUID, filters...)
if err != nil {
return nil, common.ServerError(errors.Trace(err))
Expand Down
130 changes: 115 additions & 15 deletions apiserver/crossmodel/crossmodel_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ func (s *crossmodelSuite) TestShow(c *gc.C) {
s.mockState.applications = map[string]crossmodel.Application{
applicationName: &mockApplication{charm: ch, curl: charm.MustParseURL("db2-2")},
}
s.mockState.modelUUID = "uuid"
s.mockState.model = &mockModel{uuid: "uuid", name: "prod", owner: "fred"}
s.mockState.usermodels = []crossmodel.UserModel{
&mockUserModel{model: s.mockState.model},
Expand Down Expand Up @@ -177,7 +176,6 @@ func (s *crossmodelSuite) TestShowError(c *gc.C) {
s.applicationOffers.listOffers = func(filters ...jujucrossmodel.ApplicationOfferFilter) ([]jujucrossmodel.ApplicationOffer, error) {
return nil, errors.New(msg)
}
s.mockState.modelUUID = "uuid"
s.mockState.model = &mockModel{uuid: "uuid", name: "prod", owner: "fred"}
s.mockState.usermodels = []crossmodel.UserModel{
&mockUserModel{model: s.mockState.model},
Expand All @@ -197,7 +195,6 @@ func (s *crossmodelSuite) TestShowNotFound(c *gc.C) {
s.applicationOffers.listOffers = func(filters ...jujucrossmodel.ApplicationOfferFilter) ([]jujucrossmodel.ApplicationOffer, error) {
return nil, nil
}
s.mockState.modelUUID = "uuid"
s.mockState.model = &mockModel{uuid: "uuid", name: "prod", owner: "fred"}
s.mockState.usermodels = []crossmodel.UserModel{
&mockUserModel{model: s.mockState.model},
Expand All @@ -217,12 +214,13 @@ func (s *crossmodelSuite) TestShowErrorMsgMultipleURLs(c *gc.C) {
s.applicationOffers.listOffers = func(filters ...jujucrossmodel.ApplicationOfferFilter) ([]jujucrossmodel.ApplicationOffer, error) {
return nil, nil
}
s.mockState.modelUUID = "uuid"
s.mockState.model = &mockModel{uuid: "uuid", name: "prod", owner: "fred"}
s.mockState.usermodels = []crossmodel.UserModel{
&mockUserModel{model: s.mockState.model},
&mockUserModel{model: &mockModel{uuid: "uuid2", name: "test", owner: "fred"}},
}
anotherState := &mockState{modelUUID: "uuid2"}
s.mockStatePool.st["uuid2"] = anotherState

found, err := s.api.ApplicationOffers(filter)
c.Assert(err, jc.ErrorIsNil)
Expand Down Expand Up @@ -262,17 +260,22 @@ func (s *crossmodelSuite) TestShowFoundMultiple(c *gc.C) {
}
ch := &mockCharm{meta: &charm.Meta{Description: "A pretty popular blog engine"}}
s.mockState.applications = map[string]crossmodel.Application{
"test": &mockApplication{charm: ch, curl: charm.MustParseURL("db2-2")},
"testAgain": &mockApplication{charm: ch, curl: charm.MustParseURL("mysql-2")},
"test": &mockApplication{charm: ch, curl: charm.MustParseURL("db2-2")},
}
s.mockState.modelUUID = "uuid"
s.mockState.model = &mockModel{uuid: "uuid", name: "prod", owner: "fred"}
s.mockState.usermodels = []crossmodel.UserModel{
&mockUserModel{model: s.mockState.model},
&mockUserModel{model: &mockModel{uuid: "uuid2", name: "test", owner: "mary"}},
}
s.mockState.connStatus = &mockConnectionStatus{count: 5}

anotherState := &mockState{modelUUID: "uuid2"}
anotherState.applications = map[string]crossmodel.Application{
"testAgain": &mockApplication{charm: ch, curl: charm.MustParseURL("mysql-2")},
}
anotherState.connStatus = &mockConnectionStatus{count: 5}
s.mockStatePool.st["uuid2"] = anotherState

found, err := s.api.ApplicationOffers(filter)
c.Assert(err, jc.ErrorIsNil)
var results []params.ApplicationOffer
Expand All @@ -295,7 +298,7 @@ func (s *crossmodelSuite) TestShowFoundMultiple(c *gc.C) {
s.applicationOffers.CheckCallNames(c, listOffersBackendCall, listOffersBackendCall)
}

func (s *crossmodelSuite) setupOffers(c *gc.C) {
func (s *crossmodelSuite) setupOffers(c *gc.C, filterAppName string) {
applicationName := "test"
offerName := "hosted-db2"

Expand All @@ -310,30 +313,27 @@ func (s *crossmodelSuite) setupOffers(c *gc.C) {
c.Assert(filters, gc.HasLen, 1)
c.Assert(filters[0], jc.DeepEquals, jujucrossmodel.ApplicationOfferFilter{
OfferName: offerName,
ApplicationName: applicationName,
ApplicationName: filterAppName,
})
return []jujucrossmodel.ApplicationOffer{anOffer}, nil
}
ch := &mockCharm{meta: &charm.Meta{Description: "A pretty popular blog engine"}}
s.mockState.applications = map[string]crossmodel.Application{
"test": &mockApplication{charm: ch, curl: charm.MustParseURL("db2-2")},
}
s.mockState.modelUUID = "uuid"
s.mockState.model = &mockModel{uuid: "uuid", name: "prod", owner: "fred"}
s.mockState.usermodels = []crossmodel.UserModel{
&mockUserModel{model: s.mockState.model},
}
s.mockState.connStatus = &mockConnectionStatus{count: 5}

}

func (s *crossmodelSuite) TestFind(c *gc.C) {
s.setupOffers(c)
s.setupOffers(c, "")
filter := params.OfferFilters{
Filters: []params.OfferFilter{
{
OfferName: "hosted-db2",
ApplicationName: "test",
OfferName: "hosted-db2",
},
},
}
Expand All @@ -350,6 +350,106 @@ func (s *crossmodelSuite) TestFind(c *gc.C) {
s.applicationOffers.CheckCallNames(c, listOffersBackendCall)
}

func (s *crossmodelSuite) TestFindMultiModel(c *gc.C) {
db2Offer := jujucrossmodel.ApplicationOffer{
OfferName: "hosted-db2",
ApplicationName: "db2",
ApplicationDescription: "db2 description",
Endpoints: map[string]charm.Relation{"db": {Name: "db2"}},
}
mysqlOffer := jujucrossmodel.ApplicationOffer{
OfferName: "hosted-mysql",
ApplicationName: "mysql",
ApplicationDescription: "mysql description",
Endpoints: map[string]charm.Relation{"db": {Name: "mysql"}},
}
postgresqlOffer := jujucrossmodel.ApplicationOffer{
OfferName: "hosted-postgresql",
ApplicationName: "postgresql",
ApplicationDescription: "postgresql description",
Endpoints: map[string]charm.Relation{"db": {Name: "postgresql"}},
}

s.applicationOffers.listOffers = func(filters ...jujucrossmodel.ApplicationOfferFilter) ([]jujucrossmodel.ApplicationOffer, error) {
var result []jujucrossmodel.ApplicationOffer
for _, f := range filters {
switch f.OfferName {
case "hosted-db2":
result = append(result, db2Offer)
case "hosted-mysql":
result = append(result, mysqlOffer)
case "hosted-postgresql":
result = append(result, postgresqlOffer)
}
}
return result, nil
}
ch := &mockCharm{meta: &charm.Meta{Description: "A pretty popular blog engine"}}
s.mockState.applications = map[string]crossmodel.Application{
"db2": &mockApplication{charm: ch, curl: charm.MustParseURL("db2-2")},
}
s.mockState.model = &mockModel{uuid: "uuid", name: "prod", owner: "fred"}
s.mockState.connStatus = &mockConnectionStatus{count: 5}

anotherState := &mockState{modelUUID: "uuid2"}
s.mockStatePool.st["uuid2"] = anotherState
anotherState.applications = map[string]crossmodel.Application{
"mysql": &mockApplication{charm: ch, curl: charm.MustParseURL("mysql-2")},
"postgresql": &mockApplication{charm: ch, curl: charm.MustParseURL("postgresql-2")},
}
anotherState.model = &mockModel{uuid: "uuid2", name: "another", owner: "fred"}
anotherState.usermodels = []crossmodel.UserModel{
&mockUserModel{model: anotherState.model},
}
anotherState.connStatus = &mockConnectionStatus{count: 15}

s.mockState.usermodels = []crossmodel.UserModel{
&mockUserModel{model: s.mockState.model},
&mockUserModel{model: anotherState.model},
}

filter := params.OfferFilters{
Filters: []params.OfferFilter{
{
OfferName: "hosted-db2",
},
{
OfferName: "hosted-mysql",
ModelName: "another",
},
{
OfferName: "hosted-postgresql",
ModelName: "another",
},
},
}
found, err := s.api.FindApplicationOffers(filter)
c.Assert(err, jc.ErrorIsNil)
c.Assert(found, jc.DeepEquals, params.FindApplicationOffersResults{
[]params.ApplicationOffer{
{
ApplicationDescription: "db2 description",
OfferName: "hosted-db2",
OfferURL: "fred/prod.hosted-db2",
Endpoints: []params.RemoteEndpoint{{Name: "db"}},
},
{
ApplicationDescription: "mysql description",
OfferName: "hosted-mysql",
OfferURL: "fred/another.hosted-mysql",
Endpoints: []params.RemoteEndpoint{{Name: "db"}},
},
{
ApplicationDescription: "postgresql description",
OfferName: "hosted-postgresql",
OfferURL: "fred/another.hosted-postgresql",
Endpoints: []params.RemoteEndpoint{{Name: "db"}},
},
},
})
s.applicationOffers.CheckCallNames(c, listOffersBackendCall, listOffersBackendCall)
}

func (s *crossmodelSuite) TestFindError(c *gc.C) {
filter := params.OfferFilters{
Filters: []params.OfferFilter{
Expand All @@ -371,7 +471,7 @@ func (s *crossmodelSuite) TestFindError(c *gc.C) {
}

func (s *crossmodelSuite) TestList(c *gc.C) {
s.setupOffers(c)
s.setupOffers(c, "test")
filter := params.OfferFilters{
Filters: []params.OfferFilter{
{
Expand Down
8 changes: 6 additions & 2 deletions apiserver/crossmodel/mock_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,13 @@ func (m *mockState) RemoteConnectionStatus(offerName string) (crossmodel.RemoteC
}

type mockStatePool struct {
st crossmodel.Backend
st map[string]crossmodel.Backend
}

func (st *mockStatePool) Get(modelUUID string) (crossmodel.Backend, func(), error) {
return st.st, func() {}, nil
backend, ok := st.st[modelUUID]
if !ok {
return nil, nil, errors.NotFoundf("model for uuid %s", modelUUID)
}
return backend, func() {}, nil
}
2 changes: 1 addition & 1 deletion cmd/juju/crossmodel/offer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
const (
offerCommandDoc = `
Deployed application endpoints are offered for use by consumers.
By the default, the offer is named after the application, unless
By default, the offer is named after the application, unless
an offer name is explicitly specified.
Examples:
Expand Down
2 changes: 1 addition & 1 deletion core/crossmodel/interface.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
// ApplicationOffer holds the details of an application offered
// by this model.
type ApplicationOffer struct {
// OfferName is the name of ghe offer.
// OfferName is the name of the offer.
OfferName string

// ApplicationName is the name of the application to which the offer pertains.
Expand Down
2 changes: 2 additions & 0 deletions featuretests/cmd_juju_crossmodel_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,14 @@ func (s *crossmodelSuite) TestListEndpoints(c *gc.C) {
c.Assert(ctx.Stdout.(*bytes.Buffer).String(), gc.Equals, `
kontroll:
riak:
charm: riak
url: admin/controller.riak
endpoints:
endpoint:
interface: http
role: provider
varnish:
charm: varnish
url: admin/controller.varnish
endpoints:
webcache:
Expand Down
Loading

0 comments on commit d337343

Please sign in to comment.