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

rename Table to SummaryTable #38

Merged
merged 24 commits into from
Aug 29, 2016
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
e5a5c46
rename control.Table to .SummaryTable
afeld Jul 26, 2016
af067bd
refactor SSP loading for document_test
afeld Jul 26, 2016
a87ea77
add a test for SumaryTables()
afeld Jul 28, 2016
8ccbcee
refactor SummaryTable tests
afeld Jul 29, 2016
cc537b7
change `ct` to `st` for referencing SummaryTables
afeld Aug 28, 2016
a92ad09
share some logic for finding summary tables with tests
afeld Aug 28, 2016
353014d
refactor helper methods out of templater test
afeld Jul 23, 2016
17ce563
remove redundant disclaimer about gokogiri version
afeld Jul 29, 2016
a75cd3b
share fixture helper functions between tests
afeld Aug 27, 2016
7666929
fix component fixture to match actual SSP narrative structure
afeld Jul 28, 2016
c7a2dfa
add a method to retrieve the narrative tables
afeld Jul 29, 2016
3d012ac
add a GetNarrative() method for opencontrols.Data
afeld Jul 29, 2016
fe77edd
fix narrative tests
afeld Aug 28, 2016
e961533
refactor table struct out from SummaryTable
afeld Aug 28, 2016
56c1473
create a NarrativeTable struct (broken)
afeld Aug 28, 2016
397aa02
fill narrative tables
afeld Aug 28, 2016
dcca032
refactor narrative row filling
afeld Aug 28, 2016
21a79d2
fix narrative row filling
afeld Aug 28, 2016
72139b5
refactor NarrativeTable
afeld Aug 28, 2016
55db87a
fix Code Climate errors
afeld Aug 28, 2016
3234e7a
respect newlines in narrative content
afeld Aug 28, 2016
06655b7
embed tables directly in the Narrative/SummaryTable structs
afeld Aug 28, 2016
ccae626
create a narrativeSection struct
afeld Aug 28, 2016
3965568
create an XML helper package
afeld Aug 28, 2016
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
11 changes: 6 additions & 5 deletions control/responsible_role.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
package control

import (
"github.com/jbowtie/gokogiri/xml"
"errors"
"fmt"
"regexp"
"strings"
"fmt"

"github.com/jbowtie/gokogiri/xml"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI that this is go-plus making these kinds of changes automatically. I'm not quite that OCD 😉

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol!! No worries. go-plus is running go fmt after every save. side note: we probably need to activate this for add this to our codeclimate rules: https://github.com/codeclimate-community/codeclimate-gofmt

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ (not for this PR) #41

)

// findResponsibleRole looks for the Responsible Role cell in the control table.
func findResponsibleRole(ct *Table) (*responsibleRole, error) {
func findResponsibleRole(ct *SummaryTable) (*responsibleRole, error) {
Copy link
Member

@jcscottiii jcscottiii Aug 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we aren't using Table to represent Control Table (because there are multiple tables) anymore, could we rename ct to st (here and all the other places)?

nodes, err := ct.searchSubtree(".//w:tc[starts-with(normalize-space(.), 'Responsible Role')]")
if err != nil {
return nil, err
Expand All @@ -28,7 +29,7 @@ func findResponsibleRole(ct *Table) (*responsibleRole, error) {
// responsibleRole is the container for the responsible role cell.
type responsibleRole struct {
parentNode xml.Node
textNodes *[]xml.Node
textNodes *[]xml.Node
}

// getContent returns the full string representation of the content of the cell itself.
Expand Down Expand Up @@ -73,4 +74,4 @@ func (r *responsibleRole) getValue() string {
}
}
return result
}
}
18 changes: 9 additions & 9 deletions control/table.go → control/summary_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ const (
responsibleRoleField = "Responsible Role"
)

// Table represents the node in the Word docx XML tree that corresponds to a security control.
type Table struct {
// SummaryTable represents the node in the Word docx XML tree that corresponds to a security control.
type SummaryTable struct {
Root xml.Node
}

func (ct *Table) searchSubtree(xpath string) (nodes []xml.Node, err error) {
func (ct *SummaryTable) searchSubtree(xpath string) (nodes []xml.Node, err error) {
// http://stackoverflow.com/a/25387687/358804
if !strings.HasPrefix(xpath, ".") {
err = errors.New("XPath must have leading period (`.`) to only search the subtree")
Expand All @@ -30,7 +30,7 @@ func (ct *Table) searchSubtree(xpath string) (nodes []xml.Node, err error) {
return ct.Root.Search(xpath)
}

func (ct *Table) tableHeader() (content string, err error) {
func (ct *SummaryTable) tableHeader() (content string, err error) {
nodes, err := ct.searchSubtree(".//w:tr")
if err != nil {
return
Expand All @@ -45,7 +45,7 @@ func (ct *Table) tableHeader() (content string, err error) {
return
}

func (ct *Table) controlName() (name string, err error) {
func (ct *SummaryTable) controlName() (name string, err error) {
content, err := ct.tableHeader()
if err != nil {
return
Expand All @@ -61,7 +61,7 @@ func (ct *Table) controlName() (name string, err error) {
}

// Fill inserts the OpenControl justifications into the table. Note this modifies the `table`.
func (ct *Table) Fill(openControlData opencontrols.Data) (err error) {
func (ct *SummaryTable) Fill(openControlData opencontrols.Data) (err error) {
roleCell, err := findResponsibleRole(ct)
if err != nil {
return
Expand All @@ -79,7 +79,7 @@ func (ct *Table) Fill(openControlData opencontrols.Data) (err error) {
}

// diffResponsibleRole computes the diff of the responsible role cell.
func (ct *Table) diffResponsibleRole(control string, openControlData opencontrols.Data) ([]reporter.Reporter, error) {
func (ct *SummaryTable) diffResponsibleRole(control string, openControlData opencontrols.Data) ([]reporter.Reporter, error) {
roleCell, err := findResponsibleRole(ct)
if err != nil {
return []reporter.Reporter{}, err
Expand All @@ -95,10 +95,10 @@ func (ct *Table) diffResponsibleRole(control string, openControlData opencontrol
}

// Diff returns the list of diffs in the control table.
func (ct *Table) Diff(openControlData opencontrols.Data) ([]reporter.Reporter, error) {
func (ct *SummaryTable) Diff(openControlData opencontrols.Data) ([]reporter.Reporter, error) {
control, err := ct.controlName()
if err != nil {
return []reporter.Reporter{}, err
}
return ct.diffResponsibleRole(control, openControlData)
}
}
27 changes: 15 additions & 12 deletions control/table_test.go → control/summary_table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@ func docFixture(control string) *xml.XmlDocument {
return doc
}

func getTable(control string) xml.Node {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getTable -> getSummaryTable

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that's implied by this file being for summary table tests, no?

doc := docFixture(control)
tables, err := doc.Search("//w:tbl")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice helper! i don't know why I didn't think of this before but instead of having this custom search logic i think we should use ssp.Document's SummaryTables

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is docFixture() is loading from simplified_table.xml rather than a full Word doc, and we can't construct a full Document from that. Let me know what you think of my workaround.

Expect(err).NotTo(HaveOccurred())
return tables[0]
}

func openControlFixturePath() string {
path := filepath.Join("..", "fixtures", "opencontrols")
path, err := filepath.Abs(path)
Expand All @@ -56,27 +63,23 @@ func openControlFixture() opencontrols.Data {
return data
}

var _ = Describe("Table", func() {
var _ = Describe("SummaryTable", func() {
Describe("Fill", func() {
It("fills in the Responsible Role for controls", func() {
doc := docFixture("AC-2")
tables, _ := doc.Search("//w:tbl")
table := tables[0]

ct := Table{Root: table}
table := getTable("AC-2")
ct := SummaryTable{Root: table}
openControlData := openControlFixture()

ct.Fill(openControlData)

Expect(table.Content()).To(ContainSubstring(`Responsible Role: Amazon Elastic Compute Cloud: AWS Staff`))
})

It("fills in the Responsible Role for control enhancements", func() {
doc := docFixture("AC-2 (1)")
tables, _ := doc.Search("//w:tbl")
table := tables[0]

ct := Table{Root: table}
table := getTable("AC-2 (1)")
ct := SummaryTable{Root: table}
openControlData := openControlFixture()

ct.Fill(openControlData)

Expect(table.Content()).To(ContainSubstring(`Responsible Role: Amazon Elastic Compute Cloud: AWS Staff`))
Expand All @@ -88,7 +91,7 @@ var _ = Describe("Table", func() {
tables, _ := doc.Search("//w:tbl")
table := tables[0]

ct := Table{Root: table}
ct := SummaryTable{Root: table}
openControlData := openControlFixture()
diff, err := ct.Diff(openControlData)

Expand Down
27 changes: 22 additions & 5 deletions ssp/document_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,37 @@ import (
. "github.com/onsi/gomega"
)

func loadSSP(name string) *Document {
path := filepath.Join("..", "fixtures", name)
doc, err := Load(path)
Expect(err).NotTo(HaveOccurred())
return doc
}

var _ = Describe("SSP", func() {
Describe("Load", func() {
It("gets the content from the doc", func() {
path := filepath.Join("..", "fixtures", "FedRAMP_ac-2-1_v2.1.docx")
s, err := Load(path)
Expect(err).NotTo(HaveOccurred())
defer s.Close()
doc := loadSSP("FedRAMP_ac-2-1_v2.1.docx")
defer doc.Close()

Expect(s.Content()).To(ContainSubstring("Control Enhancement"))
Expect(doc.Content()).To(ContainSubstring("Control Enhancement"))
})

It("give an error when the doc isn't found", func() {
_, err := Load("non-existent.docx")
Expect(err).To(HaveOccurred())
})
})

Describe("SummaryTables", func() {
It("returns the tables", func() {
doc := loadSSP("FedRAMP_ac-2_v2.1.docx")
defer doc.Close()

tables, err := doc.SummaryTables()

Expect(err).NotTo(HaveOccurred())
Expect(len(tables)).To(Equal(10))
})
})
})
10 changes: 5 additions & 5 deletions templater/templater.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ package templater
import (
"github.com/opencontrol/fedramp-templater/control"
"github.com/opencontrol/fedramp-templater/opencontrols"
"github.com/opencontrol/fedramp-templater/ssp"
"github.com/opencontrol/fedramp-templater/reporter"
"github.com/opencontrol/fedramp-templater/ssp"
)

// TemplatizeSSP inserts OpenControl data into (i.e. modifies) the provided SSP.
Expand All @@ -14,7 +14,7 @@ func TemplatizeSSP(s *ssp.Document, openControlData opencontrols.Data) (err erro
return
}
for _, table := range tables {
ct := control.Table{Root: table}
ct := control.SummaryTable{Root: table}
err = ct.Fill(openControlData)
if err != nil {
return err
Expand All @@ -27,19 +27,19 @@ func TemplatizeSSP(s *ssp.Document, openControlData opencontrols.Data) (err erro
}

// DiffSSP will find the differences between data in the SSP and the OpenControl data.
func DiffSSP(s *ssp.Document, openControlData opencontrols.Data) ([]reporter.Reporter, error){
func DiffSSP(s *ssp.Document, openControlData opencontrols.Data) ([]reporter.Reporter, error) {
var diffInfo []reporter.Reporter
tables, err := s.SummaryTables()
if err != nil {
return diffInfo, err
}
for _, table := range tables {
ct := control.Table{Root: table}
ct := control.SummaryTable{Root: table}
tableDiffInfo, err := ct.Diff(openControlData)
if err != nil {
return diffInfo, err
}
diffInfo = append(diffInfo, tableDiffInfo...)
}
return diffInfo, nil
}
}