Skip to content

Commit

Permalink
Remove pointers to maps.
Browse files Browse the repository at this point in the history
  • Loading branch information
amwat committed Oct 22, 2019
1 parent 38afae0 commit 5c5464f
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 33 deletions.
16 changes: 8 additions & 8 deletions prow/cmd/gerrit/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func gatherOptions(fs *flag.FlagSet, args ...string) options {
}

type syncTime struct {
val *client.LastSyncState
val client.LastSyncState
lock sync.RWMutex
path string
opener io.Opener
Expand Down Expand Up @@ -126,12 +126,12 @@ func (st *syncTime) init(hostProjects client.ProjectsFlag) error {
targetState[host][project] = currentTime
}
}
st.val = &targetState
st.val = targetState
}
return nil
}

func (st *syncTime) currentState() (*client.LastSyncState, error) {
func (st *syncTime) currentState() (client.LastSyncState, error) {
r, err := st.opener.Reader(st.ctx, st.path)
if io.IsNotExist(err) {
logrus.Warnf("lastSyncFallback not found at %q", st.path)
Expand All @@ -148,23 +148,23 @@ func (st *syncTime) currentState() (*client.LastSyncState, error) {
if err := json.Unmarshal(buf, &state); err != nil {
return nil, fmt.Errorf("unmarshall state: %v", err)
}
return &state, nil
return state, nil
}

func (st *syncTime) Current() *client.LastSyncState {
func (st *syncTime) Current() client.LastSyncState {
st.lock.RLock()
defer st.lock.RUnlock()
return st.val
}

func (st *syncTime) Update(newState *client.LastSyncState) error {
func (st *syncTime) Update(newState client.LastSyncState) error {
st.lock.Lock()
defer st.lock.Unlock()

targetState := st.val.DeepCopy()

var changed bool
for host, newLastSyncs := range *newState {
for host, newLastSyncs := range newState {
if _, ok := targetState[host]; !ok {
targetState[host] = map[string]time.Time{}
}
Expand Down Expand Up @@ -196,7 +196,7 @@ func (st *syncTime) Update(newState *client.LastSyncState) error {
if err := w.Close(); err != nil {
return fmt.Errorf("close %q: %v", st.path, err)
}
st.val = &targetState
st.val = targetState
return nil
}

Expand Down
14 changes: 7 additions & 7 deletions prow/cmd/gerrit/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,25 +149,25 @@ func TestSyncTime(t *testing.T) {
if err := st.init(testProjectsFlag); err != nil {
t.Fatalf("Failed init: %v", err)
}
cur := (*st.Current())["foo"]["bar"]
cur := st.Current()["foo"]["bar"]
if now.After(cur) {
t.Fatalf("%v should be >= time before init was called: %v", cur, now)
}

earlier := now.Add(-time.Hour)
later := now.Add(time.Hour)

if err := st.Update(&client.LastSyncState{"foo": {"bar": earlier}}); err != nil {
if err := st.Update(client.LastSyncState{"foo": {"bar": earlier}}); err != nil {
t.Fatalf("Failed update: %v", err)
}
if actual := (*st.Current())["foo"]["bar"]; !actual.Equal(cur) {
if actual := st.Current()["foo"]["bar"]; !actual.Equal(cur) {
t.Errorf("Update(%v) should not have reduced value from %v, got %v", earlier, cur, actual)
}

if err := st.Update(&client.LastSyncState{"foo": {"bar": later}}); err != nil {
if err := st.Update(client.LastSyncState{"foo": {"bar": later}}); err != nil {
t.Fatalf("Failed update: %v", err)
}
if actual := (*st.Current())["foo"]["bar"]; !actual.After(cur) {
if actual := st.Current()["foo"]["bar"]; !actual.After(cur) {
t.Errorf("Update(%v) did not move current value to after %v, got %v", later, cur, actual)
}

Expand All @@ -180,7 +180,7 @@ func TestSyncTime(t *testing.T) {
if err := st.init(testProjectsFlag); err != nil {
t.Fatalf("Failed init: %v", err)
}
if actual := (*st.Current())["foo"]["bar"]; !actual.Equal(expected) {
if actual := st.Current()["foo"]["bar"]; !actual.Equal(expected) {
t.Errorf("init() failed to reload %v, got %v", expected, actual)
}

Expand All @@ -192,7 +192,7 @@ func TestSyncTime(t *testing.T) {
if err := st.init(testProjectsFlag); err != nil {
t.Fatalf("Failed init: %v", err)
}
if actual := (*st.Current())["foo"]["bar"]; now.After(actual) || actual.After(later) {
if actual := st.Current()["foo"]["bar"]; now.After(actual) || actual.After(later) {
t.Fatalf("should initialize to start %v <= actual <= later %v, but got %v", now, later, actual)
}
}
10 changes: 5 additions & 5 deletions prow/gerrit/adapter/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type prowJobClient interface {
}

type gerritClient interface {
QueryChanges(lastState *client.LastSyncState, rateLimit int) map[string][]client.ChangeInfo
QueryChanges(lastState client.LastSyncState, rateLimit int) map[string][]client.ChangeInfo
GetBranchRevision(instance, project, branch string) (string, error)
SetReview(instance, id, revision, message string, labels map[string]string) error
Account(instance string) *gerrit.AccountInfo
Expand All @@ -59,8 +59,8 @@ type Controller struct {
}

type LastSyncTracker interface {
Current() *client.LastSyncState
Update(*client.LastSyncState) error
Current() client.LastSyncState
Update(client.LastSyncState) error
}

// NewController returns a new gerrit controller client
Expand Down Expand Up @@ -104,7 +104,7 @@ func (c *Controller) Sync() error {
logrus.Infof("Processed %d changes for instance %s", len(changes), instance)
}

return c.tracker.Update(&latest)
return c.tracker.Update(latest)
}

func makeCloneURI(instance, project string) (*url.URL, error) {
Expand Down Expand Up @@ -244,7 +244,7 @@ func (c *Controller) ProcessChange(instance string, change client.ChangeInfo) er
logger.Infof("Found latest report: %s", latestReport)
}

lastUpdate, ok := (*c.tracker.Current())[instance][change.Project]
lastUpdate, ok := c.tracker.Current()[instance][change.Project]
if !ok {
logrus.Warnf("could not find lastTime for project %q, probably something went wrong with initTracker?", change.Project)
lastUpdate = time.Now()
Expand Down
10 changes: 5 additions & 5 deletions prow/gerrit/adapter/adapter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func (f *fca) Config() *config.Config {

type fgc struct{}

func (f *fgc) QueryChanges(lastUpdate *client.LastSyncState, rateLimit int) map[string][]client.ChangeInfo {
func (f *fgc) QueryChanges(lastUpdate client.LastSyncState, rateLimit int) map[string][]client.ChangeInfo {
return nil
}

Expand Down Expand Up @@ -123,17 +123,17 @@ func TestMakeCloneURI(t *testing.T) {
}

type fakeSync struct {
val *client.LastSyncState
val client.LastSyncState
lock sync.Mutex
}

func (s *fakeSync) Current() *client.LastSyncState {
func (s *fakeSync) Current() client.LastSyncState {
s.lock.Lock()
defer s.lock.Unlock()
return s.val
}

func (s *fakeSync) Update(t *client.LastSyncState) error {
func (s *fakeSync) Update(t client.LastSyncState) error {
s.lock.Lock()
defer s.lock.Unlock()
s.val = t
Expand Down Expand Up @@ -716,7 +716,7 @@ func TestProcessChange(t *testing.T) {
config: fca.Config,
prowJobClient: fakeProwJobClient.ProwV1().ProwJobs("prowjobs"),
gc: &fgc{},
tracker: &fakeSync{val: &fakeLastSync},
tracker: &fakeSync{val: fakeLastSync},
}

err := c.ProcessChange(testInstance, tc.change)
Expand Down
14 changes: 7 additions & 7 deletions prow/gerrit/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,9 @@ type FileInfo = gerrit.FileInfo
// Map from instance name to repos to lastsync time for that repo
type LastSyncState map[string]map[string]time.Time

func (l *LastSyncState) DeepCopy() LastSyncState {
func (l LastSyncState) DeepCopy() LastSyncState {
result := LastSyncState{}
for host, lastSyncs := range *l {
for host, lastSyncs := range l {
result[host] = map[string]time.Time{}
for projects, lastSync := range lastSyncs {
result[host][projects] = lastSync
Expand Down Expand Up @@ -207,11 +207,11 @@ func (c *Client) Start(cookiefilePath string) {

// QueryChanges queries for all changes from all projects after lastUpdate time
// returns an instance:changes map
func (c *Client) QueryChanges(lastState *LastSyncState, rateLimit int) map[string][]ChangeInfo {
func (c *Client) QueryChanges(lastState LastSyncState, rateLimit int) map[string][]ChangeInfo {
result := map[string][]ChangeInfo{}
for _, h := range c.handlers {
lastStateForInstance := (*lastState)[h.instance]
changes := h.queryAllChanges(&lastStateForInstance, rateLimit)
lastStateForInstance := lastState[h.instance]
changes := h.queryAllChanges(lastStateForInstance, rateLimit)
if len(changes) > 0 {
result[h.instance] = []ChangeInfo{}
for _, change := range changes {
Expand Down Expand Up @@ -261,11 +261,11 @@ func (c *Client) Account(instance string) *gerrit.AccountInfo {

// private handler implementation details

func (h *gerritInstanceHandler) queryAllChanges(lastState *map[string]time.Time, rateLimit int) []gerrit.ChangeInfo {
func (h *gerritInstanceHandler) queryAllChanges(lastState map[string]time.Time, rateLimit int) []gerrit.ChangeInfo {
result := []gerrit.ChangeInfo{}
timeNow := time.Now()
for _, project := range h.projects {
lastUpdate, ok := (*lastState)[project]
lastUpdate, ok := lastState[project]
if !ok {
logrus.WithField("project", project).Warnf("could not find lastTime for project %q, probably something went wrong with initTracker?", project)
lastUpdate = timeNow
Expand Down
2 changes: 1 addition & 1 deletion prow/gerrit/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ func TestQueryChange(t *testing.T) {
},
}

testLastSync := &LastSyncState{"foo": tc.lastUpdate, "baz": tc.lastUpdate}
testLastSync := LastSyncState{"foo": tc.lastUpdate, "baz": tc.lastUpdate}
changes := client.QueryChanges(testLastSync, 5)

revisions := map[string][]string{}
Expand Down

0 comments on commit 5c5464f

Please sign in to comment.