Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Post and Like structure revist #40

Merged
merged 5 commits into from
Nov 19, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ go 1.13
require (
github.com/cosmos/cosmos-sdk v0.37.3
github.com/gorilla/mux v1.7.3
github.com/magiconair/properties v1.8.1
github.com/spf13/cobra v0.0.5
github.com/spf13/viper v1.5.0
github.com/stretchr/testify v1.4.0
Expand Down
1 change: 1 addition & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03
github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU=
github.com/VividCortex/gohistogram v1.0.0 h1:6+hBz+qvs0JOrrNhhmR7lFxo5sINxBCGXrdtl/UvroE=
github.com/VividCortex/gohistogram v1.0.0/go.mod h1:Pf5mBqqDxYaXu3hDrrU+w6nw50o/4+TcAqDqk/vUH7g=
github.com/Workiva/go-datastructures v1.0.50 h1:slDmfW6KCHcC7U+LP3DDBbm4fqTwZGn1beOFPfGaLvo=
github.com/Workiva/go-datastructures v1.0.50/go.mod h1:Z+F2Rca0qCsVYDS8z7bAGm8f3UkzuWYS/oBZz5a7VVA=
github.com/aead/siphash v1.0.1/go.mod h1:Nywa3cDsYNNK3gaciGTWPwHt0wlpNV15vwmswBAUSII=
github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
Expand Down
3 changes: 2 additions & 1 deletion x/magpie/alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ type (
Keeper = keeper.Keeper

// Types
Session = types.Session
SessionID = types.SessionID
Session = types.Session
Sessions = types.Sessions

// Msgs
MsgCreateSession = types.MsgCreateSession
Expand Down
2 changes: 1 addition & 1 deletion x/magpie/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
)

type GenesisState struct {
Sessions []Session `json:"sessions"`
Sessions Sessions `json:"sessions"`
}

// DefaultGenesisState returns a default GenesisState
Expand Down
8 changes: 7 additions & 1 deletion x/magpie/internal/keeper/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,13 @@ func handleMsgCreateSession(ctx sdk.Context, keeper Keeper, msg types.MsgCreateS
Signature: msg.Signature,
}

if err := keeper.CreateSession(ctx, session); err != nil {
// Check for any previously existing session
if _, found := keeper.GetSession(ctx, session.SessionID); found {
return sdk.ErrUnknownRequest(fmt.Sprintf("Session with id %s already exists", session.SessionID)).Result()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big deal but looks strange if a function is named GetSession where the function is to check whether the session exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The GetSession here is used to check if a session already exists, but elsewhere (i.e inside the querier) the same method is used to get a session using its ID.

Here we use the second returned value, which allows for faster and easier comparison than the first one. It's just a convinient way to keep the code DRY and avoiding having multiple methods with very similar functionality

}

// Save the session
if err := keeper.SaveSession(ctx, session); err != nil {
return err.Result()
}

Expand Down
24 changes: 4 additions & 20 deletions x/magpie/internal/keeper/keeper.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package keeper

import (
"fmt"

"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/desmos-labs/desmos/x/magpie/internal/types"
Expand Down Expand Up @@ -48,23 +46,9 @@ func (k Keeper) SetLastSessionID(ctx sdk.Context, id types.SessionID) {
store.Set([]byte(types.LastSessionIDStoreKey), k.Cdc.MustMarshalBinaryBare(&id))
}

// CreateSession allows to create a new session checking that no other session
// with the same id already exist
func (k Keeper) CreateSession(ctx sdk.Context, session types.Session) sdk.Error {
// Check for any previously existing session
if _, found := k.GetSession(ctx, session.SessionID); found {
return sdk.ErrUnknownRequest(fmt.Sprintf("Session with id %s already exists", session.SessionID))
}

return k.SaveSession(ctx, session)
}

// SaveSession allows to save a session inside the given context
// SaveSession allows to save a session inside the given context.
// It assumes the given session has already been validated.
func (k Keeper) SaveSession(ctx sdk.Context, session types.Session) sdk.Error {
if session.Owner.Empty() {
return sdk.ErrInvalidAddress("Owner address cannot be empty")
}

// Save the session
store := ctx.KVStore(k.StoreKey)
store.Set(k.getSessionStoreKey(session.SessionID), k.Cdc.MustMarshalBinaryBare(session))
Expand All @@ -81,7 +65,7 @@ func (k Keeper) GetSession(ctx sdk.Context, id types.SessionID) (session types.S

key := k.getSessionStoreKey(id)
if !store.Has(key) {
return types.NewSession(), false
return types.Session{}, false
}

bz := store.Get(key)
Expand All @@ -90,7 +74,7 @@ func (k Keeper) GetSession(ctx sdk.Context, id types.SessionID) (session types.S
}

// GetSessions returns the list of all the sessions present inside the current context
func (k Keeper) GetSessions(ctx sdk.Context) []types.Session {
func (k Keeper) GetSessions(ctx sdk.Context) types.Sessions {
store := ctx.KVStore(k.StoreKey)
iterator := sdk.KVStorePrefixIterator(store, []byte(types.SessionStorePrefix))

Expand Down
204 changes: 131 additions & 73 deletions x/magpie/internal/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,110 +7,168 @@ import (
"github.com/stretchr/testify/assert"
)

// --------------
// --- Sessions
// --------------
func TestKeeper_GetLastSessionID(t *testing.T) {
tests := []struct {
name string
existingID types.SessionID
expID types.SessionID
}{
{
name: "First ID is returned properly",
expID: types.SessionID(0),
},
{
name: "Existing ID is returned properly",
existingID: types.SessionID(18446744073709551615),
expID: types.SessionID(18446744073709551615),
},
}

func defaultSessionID() types.SessionID {
return types.SessionID(1)
}
for _, test := range tests {
test := test
t.Run(test.name, func(t *testing.T) {
ctx, k := SetupTestInput()

if test.existingID.Valid() {
store := ctx.KVStore(k.StoreKey)
store.Set([]byte(types.LastSessionIDStoreKey), k.Cdc.MustMarshalBinaryBare(test.existingID))
}

assert.Equal(t, test.expID, k.GetLastSessionID(ctx))
})
}

func TestKeeper_GetLastSessionId_FirstId(t *testing.T) {
ctx, k := SetupTestInput()
assert.Equal(t, types.SessionID(0), k.GetLastSessionID(ctx))
}

func TestKeeper_GetLastSessionId_Existing(t *testing.T) {
ctx, k := SetupTestInput()

ids := []types.SessionID{types.SessionID(0), types.SessionID(3), types.SessionID(18446744073709551615)}

store := ctx.KVStore(k.StoreKey)
for _, id := range ids {
store.Set([]byte(types.LastSessionIDStoreKey), k.Cdc.MustMarshalBinaryBare(id))
assert.Equal(t, id, k.GetLastSessionID(ctx))
func TestKeeper_SetLastSessionID(t *testing.T) {
tests := []struct {
id types.SessionID
}{
{id: types.SessionID(0)},
{id: types.SessionID(3)},
{id: types.SessionID(18446744073709551615)},
}
}

func TestKeeper_SetLastSessionId(t *testing.T) {
ctx, k := SetupTestInput()
for _, test := range tests {
test := test
t.Run(t.Name(), func(t *testing.T) {
ctx, k := SetupTestInput()
store := ctx.KVStore(k.StoreKey)

ids := []types.SessionID{types.SessionID(0), types.SessionID(3), types.SessionID(18446744073709551615)}
k.SetLastSessionID(ctx, test.id)

store := ctx.KVStore(k.StoreKey)
for _, id := range ids {
k.SetLastSessionID(ctx, id)
var stored types.SessionID
k.Cdc.MustUnmarshalBinaryBare(store.Get([]byte(types.LastSessionIDStoreKey)), &stored)
assert.Equal(t, id, stored)
var stored types.SessionID
k.Cdc.MustUnmarshalBinaryBare(store.Get([]byte(types.LastSessionIDStoreKey)), &stored)
assert.Equal(t, test.id, stored)
})
}
}

func TestKeeper_CreateSession_ExistingId(t *testing.T) {
func TestKeeper_SaveSession(t *testing.T) {
ctx, k := SetupTestInput()

session := types.Session{SessionID: defaultSessionID()}

store := ctx.KVStore(k.StoreKey)
store.Set([]byte(types.SessionStorePrefix+session.SessionID.String()), k.Cdc.MustMarshalBinaryBare(&session))

err := k.CreateSession(ctx, session)
assert.Error(t, err)
assert.Contains(t, err.Result().Log, "Session with id 1 already exists")
}

func TestKeeper_CreatePost_ValidSession(t *testing.T) {
ctx, k := SetupTestInput()
session := types.Session{Owner: testOwner, SessionID: types.SessionID(1)}

session := types.Session{SessionID: defaultSessionID(), Owner: testOwner}
err := k.CreateSession(ctx, session)
err := k.SaveSession(ctx, session)
assert.NoError(t, err)

var stored types.Session
store := ctx.KVStore(k.StoreKey)
k.Cdc.MustUnmarshalBinaryBare(store.Get([]byte(types.SessionStorePrefix+session.SessionID.String())), &stored)
assert.Equal(t, session, stored)
}

func TestKeeper_SaveSession_EmptyOwner(t *testing.T) {
ctx, k := SetupTestInput()

session := types.Session{SessionID: defaultSessionID()}
err := k.SaveSession(ctx, session)
assert.Error(t, err)
assert.Contains(t, err.Result().Log, "Owner address cannot be empty")
var storedLastId types.SessionID
k.Cdc.MustUnmarshalBinaryBare(store.Get([]byte(types.LastSessionIDStoreKey)), &storedLastId)
assert.Equal(t, session.SessionID, storedLastId)
}

func TestKeeper_SaveSession_ValidSession(t *testing.T) {
ctx, k := SetupTestInput()

session := types.Session{Owner: testOwner, SessionID: defaultSessionID()}

err := k.SaveSession(ctx, session)
assert.NoError(t, err)
func TestKeeper_GetSession(t *testing.T) {
tests := []struct {
name string
storedSession types.Session
id types.SessionID
expFound bool
expSession types.Session
}{
{
name: "Non existent session",
id: types.SessionID(0),
expFound: false,
expSession: types.Session{},
},
{
name: "Valid session is returned",
storedSession: types.Session{Owner: testOwner, SessionID: types.SessionID(1)},
id: types.SessionID(1),
expFound: true,
expSession: types.Session{Owner: testOwner, SessionID: types.SessionID(1)},
},
}

var stored types.Session
store := ctx.KVStore(k.StoreKey)
k.Cdc.MustUnmarshalBinaryBare(store.Get([]byte(types.SessionStorePrefix+session.SessionID.String())), &stored)
assert.Equal(t, session, stored)
}
for _, test := range tests {
test := test
t.Run(test.name, func(t *testing.T) {
ctx, k := SetupTestInput()

func TestKeeper_GetSession_NonExistent(t *testing.T) {
ctx, k := SetupTestInput()
if !(types.Session{}).Equals(test.storedSession) {
store := ctx.KVStore(k.StoreKey)
store.Set([]byte(types.SessionStorePrefix+test.id.String()), k.Cdc.MustMarshalBinaryBare(&test.storedSession))
}

_, found := k.GetSession(ctx, defaultSessionID())
assert.False(t, found)
result, found := k.GetSession(ctx, types.SessionID(1))
assert.Equal(t, test.expSession, result)
assert.Equal(t, test.expFound, found)
})
}
}

func TestKeeper_GetSession_Existent(t *testing.T) {
ctx, k := SetupTestInput()
func TestKeeper_GetSessions(t *testing.T) {
tests := []struct {
name string
storedSessions types.Sessions
expSessions types.Sessions
}{
{
name: "Empty slice",
storedSessions: types.Sessions{},
expSessions: types.Sessions{},
},
{
name: "Non empty, non double items",
storedSessions: types.Sessions{
types.Session{SessionID: types.SessionID(1)},
types.Session{SessionID: types.SessionID(2)},
},
expSessions: types.Sessions{
types.Session{SessionID: types.SessionID(1)},
types.Session{SessionID: types.SessionID(2)},
},
},
{
name: "Non empty, double items",
storedSessions: types.Sessions{
types.Session{SessionID: types.SessionID(1)},
types.Session{SessionID: types.SessionID(1)},
},
expSessions: types.Sessions{
types.Session{SessionID: types.SessionID(1)},
},
},
}

session := types.Session{Owner: testOwner, SessionID: defaultSessionID()}
for _, test := range tests {
test := test
t.Run(test.name, func(t *testing.T) {
ctx, k := SetupTestInput()

store := ctx.KVStore(k.StoreKey)
store.Set([]byte(types.SessionStorePrefix+session.SessionID.String()), k.Cdc.MustMarshalBinaryBare(&session))
for _, s := range test.storedSessions {
_ = k.SaveSession(ctx, s)
}

stored, found := k.GetSession(ctx, session.SessionID)
assert.True(t, found)
assert.Equal(t, session, stored)
sessions := k.GetSessions(ctx)
assert.True(t, test.expSessions.Equals(sessions))
})
}
}
4 changes: 2 additions & 2 deletions x/magpie/internal/keeper/querier.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func NewQuerier(keeper Keeper) sdk.Querier {
case QuerySessions:
return querySession(ctx, path[1:], req, keeper)
default:
return nil, sdk.ErrUnknownRequest("unknown magpie query endpoint")
return nil, sdk.ErrUnknownRequest("Unknown magpie query endpoint")
}
}
}
Expand All @@ -39,7 +39,7 @@ func querySession(ctx sdk.Context, path []string, _ abci.RequestQuery, keeper Ke

session, found := keeper.GetSession(ctx, id)
if !found {
return nil, sdk.ErrUnknownRequest(fmt.Sprintf("Session with id %s not found", id.String()))
return nil, sdk.ErrUnknownRequest(fmt.Sprintf("Session with id %s not found", id))
}

res, err := codec.MarshalJSONIndent(keeper.Cdc, &session)
Expand Down
Loading