Skip to content

Commit

Permalink
improve random number generator for writer
Browse files Browse the repository at this point in the history
  • Loading branch information
@qmuntal
qmuntal committed Feb 7, 2019
1 parent 1eb3224 commit cf39743
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 44 deletions.
6 changes: 2 additions & 4 deletions relationship.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"math/rand"
"net/url"
"strings"
"time"
)

// TargetMode is an enumerable for the different target modes.
Expand Down Expand Up @@ -48,15 +47,14 @@ type relationshipXML struct {
Mode string `xml:"TargetMode,attr,omitempty"`
}

func (r *Relationship) ensureID() {
func (r *Relationship) ensureID(rnd *rand.Rand) {
if r.ID != "" {
return
}

b := make([]byte, 8)
rd := rand.New(rand.NewSource(time.Now().UnixNano()))
for i := range b {
b[i] = charBytes[rd.Intn(len(charBytes))]
b[i] = charBytes[rnd.Intn(len(charBytes))]
}
r.ID = string(b)
}
Expand Down
8 changes: 5 additions & 3 deletions writer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"compress/flate"
"fmt"
"io"
"math/rand"
"path/filepath"
"time"
)
Expand Down Expand Up @@ -32,11 +33,12 @@ type Writer struct {
p *pkg
w *zip.Writer
last *Part
rnd *rand.Rand
}

// NewWriter returns a new Writer writing an OPC file to w.
func NewWriter(w io.Writer) *Writer {
return &Writer{p: newPackage(), w: zip.NewWriter(w)}
return &Writer{p: newPackage(), w: zip.NewWriter(w), rnd: rand.New(rand.NewSource(time.Now().UnixNano()))}
}

// Flush flushes any buffered data to the underlying writer.
Expand Down Expand Up @@ -123,7 +125,7 @@ func (w *Writer) createOwnRelationships() error {
return nil
}
for _, r := range w.Relationships {
r.ensureID()
r.ensureID(w.rnd)
}
if err := validateRelationships("/", w.Relationships); err != nil {
return err
Expand All @@ -140,7 +142,7 @@ func (w *Writer) createLastPartRelationships() error {
return nil
}
for _, r := range w.last.Relationships {
r.ensureID()
r.ensureID(w.rnd)
}
if err := validateRelationships(w.last.Name, w.last.Relationships); err != nil {
return err
Expand Down
57 changes: 20 additions & 37 deletions writer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,10 @@ package opc
import (
"archive/zip"
"bytes"
"reflect"
"math/rand"
"testing"
)

func TestNewWriter(t *testing.T) {
tests := []struct {
name string
want *Writer
wantW string
}{
{"base", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{})}, ""},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
w := &bytes.Buffer{}
if got := NewWriter(w); !reflect.DeepEqual(got, tt.want) {
t.Errorf("NewWriter() = %v, want %v", got, tt.want)
}
if gotW := w.String(); gotW != tt.wantW {
t.Errorf("NewWriter() = %v, want %v", gotW, tt.wantW)
}
})
}
}

func TestWriter_Flush(t *testing.T) {
tests := []struct {
name string
Expand All @@ -45,6 +24,10 @@ func TestWriter_Flush(t *testing.T) {
}
}

func fakeRand() *rand.Rand {
return rand.New(rand.NewSource(42))
}

func TestWriter_Close(t *testing.T) {
p := newPackage()
p.contentTypes.add("/a.xml", "a/b")
Expand All @@ -61,15 +44,15 @@ func TestWriter_Close(t *testing.T) {
wantErr bool
}{
{"base", NewWriter(&bytes.Buffer{}), false},
{"invalidContentType", &Writer{p: pC, w: zip.NewWriter(&bytes.Buffer{})}, true},
{"withCt", &Writer{p: p, w: zip.NewWriter(&bytes.Buffer{})}, false},
{"invalidPartRel", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{}), last: &Part{Name: "/b.xml", Relationships: []*Relationship{{}}}}, true},
{"invalidOwnRel", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{}), Relationships: []*Relationship{{}}}, true},
{"withDuplicatedCoreProps", &Writer{p: pCore, w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song"}}, true},
{"withDuplicatedRels", &Writer{p: pRel, w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song"}}, true},
{"withDuplicatedRels", &Writer{p: pRel, w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song"}}, true},
{"withCoreProps", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song"}}, false},
{"withCorePropsWithName", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song", PartName: "/props.xml"}}, false},
{"invalidContentType", &Writer{p: pC, w: zip.NewWriter(&bytes.Buffer{}), rnd: fakeRand()}, true},
{"withCt", &Writer{p: p, w: zip.NewWriter(&bytes.Buffer{}), rnd: fakeRand()}, false},
{"invalidPartRel", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{}), last: &Part{Name: "/b.xml", Relationships: []*Relationship{{}}}, rnd: fakeRand()}, true},
{"invalidOwnRel", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{}), Relationships: []*Relationship{{}}, rnd: fakeRand()}, true},
{"withDuplicatedCoreProps", &Writer{p: pCore, w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song"}, rnd: fakeRand()}, true},
{"withDuplicatedRels", &Writer{p: pRel, w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song"}, rnd: fakeRand()}, true},
{"withDuplicatedRels", &Writer{p: pRel, w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song"}, rnd: fakeRand()}, true},
{"withCoreProps", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song"}, rnd: fakeRand()}, false},
{"withCorePropsWithName", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song", PartName: "/props.xml"}, rnd: fakeRand()}, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -169,8 +152,8 @@ func TestWriter_CreatePart(t *testing.T) {
}{
{"fhErr", NewWriter(&bytes.Buffer{}), args{&Part{"/a.xml", "a/b", nil}, -3}, true},
{"nameErr", NewWriter(&bytes.Buffer{}), args{&Part{"a.xml", "a/b", nil}, CompressionNone}, true},
{"failRel", &Writer{w: zip.NewWriter(nil), last: &Part{Name: "/b.xml", Relationships: []*Relationship{{}}}}, args{&Part{"/a.xml", "a/b", nil}, CompressionNone}, true},
{"failRel2", &Writer{p: pRel, w: zip.NewWriter(nil), last: &Part{Name: "/a.xml", Relationships: []*Relationship{rel}}}, args{&Part{"/b.xml", "a/b", nil}, CompressionNone}, true},
{"failRel", &Writer{w: zip.NewWriter(nil), last: &Part{Name: "/b.xml", Relationships: []*Relationship{{}}}, rnd: fakeRand()}, args{&Part{"/a.xml", "a/b", nil}, CompressionNone}, true},
{"failRel2", &Writer{p: pRel, w: zip.NewWriter(nil), last: &Part{Name: "/a.xml", Relationships: []*Relationship{rel}}, rnd: fakeRand()}, args{&Part{"/b.xml", "a/b", nil}, CompressionNone}, true},
{"base", w, args{&Part{"/a.xml", "a/b", nil}, CompressionNone}, false},
{"multipleDiffName", w, args{&Part{"/b.xml", "a/b", nil}, CompressionNone}, false},
{"multipleDiffContentType", w, args{&Part{"/c.xml", "c/d", nil}, CompressionNone}, false},
Expand Down Expand Up @@ -199,11 +182,11 @@ func TestWriter_createLastPartRelationships(t *testing.T) {
w *Writer
wantErr bool
}{
{"base", &Writer{p: newPackage(), w: zip.NewWriter(nil), last: &Part{Name: "/a.xml", Relationships: []*Relationship{rel}}}, false},
{"base2", &Writer{p: newPackage(), w: zip.NewWriter(nil), last: &Part{Name: "/b/a.xml", Relationships: []*Relationship{rel}}}, false},
{"base", &Writer{p: newPackage(), w: zip.NewWriter(nil), last: &Part{Name: "/a.xml", Relationships: []*Relationship{rel}}, rnd: fakeRand()}, false},
{"base2", &Writer{p: newPackage(), w: zip.NewWriter(nil), last: &Part{Name: "/b/a.xml", Relationships: []*Relationship{rel}}, rnd: fakeRand()}, false},
{"hasSome", w, false},
{"duplicated", &Writer{w: zip.NewWriter(nil), last: &Part{Name: "/a.xml", Relationships: []*Relationship{rel, rel}}}, true},
{"invalidRelation", &Writer{w: zip.NewWriter(nil), last: &Part{Name: "/a.xml", Relationships: []*Relationship{{}}}}, true},
{"duplicated", &Writer{w: zip.NewWriter(nil), last: &Part{Name: "/a.xml", Relationships: []*Relationship{rel, rel}}, rnd: fakeRand()}, true},
{"invalidRelation", &Writer{w: zip.NewWriter(nil), last: &Part{Name: "/a.xml", Relationships: []*Relationship{{}}}, rnd: fakeRand()}, true},
{"empty", NewWriter(&bytes.Buffer{}), false},
}
for _, tt := range tests {
Expand Down

0 comments on commit cf39743

Please sign in to comment.