Skip to content

Commit dddd6f1

Browse files
committed
Add support for comment and label manifests.
Introduces `.MANIFEST` files to comment and label directories to keep track of sub-resources when the PullRequest resource is initialized. This allows us to keep track of what resources the user explicitly deleted, and not accidentally overwrite new comments/lables that were introduced during execution. Minor changes included: * Changed fake GitHub server to make copies of PullRequests to avoid pointer collision across tests. * Changed uploadLabels to Add/Delete labels explicitly instead of replacing all. Change manifest path to .MANIFEST to avoid collisions. Fixes #1286
1 parent 5824fd5 commit dddd6f1

File tree

7 files changed

+416
-58
lines changed

7 files changed

+416
-58
lines changed

cmd/pullrequest-init/disk.go

Lines changed: 97 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ limitations under the License.
1717
package main
1818

1919
import (
20+
"bufio"
2021
"encoding/json"
22+
"fmt"
2123
"io/ioutil"
2224
"net/url"
2325
"os"
@@ -40,6 +42,10 @@ import (
4042

4143
// Filenames for labels and statuses are URL encoded for safety.
4244

45+
const (
46+
manifestPath = ".MANIFEST"
47+
)
48+
4349
// ToDisk converts a PullRequest object to an on-disk representation at the specified path.
4450
func ToDisk(pr *PullRequest, path string) error {
4551
labelsPath := filepath.Join(path, "labels")
@@ -80,28 +86,54 @@ func ToDisk(pr *PullRequest, path string) error {
8086
}
8187

8288
func commentsToDisk(path string, comments []*Comment) error {
89+
// Create a manifest to keep track of the comments that existed when the
90+
// resource was initialized. This is used to verify that a comment that
91+
// doesn't exist on disk was actually deleted by the user and not newly
92+
// created during upload.
93+
f, err := os.Create(filepath.Join(path, manifestPath))
94+
if err != nil {
95+
return err
96+
}
97+
defer f.Close()
98+
manifest := bufio.NewWriter(f)
99+
83100
for _, c := range comments {
84-
commentPath := filepath.Join(path, strconv.FormatInt(c.ID, 10)+".json")
101+
id := strconv.FormatInt(c.ID, 10)
102+
commentPath := filepath.Join(path, id+".json")
85103
b, err := json.Marshal(c)
86104
if err != nil {
87105
return err
88106
}
89-
if err := ioutil.WriteFile(commentPath, b, 0700); err != nil {
107+
if err := ioutil.WriteFile(commentPath, b, 0600); err != nil {
108+
return err
109+
}
110+
if _, err := manifest.WriteString(fmt.Sprintf("%s\n", id)); err != nil {
90111
return err
91112
}
92113
}
93-
return nil
114+
return manifest.Flush()
94115
}
95116

96117
func labelsToDisk(path string, labels []*Label) error {
118+
f, err := os.Create(filepath.Join(path, manifestPath))
119+
if err != nil {
120+
return err
121+
}
122+
defer f.Close()
123+
manifest := bufio.NewWriter(f)
124+
97125
for _, l := range labels {
98126
name := url.QueryEscape(l.Text)
99127
labelPath := filepath.Join(path, name)
100-
if err := ioutil.WriteFile(labelPath, []byte{}, 0700); err != nil {
128+
if err := ioutil.WriteFile(labelPath, []byte{}, 0600); err != nil {
129+
return err
130+
}
131+
if _, err := manifest.WriteString(fmt.Sprintf("%s\n", l.Text)); err != nil {
101132
return err
102133
}
134+
103135
}
104-
return nil
136+
return manifest.Flush()
105137
}
106138

107139
func statusToDisk(path string, statuses []*Status) error {
@@ -127,57 +159,81 @@ func refToDisk(name, path string, r *GitReference) error {
127159
return ioutil.WriteFile(filepath.Join(path, name+".json"), b, 0700)
128160
}
129161

162+
// Manifest is a list of sub-resources that exist within the PR resource to
163+
// determine whether an item existed when the resource was initialized.
164+
type Manifest map[string]bool
165+
130166
// FromDisk outputs a PullRequest object from an on-disk representation at the specified path.
131-
func FromDisk(path string) (*PullRequest, error) {
167+
func FromDisk(path string) (*PullRequest, map[string]Manifest, error) {
132168
labelsPath := filepath.Join(path, "labels")
133169
commentsPath := filepath.Join(path, "comments")
134170
statusesPath := filepath.Join(path, "status")
135171

136172
pr := PullRequest{}
173+
manifests := make(map[string]Manifest)
137174

138175
var err error
176+
var manifest Manifest
139177

140-
// Start with comments
141-
pr.Comments, err = commentsFromDisk(commentsPath)
178+
pr.Comments, manifest, err = commentsFromDisk(commentsPath)
142179
if err != nil {
143-
return nil, err
180+
return nil, nil, err
144181
}
182+
manifests["comments"] = manifest
145183

146-
// Now Labels
147-
pr.Labels, err = labelsFromDisk(labelsPath)
184+
pr.Labels, manifest, err = labelsFromDisk(labelsPath)
148185
if err != nil {
149-
return nil, err
186+
return nil, nil, err
150187
}
188+
manifests["labels"] = manifest
151189

152-
// Now statuses
153190
pr.Statuses, err = statusesFromDisk(statusesPath)
154191
if err != nil {
155-
return nil, err
192+
return nil, nil, err
156193
}
157194

158-
// Finally refs
159-
160195
pr.Base, err = refFromDisk(path, "base.json")
161196
if err != nil {
162-
return nil, err
197+
return nil, nil, err
163198
}
164199
pr.Head, err = refFromDisk(path, "head.json")
165200
if err != nil {
201+
return nil, nil, err
202+
}
203+
return &pr, manifests, nil
204+
}
205+
206+
func manifestFromDisk(path string) (map[string]bool, error) {
207+
f, err := os.Open(path)
208+
if err != nil {
209+
return nil, err
210+
}
211+
defer f.Close()
212+
213+
out := make(map[string]bool)
214+
s := bufio.NewScanner(f)
215+
for s.Scan() {
216+
out[s.Text()] = true
217+
}
218+
if s.Err() != nil {
166219
return nil, err
167220
}
168-
return &pr, nil
221+
return out, nil
169222
}
170223

171-
func commentsFromDisk(path string) ([]*Comment, error) {
224+
func commentsFromDisk(path string) ([]*Comment, Manifest, error) {
172225
fis, err := ioutil.ReadDir(path)
173226
if err != nil {
174-
return nil, err
227+
return nil, nil, err
175228
}
176229
comments := []*Comment{}
177230
for _, fi := range fis {
231+
if fi.Name() == manifestPath {
232+
continue
233+
}
178234
b, err := ioutil.ReadFile(filepath.Join(path, fi.Name()))
179235
if err != nil {
180-
return nil, err
236+
return nil, nil, err
181237
}
182238
comment := Comment{}
183239
if err := json.Unmarshal(b, &comment); err != nil {
@@ -186,23 +242,38 @@ func commentsFromDisk(path string) ([]*Comment, error) {
186242
}
187243
comments = append(comments, &comment)
188244
}
189-
return comments, nil
245+
246+
manifest, err := manifestFromDisk(filepath.Join(path, manifestPath))
247+
if err != nil {
248+
return nil, nil, err
249+
}
250+
251+
return comments, manifest, nil
190252
}
191253

192-
func labelsFromDisk(path string) ([]*Label, error) {
254+
func labelsFromDisk(path string) ([]*Label, Manifest, error) {
193255
fis, err := ioutil.ReadDir(path)
194256
if err != nil {
195-
return nil, err
257+
return nil, nil, err
196258
}
197259
labels := []*Label{}
198260
for _, fi := range fis {
261+
if fi.Name() == manifestPath {
262+
continue
263+
}
199264
text, err := url.QueryUnescape(fi.Name())
200265
if err != nil {
201-
return nil, err
266+
return nil, nil, err
202267
}
203268
labels = append(labels, &Label{Text: text})
204269
}
205-
return labels, nil
270+
271+
manifest, err := manifestFromDisk(filepath.Join(path, manifestPath))
272+
if err != nil {
273+
return nil, nil, err
274+
}
275+
276+
return labels, manifest, nil
206277
}
207278

208279
func statusesFromDisk(path string) ([]*Status, error) {

0 commit comments

Comments
 (0)