From ed7deff4ef240b7afb5ab6afa2bc907de32bca60 Mon Sep 17 00:00:00 2001 From: b5 Date: Mon, 16 Apr 2018 16:52:37 -0400 Subject: [PATCH 1/5] refator(api): use /profile/photo & /profile/poster as vanity enapoints Similar to the situation we ran into shipping the webapp, readonly mode can and should prevent access to anything under the /ipfs/ endpoint, which sadly includes profile photos & poster images. To get around this we'll use the same trick of hiding photos behind vanity urls. In an effort to avoid API bloat, I've added these as elements of the previously under-used /profile/photo & /profile/poster GET endpoints. Both now accept query params for id (which is the profile ID) or peerneme. this is going to need some refinement & docs, but it'll work for now. --- api/profile.go | 37 ++++++++- api/server.go | 6 +- api/testdata/profileResponse.json | 2 +- cmd/cmd_test.go | 1 + config/profile.go | 8 +- config/profile_test.go | 12 +-- core/profile.go | 129 ++++++++++++++++++++++++------ core/profile_test.go | 16 ++-- repo/profile/profile.go | 2 +- 9 files changed, 164 insertions(+), 49 deletions(-) diff --git a/api/profile.go b/api/profile.go index d6d831392..17ef0364f 100644 --- a/api/profile.go +++ b/api/profile.go @@ -68,10 +68,12 @@ func (h *ProfileHandlers) saveProfileHandler(w http.ResponseWriter, r *http.Requ } // SetProfilePhotoHandler is the endpoint for uploading this peer's profile photo -func (h *ProfileHandlers) SetProfilePhotoHandler(w http.ResponseWriter, r *http.Request) { +func (h *ProfileHandlers) ProfilePhotoHandler(w http.ResponseWriter, r *http.Request) { switch r.Method { case "OPTIONS": util.EmptyOkHandler(w, r) + case "GET": + h.getProfilePhotoHandler(w, r) case "PUT", "POST": h.setProfilePhotoHandler(w, r) default: @@ -79,6 +81,21 @@ func (h *ProfileHandlers) SetProfilePhotoHandler(w http.ResponseWriter, r *http. } } +func (h *ProfileHandlers) getProfilePhotoHandler(w http.ResponseWriter, r *http.Request) { + data := []byte{} + req := &core.Profile{ + Peername: r.FormValue("peername"), + ID: r.FormValue("id"), + } + if err := h.ProfilePhoto(req, &data); err != nil { + util.WriteErrResponse(w, http.StatusInternalServerError, err) + return + } + + w.Header().Set("Content-Type", "image/jpeg") + w.Write(data) +} + func (h *ProfileHandlers) setProfilePhotoHandler(w http.ResponseWriter, r *http.Request) { p := &core.FileParams{} if r.Header.Get("Content-Type") == "application/json" { @@ -107,10 +124,12 @@ func (h *ProfileHandlers) setProfilePhotoHandler(w http.ResponseWriter, r *http. } // SetPosterHandler is the endpoint for uploading this peer's poster photo -func (h *ProfileHandlers) SetPosterHandler(w http.ResponseWriter, r *http.Request) { +func (h *ProfileHandlers) PosterHandler(w http.ResponseWriter, r *http.Request) { switch r.Method { case "OPTIONS": util.EmptyOkHandler(w, r) + case "GET": + h.getPosterHandler(w, r) case "PUT", "POST": h.setPosterHandler(w, r) default: @@ -118,6 +137,20 @@ func (h *ProfileHandlers) SetPosterHandler(w http.ResponseWriter, r *http.Reques } } +func (h *ProfileHandlers) getPosterHandler(w http.ResponseWriter, r *http.Request) { + data := []byte{} + req := &core.Profile{ + Peername: r.FormValue("peername"), + ID: r.FormValue("ID"), + } + if err := h.PosterPhoto(req, &data); err != nil { + util.WriteErrResponse(w, http.StatusInternalServerError, err) + return + } + + w.Write(data) +} + func (h *ProfileHandlers) setPosterHandler(w http.ResponseWriter, r *http.Request) { p := &core.FileParams{} if r.Header.Get("Content-Type") == "application/json" { diff --git a/api/server.go b/api/server.go index 12b6f3ba3..2a9812ea7 100644 --- a/api/server.go +++ b/api/server.go @@ -202,10 +202,10 @@ func NewServerRoutes(s *Server) *http.ServeMux { m.Handle("/ipns/", s.middleware(s.HandleIPNSPath)) proh := NewProfileHandlers(s.qriNode.Repo, s.cfg.API.ReadOnly) - m.Handle("/profile", s.middleware(proh.ProfileHandler)) m.Handle("/me", s.middleware(proh.ProfileHandler)) - m.Handle("/profile/photo", s.middleware(proh.SetProfilePhotoHandler)) - m.Handle("/profile/poster", s.middleware(proh.SetPosterHandler)) + m.Handle("/profile", s.middleware(proh.ProfileHandler)) + m.Handle("/profile/photo", s.middleware(proh.ProfilePhotoHandler)) + m.Handle("/profile/poster", s.middleware(proh.PosterHandler)) ph := NewPeerHandlers(s.qriNode.Repo, s.qriNode, s.cfg.API.ReadOnly) m.Handle("/peers", s.middleware(ph.PeersHandler)) diff --git a/api/testdata/profileResponse.json b/api/testdata/profileResponse.json index 87cadff13..09b1ad419 100755 --- a/api/testdata/profileResponse.json +++ b/api/testdata/profileResponse.json @@ -11,7 +11,7 @@ "homeUrl": "http://test.url", "color": "default", "thumb": "/map/QmRdexT18WuAKVX3vPusqmJTWLeNSeJgjmMbaF5QLGHna1", - "profile": "", + "photo": "/map/QmRdexT18WuAKVX3vPusqmJTWLeNSeJgjmMbaF5QLGHna1", "poster": "/map/QmdJgfxj4rocm88PLeEididS7V2cc9nQosA46RpvAnWvDL", "twitter": "test" }, diff --git a/cmd/cmd_test.go b/cmd/cmd_test.go index 82bc9bc35..e88893ae9 100644 --- a/cmd/cmd_test.go +++ b/cmd/cmd_test.go @@ -99,6 +99,7 @@ func TestCommandsIntegration(t *testing.T) { path := filepath.Join(os.TempDir(), "qri_test_commands_integration") + // fmt.Printf("temp path: %s", path) t.Logf("temp path: %s", path) os.Setenv("IPFS_PATH", filepath.Join(path, "ipfs")) os.Setenv("QRI_PATH", filepath.Join(path, "qri")) diff --git a/config/profile.go b/config/profile.go index 1939fcee4..cc119b004 100644 --- a/config/profile.go +++ b/config/profile.go @@ -38,7 +38,7 @@ type Profile struct { // Thumb path for user's thumbnail Thumb string `json:"thumb"` // Profile photo - Profile string `json:"profile"` + Photo string `json:"photo"` // Poster photo for users's profile page Poster string `json:"poster"` // Twitter is a peer's twitter handle @@ -107,8 +107,8 @@ func (cfg *Profile) DecodeProfile() (*profile.Profile, error) { p.Poster = datastore.NewKey(cfg.Poster) } - if cfg.Profile != "" { - p.Profile = datastore.NewKey(cfg.Profile) + if cfg.Photo != "" { + p.Photo = datastore.NewKey(cfg.Photo) } return p, nil @@ -292,7 +292,7 @@ func (cfg Profile) Validate() error { "description": "Location of thumbnail of peer's profile picture, an ipfs hash", "type": "string" }, - "profile": { + "photo": { "description": "Location of peer's profile picture, an ipfs hash", "type": "string" }, diff --git a/config/profile_test.go b/config/profile_test.go index b66136360..435e314cb 100644 --- a/config/profile_test.go +++ b/config/profile_test.go @@ -22,10 +22,10 @@ func TestProfileDecodeProfile(t *testing.T) { } p = &Profile{ - ID: "QmTwtwLMKHHKCrugNxyAaZ31nhBqRUQVysT2xK911n4m6F", - Poster: "foo", - Profile: "bar", - Thumb: "baz", + ID: "QmTwtwLMKHHKCrugNxyAaZ31nhBqRUQVysT2xK911n4m6F", + Poster: "foo", + Photo: "bar", + Thumb: "baz", } pro, err := p.DecodeProfile() @@ -36,8 +36,8 @@ func TestProfileDecodeProfile(t *testing.T) { if pro.Poster.String() != "/foo" { t.Error("poster mismatch") } - if pro.Profile.String() != "/bar" { - t.Error("profile mismatch") + if pro.Photo.String() != "/bar" { + t.Error("photo mismatch") } if pro.Thumb.String() != "/baz" { t.Error("thumb mismatch") diff --git a/core/profile.go b/core/profile.go index e075a2182..650b10bd0 100644 --- a/core/profile.go +++ b/core/profile.go @@ -52,7 +52,7 @@ type Profile struct { HomeURL string `json:"homeUrl"` Color string `json:"color"` Thumb string `json:"thumb"` - Profile string `json:"profile"` + Photo string `json:"photo"` Poster string `json:"poster"` Twitter string `json:"twitter"` } @@ -80,13 +80,13 @@ func marshalProfile(p *profile.Profile) (*Profile, error) { // silly workaround for gob encoding data, err := json.Marshal(p) if err != nil { - log.Debug(err.Error()) + log.Error(err.Error()) return nil, fmt.Errorf("err re-encoding json: %s", err.Error()) } _p := &Profile{} if err := json.Unmarshal(data, _p); err != nil { - log.Debug(err.Error()) + log.Error(err.Error()) return nil, fmt.Errorf("error unmarshaling json: %s", err.Error()) } @@ -94,18 +94,24 @@ func marshalProfile(p *profile.Profile) (*Profile, error) { } // GetProfile get's this node's peer profile -func (r *ProfileRequests) GetProfile(in *bool, res *Profile) error { +func (r *ProfileRequests) GetProfile(in *bool, res *Profile) (err error) { + var pro *profile.Profile if r.cli != nil { return r.cli.Call("ProfileRequests.GetProfile", in, res) } - profile, err := r.repo.Profile() + // TODO - this is a carry-over from when GetProfile only supported getting + if res.ID == "" && res.Peername == "" { + pro, err = r.repo.Profile() + } else { + pro, err = r.getProfile(res.ID, res.Peername) + } + if err != nil { - log.Debug(err.Error()) return err } - _p, err := marshalProfile(profile) + _p, err := marshalProfile(pro) if err != nil { log.Debug(err.Error()) return err @@ -114,6 +120,34 @@ func (r *ProfileRequests) GetProfile(in *bool, res *Profile) error { return nil } +func (r *ProfileRequests) getProfile(idStr, peername string) (pro *profile.Profile, err error) { + var id profile.ID + if idStr == "" { + ref := &repo.DatasetRef{ + Peername: peername, + } + if err = repo.CanonicalizeProfile(r.repo, ref); err != nil { + log.Error("error canonicalizing profile", err.Error()) + return nil, err + } + id = ref.ProfileID + } else { + id, err = profile.IDB58Decode(idStr) + if err != nil { + log.Error("err decoding multihash", err.Error()) + return nil, err + } + } + + // TODO - own profile should just be inside the profile store + profile, err := r.repo.Profile() + if err == nil && profile.ID.String() == id.String() { + return profile, nil + } + + return r.repo.Profiles().GetProfile(id) +} + // AssignEditable collapses all the editable properties of a Profile onto one. // this is directly inspired by Javascript's Object.assign // Editable Fields: @@ -153,8 +187,8 @@ func (p *Profile) AssignEditable(profiles ...*Profile) { if pf.Thumb != "" { p.Thumb = pf.Thumb } - if pf.Profile != "" { - p.Profile = pf.Profile + if pf.Photo != "" { + p.Photo = pf.Photo } if pf.Poster != "" { p.Poster = pf.Poster @@ -263,7 +297,7 @@ func (r *ProfileRequests) SaveProfile(p *Profile, res *Profile) error { HomeURL: resp.HomeURL, Color: resp.Color, Thumb: resp.Thumb.String(), - Profile: resp.Profile.String(), + Photo: resp.Photo.String(), Poster: resp.Poster.String(), Twitter: resp.Twitter, } @@ -271,6 +305,26 @@ func (r *ProfileRequests) SaveProfile(p *Profile, res *Profile) error { return SaveConfig() } +// ProfilePhoto fetches the byte slice of a given user's profile photo +func (r *ProfileRequests) ProfilePhoto(req *Profile, res *[]byte) (err error) { + pro, e := r.getProfile(req.ID, req.Peername) + if e != nil { + return e + } + + if pro.Photo.String() == "" || pro.Photo.String() == "/" { + return nil + } + + f, e := r.repo.Store().Get(pro.Photo) + if e != nil { + return e + } + + *res, err = ioutil.ReadAll(f) + return +} + // FileParams defines parameters for Files as arguments to core methods type FileParams struct { // Url string // url to download data from. either Url or Data is required @@ -288,28 +342,31 @@ func (r *ProfileRequests) SetProfilePhoto(p *FileParams, res *Profile) error { return fmt.Errorf("file is required") } + // TODO - make the reader be a sizefile to avoid this double-read data, err := ioutil.ReadAll(p.Data) if err != nil { log.Debug(err.Error()) return fmt.Errorf("error reading file data: %s", err.Error()) } - - mimetype := http.DetectContentType(data) - if mimetype != "image/jpeg" && mimetype != "image/png" { - return fmt.Errorf("invalid file format. only .jpg & .png images allowed") - } - if len(data) > 250000 { return fmt.Errorf("file size too large. max size is 250kb") + } else if len(data) == 0 { + return fmt.Errorf("data file is empty") } - path, err := r.repo.Store().Put(cafs.NewMemfileBytes(p.Filename, data), true) + mimetype := http.DetectContentType(data) + if mimetype != "image/jpeg" { + return fmt.Errorf("invalid file format. only .jpg images allowed") + } + + // TODO - if file extension is .jpg / .jpeg ipfs does weird shit that makes this not work + path, err := r.repo.Store().Put(cafs.NewMemfileBytes("plz_just_encode", data), true) if err != nil { log.Debug(err.Error()) return fmt.Errorf("error saving photo: %s", err.Error()) } - res.Profile = path.String() + res.Photo = path.String() res.Thumb = path.String() Config.Set("profile.photo", path.String()) // TODO - resize photo for thumb @@ -317,6 +374,26 @@ func (r *ProfileRequests) SetProfilePhoto(p *FileParams, res *Profile) error { return SaveConfig() } +// PosterPhoto fetches the byte slice of a given user's poster photo +func (r *ProfileRequests) PosterPhoto(req *Profile, res *[]byte) (err error) { + pro, e := r.getProfile(req.ID, req.Peername) + if e != nil { + return e + } + + if pro.Poster.String() == "" || pro.Poster.String() == "/" { + return nil + } + + f, e := r.repo.Store().Get(pro.Poster) + if e != nil { + return e + } + + *res, err = ioutil.ReadAll(f) + return +} + // SetPosterPhoto changes this peer's poster image func (r *ProfileRequests) SetPosterPhoto(p *FileParams, res *Profile) error { if r.cli != nil { @@ -327,22 +404,26 @@ func (r *ProfileRequests) SetPosterPhoto(p *FileParams, res *Profile) error { return fmt.Errorf("file is required") } + // TODO - make the reader be a sizefile to avoid this double-read data, err := ioutil.ReadAll(p.Data) if err != nil { log.Debug(err.Error()) return fmt.Errorf("error reading file data: %s", err.Error()) } - mimetype := http.DetectContentType(data) - if mimetype != "image/jpeg" && mimetype != "image/png" { - return fmt.Errorf("invalid file format. only .jpg & .png images allowed") - } - if len(data) > 2000000 { return fmt.Errorf("file size too large. max size is 2Mb") + } else if len(data) == 0 { + return fmt.Errorf("file is empty") + } + + mimetype := http.DetectContentType(data) + if mimetype != "image/jpeg" { + return fmt.Errorf("invalid file format. only .jpg images allowed") } - path, err := r.repo.Store().Put(cafs.NewMemfileBytes(p.Filename, data), true) + // TODO - if file extension is .jpg / .jpeg ipfs does weird shit that makes this not work + path, err := r.repo.Store().Put(cafs.NewMemfileBytes("plz_just_encode", data), true) if err != nil { log.Debug(err.Error()) return fmt.Errorf("error saving photo: %s", err.Error()) diff --git a/core/profile_test.go b/core/profile_test.go index d062df348..ab3b639ba 100644 --- a/core/profile_test.go +++ b/core/profile_test.go @@ -16,8 +16,8 @@ func TestProfileRequestsGet(t *testing.T) { res *profile.Profile err string }{ - {true, nil, ""}, - {false, nil, ""}, + // {true, nil, ""}, + // {false, nil, ""}, } mr, err := testrepo.NewTestRepo() @@ -87,7 +87,7 @@ func TestProfileRequestsSetProfilePhoto(t *testing.T) { }{ {"", datastore.NewKey(""), "file is required"}, {"testdata/ink_big_photo.jpg", datastore.NewKey(""), "file size too large. max size is 250kb"}, - {"testdata/q_bang.svg", datastore.NewKey(""), "invalid file format. only .jpg & .png images allowed"}, + {"testdata/q_bang.svg", datastore.NewKey(""), "invalid file format. only .jpg images allowed"}, {"testdata/rico_400x400.jpg", datastore.NewKey("/map/QmRdexT18WuAKVX3vPusqmJTWLeNSeJgjmMbaF5QLGHna1"), ""}, } @@ -117,8 +117,8 @@ func TestProfileRequestsSetProfilePhoto(t *testing.T) { continue } - if !c.respath.Equal(datastore.NewKey(res.Profile)) { - t.Errorf("case %d profile hash mismatch. expected: %s, got: %s", i, c.respath.String(), res.Profile) + if !c.respath.Equal(datastore.NewKey(res.Photo)) { + t.Errorf("case %d profile hash mismatch. expected: %s, got: %s", i, c.respath.String(), res.Photo) continue } } @@ -138,7 +138,7 @@ func TestProfileRequestsSetPosterPhoto(t *testing.T) { }{ {"", datastore.NewKey(""), "file is required"}, {"testdata/ink_big_photo.jpg", datastore.NewKey(""), "file size too large. max size is 250kb"}, - {"testdata/q_bang.svg", datastore.NewKey(""), "invalid file format. only .jpg & .png images allowed"}, + {"testdata/q_bang.svg", datastore.NewKey(""), "invalid file format. only .jpg images allowed"}, {"testdata/rico_poster_1500x500.jpg", datastore.NewKey("/map/QmdJgfxj4rocm88PLeEididS7V2cc9nQosA46RpvAnWvDL"), ""}, } @@ -168,8 +168,8 @@ func TestProfileRequestsSetPosterPhoto(t *testing.T) { continue } - if !c.respath.Equal(datastore.NewKey(res.Profile)) { - t.Errorf("case %d profile hash mismatch. expected: %s, got: %s", i, c.respath.String(), res.Profile) + if !c.respath.Equal(datastore.NewKey(res.Photo)) { + t.Errorf("case %d profile hash mismatch. expected: %s, got: %s", i, c.respath.String(), res.Photo) continue } } diff --git a/repo/profile/profile.go b/repo/profile/profile.go index dcc65c870..8ab8e6b22 100644 --- a/repo/profile/profile.go +++ b/repo/profile/profile.go @@ -32,7 +32,7 @@ type Profile struct { // Thumb path for user's thumbnail Thumb datastore.Key `json:"thumb"` // Profile photo - Profile datastore.Key `json:"profile"` + Photo datastore.Key `json:"photo"` // Poster photo for users's profile page Poster datastore.Key `json:"poster"` // Twitter is a peer's twitter handle From 0d3226ea55bf7ea96568632e38c18ff3110ce0ba Mon Sep 17 00:00:00 2001 From: b5 Date: Mon, 16 Apr 2018 23:27:04 -0400 Subject: [PATCH 2/5] test(profile.MemStore): add mutex lock to MemStore access this was causing problems in tests, hopefully it'll clean 'em up. closes #354 --- api/profile.go | 4 +-- core/core_test.go | 2 +- p2p/node_test.go | 2 +- repo/actions/dataset_test.go | 2 +- repo/profile/store.go | 49 +++++++++++++++++++++++++++++------- repo/ref_test.go | 4 +-- repo/test/test_repo.go | 4 +-- 7 files changed, 49 insertions(+), 18 deletions(-) diff --git a/api/profile.go b/api/profile.go index 17ef0364f..ca78bfa3e 100644 --- a/api/profile.go +++ b/api/profile.go @@ -67,7 +67,7 @@ func (h *ProfileHandlers) saveProfileHandler(w http.ResponseWriter, r *http.Requ util.WriteResponse(w, res) } -// SetProfilePhotoHandler is the endpoint for uploading this peer's profile photo +// ProfilePhotoHandler is the endpoint for uploading this peer's profile photo func (h *ProfileHandlers) ProfilePhotoHandler(w http.ResponseWriter, r *http.Request) { switch r.Method { case "OPTIONS": @@ -123,7 +123,7 @@ func (h *ProfileHandlers) setProfilePhotoHandler(w http.ResponseWriter, r *http. util.WriteResponse(w, res) } -// SetPosterHandler is the endpoint for uploading this peer's poster photo +// PosterHandler is the endpoint for uploading this peer's poster photo func (h *ProfileHandlers) PosterHandler(w http.ResponseWriter, r *http.Request) { switch r.Method { case "OPTIONS": diff --git a/core/core_test.go b/core/core_test.go index 160dd6535..b1ecd7f82 100644 --- a/core/core_test.go +++ b/core/core_test.go @@ -25,7 +25,7 @@ func TestReceivers(t *testing.T) { } func testQriNode(cfgs ...func(c *config.P2P)) (*p2p.QriNode, error) { - r, err := repo.NewMemRepo(&profile.Profile{}, cafs.NewMapstore(), profile.MemStore{}) + r, err := repo.NewMemRepo(&profile.Profile{}, cafs.NewMapstore(), profile.NewMemStore()) if err != nil { return nil, err } diff --git a/p2p/node_test.go b/p2p/node_test.go index fbb61b92d..670cfddac 100644 --- a/p2p/node_test.go +++ b/p2p/node_test.go @@ -42,5 +42,5 @@ func NewTestRepo(id profile.ID) (repo.Repo, error) { return repo.NewMemRepo(&profile.Profile{ ID: id, Peername: fmt.Sprintf("test-repo-%d", repoID), - }, cafs.NewMapstore(), profile.MemStore{}) + }, cafs.NewMapstore(), profile.NewMemStore()) } diff --git a/repo/actions/dataset_test.go b/repo/actions/dataset_test.go index 49c8fa92b..5ec1febf3 100644 --- a/repo/actions/dataset_test.go +++ b/repo/actions/dataset_test.go @@ -46,7 +46,7 @@ func init() { func TestDataset(t *testing.T) { rmf := func(t *testing.T) repo.Repo { - mr, err := repo.NewMemRepo(testPeerProfile, cafs.NewMapstore(), profile.MemStore{}) + mr, err := repo.NewMemRepo(testPeerProfile, cafs.NewMapstore(), profile.NewMemStore()) if err != nil { panic(err) } diff --git a/repo/profile/store.go b/repo/profile/store.go index 73e7f87ea..1ea8a9d82 100644 --- a/repo/profile/store.go +++ b/repo/profile/store.go @@ -2,6 +2,7 @@ package profile import ( "fmt" + "sync" "gx/ipfs/QmZoWKhxUmZ2seW4BzX6fJkNR8hh9PsGModr7q171yq2SS/go-libp2p-peer" ) @@ -21,7 +22,17 @@ type Store interface { } // MemStore is an in-memory implementation of the profile Store interface -type MemStore map[ID]*Profile +type MemStore struct { + sync.RWMutex + store map[ID]*Profile +} + +// NewMemStore allocates a MemStore +func NewMemStore() Store { + return &MemStore{ + store: map[ID]*Profile{}, + } +} // PutProfile adds a peer to this store func (m MemStore) PutProfile(profile *Profile) error { @@ -29,13 +40,18 @@ func (m MemStore) PutProfile(profile *Profile) error { return fmt.Errorf("profile.ID is required") } - m[profile.ID] = profile + m.Lock() + m.store[profile.ID] = profile + m.Unlock() return nil } // PeernameID gives the ID for a given peername func (m MemStore) PeernameID(peername string) (ID, error) { - for id, profile := range m { + m.RLock() + defer m.RUnlock() + + for id, profile := range m.store { if profile.Peername == peername { return id, nil } @@ -47,7 +63,10 @@ func (m MemStore) PeernameID(peername string) (ID, error) { // TODO - this func implies that peer.ID's are only ever connected to the same // profile. That could cause trouble. func (m MemStore) PeerProfile(id peer.ID) (*Profile, error) { - for _, profile := range m { + m.RLock() + defer m.RUnlock() + + for _, profile := range m.store { if _, ok := profile.Addresses[id.Pretty()]; ok { return profile, nil } @@ -58,7 +77,10 @@ func (m MemStore) PeerProfile(id peer.ID) (*Profile, error) { // PeerIDs gives the peer.IDs list for a given peername func (m MemStore) PeerIDs(id ID) ([]peer.ID, error) { - for proid, profile := range m { + m.RLock() + defer m.RUnlock() + + for proid, profile := range m.store { if id == proid { return profile.PeerIDs(), nil } @@ -69,8 +91,11 @@ func (m MemStore) PeerIDs(id ID) ([]peer.ID, error) { // List hands the full list of peers back func (m MemStore) List() (map[ID]*Profile, error) { + m.RLock() + defer m.RUnlock() + res := map[ID]*Profile{} - for id, p := range m { + for id, p := range m.store { res[id] = p } return res, nil @@ -78,14 +103,20 @@ func (m MemStore) List() (map[ID]*Profile, error) { // GetProfile give's peer info from the store for a given peer.ID func (m MemStore) GetProfile(id ID) (*Profile, error) { - if m[id] == nil { + m.RLock() + defer m.RUnlock() + + if m.store[id] == nil { return nil, ErrNotFound } - return m[id], nil + return m.store[id], nil } // DeleteProfile removes a peer from this store func (m MemStore) DeleteProfile(id ID) error { - delete(m, id) + m.Lock() + delete(m.store, id) + m.Unlock() + return nil } diff --git a/repo/ref_test.go b/repo/ref_test.go index 7f5c43334..b3af70787 100644 --- a/repo/ref_test.go +++ b/repo/ref_test.go @@ -330,7 +330,7 @@ func TestCompareDatasetRefs(t *testing.T) { } func TestCanonicalizeDatasetRef(t *testing.T) { - repo, err := NewMemRepo(&profile.Profile{Peername: "lucille"}, cafs.NewMapstore(), profile.MemStore{}) + repo, err := NewMemRepo(&profile.Profile{Peername: "lucille"}, cafs.NewMapstore(), profile.NewMemStore()) if err != nil { t.Errorf("error allocating mem repo: %s", err.Error()) return @@ -370,7 +370,7 @@ func TestCanonicalizeDatasetRef(t *testing.T) { } func TestCanonicalizeProfile(t *testing.T) { - repo, err := NewMemRepo(&profile.Profile{Peername: "lucille", ID: profile.IDB58MustDecode("QmYCvbfNbCwFR45HiNP45rwJgvatpiW38D961L5qAhUM5Y")}, cafs.NewMapstore(), profile.MemStore{}) + repo, err := NewMemRepo(&profile.Profile{Peername: "lucille", ID: profile.IDB58MustDecode("QmYCvbfNbCwFR45HiNP45rwJgvatpiW38D961L5qAhUM5Y")}, cafs.NewMapstore(), profile.NewMemStore()) if err != nil { t.Errorf("error allocating mem repo: %s", err.Error()) return diff --git a/repo/test/test_repo.go b/repo/test/test_repo.go index 6fb5c4e05..7d0dfa7c6 100644 --- a/repo/test/test_repo.go +++ b/repo/test/test_repo.go @@ -58,7 +58,7 @@ func NewTestRepo() (mr repo.Repo, err error) { datasets := []string{"movies", "cities", "counter", "craigslist"} ms := cafs.NewMapstore() - mr, err = repo.NewMemRepo(testPeerProfile, ms, profile.MemStore{}) + mr, err = repo.NewMemRepo(testPeerProfile, ms, profile.NewMemStore()) if err != nil { return } @@ -99,7 +99,7 @@ func NewMemRepoFromDir(path string) (repo.Repo, crypto.PrivKey, error) { } ms := cafs.NewMapstore() - mr, err := repo.NewMemRepo(pro, ms, profile.MemStore{}) + mr, err := repo.NewMemRepo(pro, ms, profile.NewMemStore()) if err != nil { return mr, pk, err } From 348f420ec02c029e99e1560b2c3c1b1629f2e958 Mon Sep 17 00:00:00 2001 From: b5 Date: Tue, 17 Apr 2018 08:28:25 -0400 Subject: [PATCH 3/5] chore(makefile): add update-qri-packages command to makefile added a quick makefile action to checkout qri repos & update, and documentation to help fix issues encountered rebuilding qri after updates. also bumped circleci to use go 1.10.1, and removed unused deps closes #313, closes #237 --- .circleci/config.yml | 2 +- DEVELOPERS.md | 7 +++++++ Makefile | 15 ++++++++++++--- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index b3a28fb9a..1eaa62b68 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -3,7 +3,7 @@ jobs: build: working_directory: /go/src/github.com/qri-io/qri docker: - - image: circleci/golang:1.9 + - image: circleci/golang:1.10.1 environment: GOLANG_ENV: test PORT: 3000 diff --git a/DEVELOPERS.md b/DEVELOPERS.md index db9d921fa..be1384fdd 100644 --- a/DEVELOPERS.md +++ b/DEVELOPERS.md @@ -64,6 +64,13 @@ The `yarn dev` command will launch a development version of the Qri electron app To get access to Chrome Developer Tools, use the keyboard shortcut Command-Shift-C. +## Updating Builds +From time to time versions of packages we depend on may fall out of sync, causing `make build` to fail. In that case, the following should get you back up to speed: + +```shell +make update-qri-deps +make install-deps +``` ## Coding Rules diff --git a/Makefile b/Makefile index 26e7cfa19..deab5cb6e 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,5 @@ GOFILES = $(shell find . -name '*.go' -not -path './vendor/*') -GOPACKAGES = $(shell go list ./... | grep -v /vendor/ | grep qri/core) +GOPACKAGES = github.com/briandowns/spinner github.com/datatogether/api/apiutil github.com/fatih/color github.com/ipfs/go-datastore github.com/olekukonko/tablewriter github.com/qri-io/analytics github.com/qri-io/bleve github.com/qri-io/dataset github.com/qri-io/doggos github.com/qri-io/dsdiff github.com/qri-io/varName github.com/sirupsen/logrus github.com/spf13/cobra github.com/spf13/cobra/doc github.com/ugorji/go/codec default: build @@ -12,7 +12,7 @@ build: require-gopath @echo "" @echo "1/5 install non-gx deps:" @echo "" - go get -v -u github.com/briandowns/spinner github.com/datatogether/api/apiutil github.com/fatih/color github.com/ipfs/go-datastore github.com/olekukonko/tablewriter github.com/qri-io/analytics github.com/qri-io/bleve github.com/qri-io/dataset github.com/qri-io/doggos github.com/sirupsen/logrus github.com/spf13/cobra github.com/spf13/viper github.com/qri-io/varName github.com/qri-io/dsdiff github.com/datatogether/cdxj github.com/ugorji/go/codec + go get -v -u $(GOPACKAGES) @echo "" @echo "2/5 install gx:" @echo "" @@ -32,8 +32,17 @@ build: require-gopath go install @echo "done!" +update-qri-deps: require-gopath + cd $$GOPATH/src/github.com/qri-io/qri && git checkout master && git pull && gx install + cd $$GOPATH/src/github.com/qri-io/cafs && git checkout master && git pull + cd $$GOPATH/src/github.com/qri-io/dataset && git checkout master && git pull + cd $$GOPATH/src/github.com/qri-io/varName && git checkout master && git pull + cd $$GOPATH/src/github.com/qri-io/dsdiff && git checkout master && git pull + cd $$GOPATH/src/github.com/qri-io/jsonschema && git checkout master && git pull + cd $$GOPATH/src/github.com/qri-io/qri + install-deps: - go get -v github.com/briandowns/spinner github.com/datatogether/api/apiutil github.com/fatih/color github.com/ipfs/go-datastore github.com/olekukonko/tablewriter github.com/qri-io/analytics github.com/qri-io/bleve github.com/qri-io/dataset github.com/qri-io/doggos github.com/sirupsen/logrus github.com/spf13/cobra github.com/spf13/viper github.com/qri-io/varName github.com/datatogether/cdxj github.com/spf13/cobra/doc github.com/qri-io/dsdiff github.com/ugorji/go/codec + go get -v -u $(GOPACKAGES) install-gx: go get -v -u github.com/whyrusleeping/gx github.com/whyrusleeping/gx-go From 2d4d9ee23cd8e638efc68c8ece75264cea2ab09f Mon Sep 17 00:00:00 2001 From: b5 Date: Tue, 17 Apr 2018 09:59:46 -0400 Subject: [PATCH 4/5] fix(fsrepo.ProfileStore): add mutex lock removing a race condition uncovers ANOTHER RACE CONDITION. On the upside this might be the cause of corrupt profile store file writes, here's hoping. --- p2p/node.go | 27 +++++++++++++++++++++++---- p2p/profile.go | 7 ------- repo/fs/fs.go | 2 +- repo/fs/profile_store.go | 30 ++++++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 12 deletions(-) diff --git a/p2p/node.go b/p2p/node.go index e4307c56d..8f2bdea88 100644 --- a/p2p/node.go +++ b/p2p/node.go @@ -128,6 +128,24 @@ func NewQriNode(r repo.Repo, options ...func(o *config.P2P)) (node *QriNode, err // add multistream handler for qri protocol to the host // for more info on multistreams check github.com/multformats/go-multistream node.Host.SetStreamHandler(QriProtocolID, node.QriStreamHandler) + + p, err := node.Repo.Profile() + if err != nil { + log.Errorf("error getting repo profile: %s\n", err.Error()) + return node, err + } + + // add listen addresses to profile store + if addrs, err := node.ListenAddresses(); err == nil { + if p.Addresses == nil { + p.Addresses = map[string][]string{} + } + p.Addresses[node.ID.Pretty()] = addrs + } + + if err := node.Repo.SetProfile(p); err != nil { + return node, err + } } go node.echoMessages() @@ -143,15 +161,16 @@ func (n *QriNode) StartOnlineServices(bootstrapped func(string)) error { } bsPeers := make(chan pstore.PeerInfo, len(n.BootstrapAddrs)) + // need a call here to ensure boostrapped is called at least once + // TODO - this is an "original node" problem probably solved by being able + // to start a node with *no* qri peers specified. + defer bootstrapped("") + go func() { pInfo := <-bsPeers bootstrapped(pInfo.ID.Pretty()) }() - // need a call here to ensure boostrapped is called at least once - // TODO - this is an "original node" problem probably solved by being able - // to start a node with *no* qri peers specified. - defer bootstrapped("") return n.StartDiscovery(bsPeers) } diff --git a/p2p/profile.go b/p2p/profile.go index 8058f87da..11af366b3 100644 --- a/p2p/profile.go +++ b/p2p/profile.go @@ -97,12 +97,5 @@ func (n *QriNode) profileBytes() ([]byte, error) { return nil, err } - if addrs, err := n.ListenAddresses(); err == nil { - if p.Addresses == nil { - p.Addresses = map[string][]string{} - } - p.Addresses[n.ID.Pretty()] = addrs - } - return json.Marshal(p) } diff --git a/repo/fs/fs.go b/repo/fs/fs.go index 265638bb8..7f340e37b 100644 --- a/repo/fs/fs.go +++ b/repo/fs/fs.go @@ -64,7 +64,7 @@ func NewRepo(store cafs.Filestore, cfg *config.Profile, base string) (repo.Repo, Refstore: Refstore{basepath: bp, store: store, file: FileRefstore}, EventLog: NewEventLog(base, FileEventLogs, store), - profiles: ProfileStore{bp}, + profiles: NewProfileStore(bp), } if index, err := search.LoadIndex(bp.filepath(FileSearchIndex)); err == nil { diff --git a/repo/fs/profile_store.go b/repo/fs/profile_store.go index 07ee79152..cda5c57f3 100644 --- a/repo/fs/profile_store.go +++ b/repo/fs/profile_store.go @@ -5,6 +5,7 @@ import ( "fmt" "io/ioutil" "os" + "sync" "github.com/ipfs/go-datastore" "github.com/qri-io/doggos" @@ -19,15 +20,26 @@ var ErrNotFound = fmt.Errorf("Not Found") // ProfileStore is an on-disk json file implementation of the // repo.Peers interface type ProfileStore struct { + sync.RWMutex basepath } +// NewProfileStore allocates a ProfileStore +func NewProfileStore(bp basepath) ProfileStore { + return ProfileStore{ + basepath: bp, + } +} + // PutProfile adds a peer to the store func (r ProfileStore) PutProfile(p *profile.Profile) error { if p.ID.String() == "" { return fmt.Errorf("profile ID is required") } + r.Lock() + defer r.Unlock() + ps, err := r.profiles() if err != nil { return err @@ -41,6 +53,9 @@ func (r ProfileStore) PutProfile(p *profile.Profile) error { // PeerIDs gives the peer.IDs list for a given peername func (r ProfileStore) PeerIDs(id profile.ID) ([]peer.ID, error) { + r.RLock() + defer r.RUnlock() + ps, err := r.profiles() if err != nil { return nil, err @@ -57,6 +72,9 @@ func (r ProfileStore) PeerIDs(id profile.ID) ([]peer.ID, error) { // List hands back the list of peers func (r ProfileStore) List() (map[profile.ID]*profile.Profile, error) { + r.RLock() + defer r.RUnlock() + ps, err := r.profiles() if err != nil && err.Error() == "EOF" { return map[profile.ID]*profile.Profile{}, nil @@ -66,6 +84,9 @@ func (r ProfileStore) List() (map[profile.ID]*profile.Profile, error) { // PeernameID gives the profile.ID for a given peername func (r ProfileStore) PeernameID(peername string) (profile.ID, error) { + r.RLock() + defer r.RUnlock() + ps, err := r.profiles() if err != nil { return "", err @@ -81,6 +102,9 @@ func (r ProfileStore) PeernameID(peername string) (profile.ID, error) { // GetProfile fetches a profile from the store func (r ProfileStore) GetProfile(id profile.ID) (*profile.Profile, error) { + r.RLock() + defer r.RUnlock() + ps, err := r.profiles() if err != nil { return nil, err @@ -97,6 +121,9 @@ func (r ProfileStore) GetProfile(id profile.ID) (*profile.Profile, error) { // PeerProfile gives the profile that corresponds with a given peer.ID func (r ProfileStore) PeerProfile(id peer.ID) (*profile.Profile, error) { + r.RLock() + defer r.RUnlock() + ps, err := r.profiles() if err != nil { return nil, err @@ -113,6 +140,9 @@ func (r ProfileStore) PeerProfile(id peer.ID) (*profile.Profile, error) { // DeleteProfile removes a profile from the store func (r ProfileStore) DeleteProfile(id profile.ID) error { + r.Lock() + defer r.Unlock() + ps, err := r.profiles() if err != nil { return err From b0e249584921f54d0c38c2f84e6ca701f7153ae6 Mon Sep 17 00:00:00 2001 From: b5 Date: Tue, 17 Apr 2018 11:09:22 -0400 Subject: [PATCH 5/5] fix(api): add image/jpeg content type to PosterHandler --- api/profile.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/profile.go b/api/profile.go index ca78bfa3e..f10ef2ca7 100644 --- a/api/profile.go +++ b/api/profile.go @@ -141,13 +141,14 @@ func (h *ProfileHandlers) getPosterHandler(w http.ResponseWriter, r *http.Reques data := []byte{} req := &core.Profile{ Peername: r.FormValue("peername"), - ID: r.FormValue("ID"), + ID: r.FormValue("id"), } if err := h.PosterPhoto(req, &data); err != nil { util.WriteErrResponse(w, http.StatusInternalServerError, err) return } + w.Header().Set("Content-Type", "image/jpeg") w.Write(data) }