diff --git a/relationship.go b/relationship.go index a541858..6c8aaf3 100644 --- a/relationship.go +++ b/relationship.go @@ -4,8 +4,8 @@ import ( "encoding/xml" "fmt" "io" - "math/rand" "net/url" + "sort" "strings" ) @@ -25,8 +25,8 @@ const externalMode = "External" const charBytes = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ123456789" // Relationship is used to express a relationship between a source and a target part. -// If the ID is not specified a random string with 8 characters will be generated. -// If The TargetMode is not specified the default value is Internal. +// If the ID is not specified a unique ID will be generated following the pattern rIdN. +// If the TargetMode is not specified the default value is Internal. // Defined in ISO/IEC 29500-2 ยง9.3. type Relationship struct { ID string // The relationship identifier which shall conform the xsd:ID naming restrictions and unique within the part. @@ -48,16 +48,22 @@ type relationshipXML struct { Mode string `xml:"TargetMode,attr,omitempty"` } -func (r *Relationship) ensureID(rnd *rand.Rand) { - if r.ID != "" { - return - } - - b := make([]byte, 8) - for i := range b { - b[i] = charBytes[rnd.Intn(len(charBytes))] - } - r.ID = string(b) +func newRelationshipID(rels []*Relationship) string { + ids := make([]string, len(rels)) + for i, rel := range rels { + ids[i] = rel.ID + } + sort.Strings(ids) + idFunc := func(i int) string { return fmt.Sprintf("rId%d", i) } + var ( + i int + id = idFunc(0) + ) + for sort.SearchStrings(ids, id) < 0 { + i++ + id = idFunc(i) + } + return id } func (r *Relationship) validate(sourceURI string) error { diff --git a/writer.go b/writer.go index 74f9bd9..3bb51cd 100644 --- a/writer.go +++ b/writer.go @@ -5,7 +5,6 @@ import ( "compress/flate" "fmt" "io" - "math/rand" "path/filepath" "strings" "time" @@ -34,7 +33,6 @@ type Writer struct { p *pkg w *zip.Writer last *Part - rnd *rand.Rand } // NewWriter returns a new Writer writing an OPC file to w. @@ -48,7 +46,7 @@ func NewWriter(w io.Writer) *Writer { }, overrides: map[string]string{}, }, - }, w: zip.NewWriter(w), rnd: rand.New(rand.NewSource(42))} + }, w: zip.NewWriter(w)} } // Flush flushes any buffered data to the underlying writer. @@ -148,7 +146,9 @@ func (w *Writer) createOwnRelationships() error { return nil } for _, r := range w.Relationships { - r.ensureID(w.rnd) + if r.ID == "" { + r.ID = newRelationshipID(w.Relationships) + } } if err := validateRelationships("/", w.Relationships); err != nil { return err @@ -165,7 +165,9 @@ func (w *Writer) createLastPartRelationships() error { return nil } for _, r := range w.last.Relationships { - r.ensureID(w.rnd) + if r.ID == "" { + r.ID = newRelationshipID(w.Relationships) + } } if err := validateRelationships(w.last.Name, w.last.Relationships); err != nil { return err diff --git a/writer_test.go b/writer_test.go index 027da54..b9f3261 100644 --- a/writer_test.go +++ b/writer_test.go @@ -3,7 +3,6 @@ package opc import ( "archive/zip" "bytes" - "math/rand" "testing" ) @@ -24,10 +23,6 @@ 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") @@ -44,17 +39,17 @@ func TestWriter_Close(t *testing.T) { wantErr bool }{ {"base", NewWriter(&bytes.Buffer{}), 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}, - {"withCoreProps", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song"}, rnd: fakeRand()}, 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}, + {"withCoreProps", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song"}}, false}, {"withCorePropsWithName", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{}), Relationships: []*Relationship{ {TargetURI: "props.xml", Type: corePropsRel}, - }, Properties: CoreProperties{Title: "Song", PartName: "props.xml"}, rnd: fakeRand()}, false}, - {"withCorePropsWithNameAndId", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song", PartName: "/docProps/props.xml", RelationshipID: "rId1"}, rnd: fakeRand()}, false}, + }, Properties: CoreProperties{Title: "Song", PartName: "props.xml"}}, false}, + {"withCorePropsWithNameAndId", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song", PartName: "/docProps/props.xml", RelationshipID: "rId1"}}, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -154,8 +149,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{{}}}, 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}, + {"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}, {"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}, @@ -184,11 +179,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}}, rnd: fakeRand()}, false}, - {"base2", &Writer{p: newPackage(), w: zip.NewWriter(nil), last: &Part{Name: "/b/a.xml", Relationships: []*Relationship{rel}}, rnd: fakeRand()}, false}, + {"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}, {"hasSome", w, false}, - {"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}, + {"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}, {"empty", NewWriter(&bytes.Buffer{}), false}, } for _, tt := range tests {