Skip to content

Commit

Permalink
Merge pull request juju#17381 from tlm/fix-bad-domain-naming
Browse files Browse the repository at this point in the history
juju#17381

Previous commits introduced a function called "Get" for getting a model from state. As far as naming convensions go this is a poor choice of names. This commit makes the change to GetModel to be more clear about the purpose of the function.

## Checklist

- [x] Code style: imports ordered, good names, simple structure, etc
- ~[ ] Comments saying why design decisions were made~
- [x] Go unit tests, with comments saying what you're testing
- ~[ ] [Integration tests](https://github.com/juju/juju/tree/main/tests), with comments saying what you're testing~
- ~[ ] [doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages~

## QA steps

This is just a name change. If unit tests pass and Juju compiles everything is happy.

## Documentation changes

N/A
  • Loading branch information
jujubot authored May 29, 2024
2 parents e84406f + 15c4d24 commit 5afad50
Show file tree
Hide file tree
Showing 8 changed files with 20 additions and 20 deletions.
2 changes: 1 addition & 1 deletion domain/model/bootstrap/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func CreateReadOnlyModel(
var m coremodel.Model
err := controllerDB.StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error {
var err error
m, err = state.Get(ctx, tx, id)
m, err = state.GetModel(ctx, tx, id)
return err
})
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions domain/model/service/modelservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ type ModelState interface {

// ModelGetterState represents the state required for reading all model information.
type ModelGetterState interface {
Get(context.Context, coremodel.UUID) (coremodel.Model, error)
GetModel(context.Context, coremodel.UUID) (coremodel.Model, error)
}

// ModelService defines a service for interacting with the underlying model
Expand Down Expand Up @@ -65,7 +65,7 @@ func (s *ModelService) CreateModel(
ctx context.Context,
controllerUUID uuid.UUID,
) error {
m, err := s.modelGetterSt.Get(ctx, s.modelID)
m, err := s.modelGetterSt.GetModel(ctx, s.modelID)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion domain/model/service/modelservice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (d *dummyModelState) Create(ctx context.Context, args model.ReadOnlyModelCr
return nil
}

func (d *dummyModelState) Get(ctx context.Context, id coremodel.UUID) (coremodel.Model, error) {
func (d *dummyModelState) GetModel(ctx context.Context, id coremodel.UUID) (coremodel.Model, error) {
args, exists := d.models[id]
if !exists {
return coremodel.Model{}, modelerrors.NotFound
Expand Down
6 changes: 3 additions & 3 deletions domain/model/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ type State interface {
// [modelerrors.AlreadyActivated] error will be returned.
Activate(ctx context.Context, uuid coremodel.UUID) error

// Get returns the model associated with the provided uuid.
Get(context.Context, coremodel.UUID) (coremodel.Model, error)
// GetModel returns the model associated with the provided uuid.
GetModel(context.Context, coremodel.UUID) (coremodel.Model, error)

// GetModelType returns the model type for a model with the provided uuid.
GetModelType(context.Context, coremodel.UUID) (coremodel.ModelType, error)
Expand Down Expand Up @@ -243,7 +243,7 @@ func (s *Service) Model(ctx context.Context, uuid coremodel.UUID) (coremodel.Mod
return coremodel.Model{}, fmt.Errorf("model uuid: %w", err)
}

return s.st.Get(ctx, uuid)
return s.st.GetModel(ctx, uuid)
}

// ModelType returns the current model type based on the cloud name being used
Expand Down
2 changes: 1 addition & 1 deletion domain/model/service/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func (d *dummyState) Activate(
return modelerrors.NotFound
}

func (d *dummyState) Get(
func (d *dummyState) GetModel(
_ context.Context,
uuid coremodel.UUID,
) (coremodel.Model, error) {
Expand Down
10 changes: 5 additions & 5 deletions domain/model/state/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,10 @@ func Create(
return nil
}

// Get returns the model associated with the provided uuid.
// GetModel returns the model associated with the provided uuid.
// If the model does not exist then an error satisfying [modelerrors.NotFound]
// will be returned.
func (s *State) Get(ctx context.Context, uuid coremodel.UUID) (coremodel.Model, error) {
func (s *State) GetModel(ctx context.Context, uuid coremodel.UUID) (coremodel.Model, error) {
db, err := s.DB()
if err != nil {
return coremodel.Model{}, errors.Trace(err)
Expand All @@ -152,7 +152,7 @@ func (s *State) Get(ctx context.Context, uuid coremodel.UUID) (coremodel.Model,
var model coremodel.Model
return model, db.StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error {
var err error
model, err = Get(ctx, tx, uuid)
model, err = GetModel(ctx, tx, uuid)
return err
})
}
Expand Down Expand Up @@ -203,10 +203,10 @@ WHERE uuid = ?
return modelType, nil
}

// Get returns the model associated with the provided uuid.
// GetModel returns the model associated with the provided uuid.
// If the model does not exist then an error satisfying [modelerrors.NotFound]
// will be returned.
func Get(
func GetModel(
ctx context.Context,
tx *sql.Tx,
uuid coremodel.UUID,
Expand Down
12 changes: 6 additions & 6 deletions domain/model/state/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ func (m *stateSuite) TestGetModel(c *gc.C) {
runner := m.TxnRunnerFactory()

modelSt := NewState(runner)
modelInfo, err := modelSt.Get(context.Background(), m.uuid)
modelInfo, err := modelSt.GetModel(context.Background(), m.uuid)
c.Assert(err, jc.ErrorIsNil)
c.Check(modelInfo, gc.Equals, coremodel.Model{
AgentVersion: jujuversion.Current,
Expand Down Expand Up @@ -293,7 +293,7 @@ func (m *stateSuite) TestGetModelType(c *gc.C) {
func (m *stateSuite) TestGetModelNotFound(c *gc.C) {
runner := m.TxnRunnerFactory()
modelSt := NewState(runner)
_, err := modelSt.Get(context.Background(), modeltesting.GenModelUUID(c))
_, err := modelSt.GetModel(context.Background(), modeltesting.GenModelUUID(c))
c.Assert(err, jc.ErrorIs, modelerrors.NotFound)
}

Expand Down Expand Up @@ -410,7 +410,7 @@ func (m *stateSuite) TestCreateWithEmptyRegion(c *gc.C) {
err = modelSt.Activate(context.Background(), testUUID)
c.Assert(err, jc.ErrorIsNil)

modelInfo, err := modelSt.Get(context.Background(), testUUID)
modelInfo, err := modelSt.GetModel(context.Background(), testUUID)
c.Assert(err, jc.ErrorIsNil)
c.Check(modelInfo.CloudRegion, gc.Equals, "")
}
Expand Down Expand Up @@ -457,7 +457,7 @@ func (m *stateSuite) TestCreateWithEmptyRegionUsesControllerRegion(c *gc.C) {
err = modelSt.Activate(context.Background(), testUUID)
c.Assert(err, jc.ErrorIsNil)

modelInfo, err := modelSt.Get(context.Background(), testUUID)
modelInfo, err := modelSt.GetModel(context.Background(), testUUID)
c.Assert(err, jc.ErrorIsNil)
c.Check(modelInfo.CloudRegion, gc.Equals, "my-region")
}
Expand Down Expand Up @@ -488,7 +488,7 @@ func (m *stateSuite) TestCreateWithEmptyRegionDoesNotUseControllerRegionForDiffe
err = modelSt.Activate(context.Background(), controllerUUID)
c.Assert(err, jc.ErrorIsNil)

modelInfo, err := modelSt.Get(context.Background(), controllerUUID)
modelInfo, err := modelSt.GetModel(context.Background(), controllerUUID)
c.Assert(err, jc.ErrorIsNil)
c.Check(modelInfo.CloudRegion, gc.Equals, "my-region")

Expand All @@ -513,7 +513,7 @@ func (m *stateSuite) TestCreateWithEmptyRegionDoesNotUseControllerRegionForDiffe
err = modelSt.Activate(context.Background(), testUUID)
c.Assert(err, jc.ErrorIsNil)

modelInfo, err = modelSt.Get(context.Background(), testUUID)
modelInfo, err = modelSt.GetModel(context.Background(), testUUID)
c.Assert(err, jc.ErrorIsNil)

// We should never set the region to the controller region if the cloud
Expand Down
2 changes: 1 addition & 1 deletion domain/modelconfig/bootstrap/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func SetModelConfig(
var m coremodel.Model
err = controller.StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error {
var err error
m, err = modelstate.Get(ctx, tx, modelID)
m, err = modelstate.GetModel(ctx, tx, modelID)
return err
})

Expand Down

0 comments on commit 5afad50

Please sign in to comment.