Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate rel IDs using rIdN pattern #15

Merged
merged 2 commits into from
Oct 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 19 additions & 13 deletions relationship.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import (
"encoding/xml"
"fmt"
"io"
"math/rand"
"net/url"
"sort"
"strings"
)

Expand All @@ -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.
Expand All @@ -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 {
Expand Down
12 changes: 7 additions & 5 deletions writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"compress/flate"
"fmt"
"io"
"math/rand"
"path/filepath"
"strings"
"time"
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
35 changes: 15 additions & 20 deletions writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package opc
import (
"archive/zip"
"bytes"
"math/rand"
"testing"
)

Expand All @@ -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")
Expand All @@ -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) {
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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 {
Expand Down