dhcpv4: remote OptionGetter interface in favor of Options type.

7 files changed, 80 insertions, 88 deletions

diff --git a/dhcpv4/bsdp/bsdp_test.go b/dhcpv4/bsdp/bsdp_test.go
index cb703da..9f756a7 100644
--- a/dhcpv4/bsdp/bsdp_test.go
+++ b/dhcpv4/bsdp/bsdp_test.go
@@ -9,11 +9,11 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func RequireHasOption(t *testing.T, opts dhcpv4.OptionGetter, opt dhcpv4.Option) {
+func RequireHasOption(t *testing.T, opts dhcpv4.Options, opt dhcpv4.Option) {
 	require.NotNil(t, opts, "must pass list of options")
 	require.NotNil(t, opt, "must pass option")
-	require.True(t, dhcpv4.HasOption(opts, opt.Code()))
-	actual := opts.GetOneOption(opt.Code())
+	require.True(t, opts.Has(opt.Code()))
+	actual := opts.GetOne(opt.Code())
 	require.Equal(t, opt, actual)
 }
 
@@ -66,16 +66,16 @@ func TestNewInformList_NoReplyPort(t *testing.T) {
 	m, err := NewInformList(hwAddr, localIP, 0)
 
 	require.NoError(t, err)
-	require.True(t, dhcpv4.HasOption(m, dhcpv4.OptionVendorSpecificInformation))
-	require.True(t, dhcpv4.HasOption(m, dhcpv4.OptionParameterRequestList))
-	require.True(t, dhcpv4.HasOption(m, dhcpv4.OptionMaximumDHCPMessageSize))
-	require.True(t, dhcpv4.HasOption(m, dhcpv4.OptionEnd))
+	require.True(t, m.Options.Has(dhcpv4.OptionVendorSpecificInformation))
+	require.True(t, m.Options.Has(dhcpv4.OptionParameterRequestList))
+	require.True(t, m.Options.Has(dhcpv4.OptionMaximumDHCPMessageSize))
+	require.True(t, m.Options.Has(dhcpv4.OptionEnd))
 
 	opt := m.GetOneOption(dhcpv4.OptionVendorSpecificInformation)
 	require.NotNil(t, opt, "vendor opts not present")
 	vendorInfo := opt.(*OptVendorSpecificInformation)
-	require.True(t, dhcpv4.HasOption(vendorInfo, OptionMessageType))
-	require.True(t, dhcpv4.HasOption(vendorInfo, OptionVersion))
+	require.True(t, vendorInfo.Options.Has(OptionMessageType))
+	require.True(t, vendorInfo.Options.Has(OptionVersion))
 
 	opt = vendorInfo.GetOneOption(OptionMessageType)
 	require.Equal(t, MessageTypeList, opt.(*OptMessageType).Type)
@@ -97,7 +97,7 @@ func TestNewInformList_ReplyPort(t *testing.T) {
 
 	opt := m.GetOneOption(dhcpv4.OptionVendorSpecificInformation)
 	vendorInfo := opt.(*OptVendorSpecificInformation)
-	require.True(t, dhcpv4.HasOption(vendorInfo, OptionReplyPort))
+	require.True(t, vendorInfo.Options.Has(OptionReplyPort))
 
 	opt = vendorInfo.GetOneOption(OptionReplyPort)
 	require.Equal(t, replyPort, opt.(*OptReplyPort).Port)
@@ -138,21 +138,21 @@ func TestInformSelectForAck_Broadcast(t *testing.T) {
 	require.True(t, m.IsBroadcast())
 
 	// Validate options.
-	require.True(t, dhcpv4.HasOption(m, dhcpv4.OptionClassIdentifier))
-	require.True(t, dhcpv4.HasOption(m, dhcpv4.OptionParameterRequestList))
-	require.True(t, dhcpv4.HasOption(m, dhcpv4.OptionDHCPMessageType))
+	require.True(t, m.Options.Has(dhcpv4.OptionClassIdentifier))
+	require.True(t, m.Options.Has(dhcpv4.OptionParameterRequestList))
+	require.True(t, m.Options.Has(dhcpv4.OptionDHCPMessageType))
 	opt := m.GetOneOption(dhcpv4.OptionDHCPMessageType)
 	require.Equal(t, dhcpv4.MessageTypeInform, opt.(*dhcpv4.OptMessageType).MessageType)
-	require.True(t, dhcpv4.HasOption(m, dhcpv4.OptionEnd))
+	require.True(t, m.Options.Has(dhcpv4.OptionEnd))
 
 	// Validate vendor opts.
-	require.True(t, dhcpv4.HasOption(m, dhcpv4.OptionVendorSpecificInformation))
+	require.True(t, m.Options.Has(dhcpv4.OptionVendorSpecificInformation))
 	opt = m.GetOneOption(dhcpv4.OptionVendorSpecificInformation)
 	vendorInfo := opt.(*OptVendorSpecificInformation)
-	RequireHasOption(t, vendorInfo, &OptMessageType{Type: MessageTypeSelect})
-	require.True(t, dhcpv4.HasOption(vendorInfo, OptionVersion))
-	RequireHasOption(t, vendorInfo, &OptSelectedBootImageID{ID: bootImage.ID})
-	RequireHasOption(t, vendorInfo, &OptServerIdentifier{ServerID: serverID})
+	RequireHasOption(t, vendorInfo.Options, &OptMessageType{Type: MessageTypeSelect})
+	require.True(t, vendorInfo.Options.Has(OptionVersion))
+	RequireHasOption(t, vendorInfo.Options, &OptSelectedBootImageID{ID: bootImage.ID})
+	RequireHasOption(t, vendorInfo.Options, &OptServerIdentifier{ServerID: serverID})
 }
 
 func TestInformSelectForAck_NoServerID(t *testing.T) {
@@ -212,10 +212,10 @@ func TestInformSelectForAck_ReplyPort(t *testing.T) {
 	m, err := InformSelectForAck(*ack, replyPort, bootImage)
 	require.NoError(t, err)
 
-	require.True(t, dhcpv4.HasOption(m, dhcpv4.OptionVendorSpecificInformation))
+	require.True(t, m.Options.Has(dhcpv4.OptionVendorSpecificInformation))
 	opt := m.GetOneOption(dhcpv4.OptionVendorSpecificInformation)
 	vendorInfo := opt.(*OptVendorSpecificInformation)
-	RequireHasOption(t, vendorInfo, &OptReplyPort{Port: replyPort})
+	RequireHasOption(t, vendorInfo.Options, &OptReplyPort{Port: replyPort})
 }
 
 func TestNewReplyForInformList_NoDefaultImage(t *testing.T) {
@@ -276,9 +276,9 @@ func TestNewReplyForInformList(t *testing.T) {
 	require.Equal(t, "bsdp.foo.com", ack.ServerHostName)
 
 	// Validate options.
-	RequireHasOption(t, ack, &dhcpv4.OptMessageType{MessageType: dhcpv4.MessageTypeAck})
-	RequireHasOption(t, ack, &dhcpv4.OptServerIdentifier{ServerID: net.IP{9, 9, 9, 9}})
-	RequireHasOption(t, ack, &dhcpv4.OptClassIdentifier{Identifier: AppleVendorID})
+	RequireHasOption(t, ack.Options, &dhcpv4.OptMessageType{MessageType: dhcpv4.MessageTypeAck})
+	RequireHasOption(t, ack.Options, &dhcpv4.OptServerIdentifier{ServerID: net.IP{9, 9, 9, 9}})
+	RequireHasOption(t, ack.Options, &dhcpv4.OptClassIdentifier{Identifier: AppleVendorID})
 	require.NotNil(t, ack.GetOneOption(dhcpv4.OptionVendorSpecificInformation))
 
 	// Ensure options terminated with End option.
@@ -286,17 +286,17 @@ func TestNewReplyForInformList(t *testing.T) {
 
 	// Vendor-specific options.
 	vendorOpts := ack.GetOneOption(dhcpv4.OptionVendorSpecificInformation).(*OptVendorSpecificInformation)
-	RequireHasOption(t, vendorOpts, &OptMessageType{Type: MessageTypeList})
-	RequireHasOption(t, vendorOpts, &OptDefaultBootImageID{ID: images[0].ID})
-	RequireHasOption(t, vendorOpts, &OptServerPriority{Priority: 0x7070})
-	RequireHasOption(t, vendorOpts, &OptBootImageList{Images: images})
+	RequireHasOption(t, vendorOpts.Options, &OptMessageType{Type: MessageTypeList})
+	RequireHasOption(t, vendorOpts.Options, &OptDefaultBootImageID{ID: images[0].ID})
+	RequireHasOption(t, vendorOpts.Options, &OptServerPriority{Priority: 0x7070})
+	RequireHasOption(t, vendorOpts.Options, &OptBootImageList{Images: images})
 
 	// Add in selected boot image, ensure it's in the generated ACK.
 	config.SelectedImage = &images[0]
 	ack, err = NewReplyForInformList(inform, config)
 	require.NoError(t, err)
 	vendorOpts = ack.GetOneOption(dhcpv4.OptionVendorSpecificInformation).(*OptVendorSpecificInformation)
-	RequireHasOption(t, vendorOpts, &OptSelectedBootImageID{ID: images[0].ID})
+	RequireHasOption(t, vendorOpts.Options, &OptSelectedBootImageID{ID: images[0].ID})
 }
 
 func TestNewReplyForInformSelect_NoSelectedImage(t *testing.T) {
@@ -357,18 +357,18 @@ func TestNewReplyForInformSelect(t *testing.T) {
 	require.Equal(t, "bsdp.foo.com", ack.ServerHostName)
 
 	// Validate options.
-	RequireHasOption(t, ack, &dhcpv4.OptMessageType{MessageType: dhcpv4.MessageTypeAck})
-	RequireHasOption(t, ack, &dhcpv4.OptServerIdentifier{ServerID: net.IP{9, 9, 9, 9}})
-	RequireHasOption(t, ack, &dhcpv4.OptServerIdentifier{ServerID: net.IP{9, 9, 9, 9}})
-	RequireHasOption(t, ack, &dhcpv4.OptClassIdentifier{Identifier: AppleVendorID})
+	RequireHasOption(t, ack.Options, &dhcpv4.OptMessageType{MessageType: dhcpv4.MessageTypeAck})
+	RequireHasOption(t, ack.Options, &dhcpv4.OptServerIdentifier{ServerID: net.IP{9, 9, 9, 9}})
+	RequireHasOption(t, ack.Options, &dhcpv4.OptServerIdentifier{ServerID: net.IP{9, 9, 9, 9}})
+	RequireHasOption(t, ack.Options, &dhcpv4.OptClassIdentifier{Identifier: AppleVendorID})
 	require.NotNil(t, ack.GetOneOption(dhcpv4.OptionVendorSpecificInformation))
 
 	// Ensure options are terminated with End option.
 	require.Equal(t, &dhcpv4.OptionGeneric{OptionCode: dhcpv4.OptionEnd}, ack.Options[len(ack.Options)-1])
 
 	vendorOpts := ack.GetOneOption(dhcpv4.OptionVendorSpecificInformation).(*OptVendorSpecificInformation)
-	RequireHasOption(t, vendorOpts, &OptMessageType{Type: MessageTypeSelect})
-	RequireHasOption(t, vendorOpts, &OptSelectedBootImageID{ID: images[0].ID})
+	RequireHasOption(t, vendorOpts.Options, &OptMessageType{Type: MessageTypeSelect})
+	RequireHasOption(t, vendorOpts.Options, &OptSelectedBootImageID{ID: images[0].ID})
 }
 
 func TestMessageTypeForPacket(t *testing.T) {
diff --git a/dhcpv4/bsdp/option_vendor_specific_information.go b/dhcpv4/bsdp/option_vendor_specific_information.go
index 1bd41a7..4219e40 100644
--- a/dhcpv4/bsdp/option_vendor_specific_information.go
+++ b/dhcpv4/bsdp/option_vendor_specific_information.go
@@ -99,10 +99,10 @@ func (o *OptVendorSpecificInformation) Length() int {
 
 // GetOption returns all suboptions that match the given OptionCode code.
 func (o *OptVendorSpecificInformation) GetOption(code dhcpv4.OptionCode) []dhcpv4.Option {
-	return o.Options.GetOption(code)
+	return o.Options.Get(code)
 }
 
 // GetOneOption returns the first suboption that matches the OptionCode code.
 func (o *OptVendorSpecificInformation) GetOneOption(code dhcpv4.OptionCode) dhcpv4.Option {
-	return o.Options.GetOneOption(code)
+	return o.Options.GetOne(code)
 }
diff --git a/dhcpv4/bsdp/option_vendor_specific_information_test.go b/dhcpv4/bsdp/option_vendor_specific_information_test.go
index 8aa2bdf..689797b 100644
--- a/dhcpv4/bsdp/option_vendor_specific_information_test.go
+++ b/dhcpv4/bsdp/option_vendor_specific_information_test.go
@@ -101,7 +101,7 @@ func TestParseOptVendorSpecificInformation(t *testing.T) {
 		OptionBootImageList,
 		OptionDefaultBootImageID,
 	} {
-		require.True(t, dhcpv4.HasOption(o, opt))
+		require.True(t, o.Options.Has(opt))
 	}
 	optBootImage := o.GetOneOption(OptionBootImageList).(*OptBootImageList)
 	expectedBootImages := []BootImage{
diff --git a/dhcpv4/dhcpv4.go b/dhcpv4/dhcpv4.go
index 2f832e8..98f0eae 100644
--- a/dhcpv4/dhcpv4.go
+++ b/dhcpv4/dhcpv4.go
@@ -367,45 +367,14 @@ func (d *DHCPv4) SetUnicast() {
 // According to RFC 3396, options that are specified more than once are
 // concatenated, and hence this should always just return one option.
 func (d *DHCPv4) GetOption(code OptionCode) []Option {
-	return d.Options.GetOption(code)
+	return d.Options.Get(code)
 }
 
 // GetOneOption will attempt to get an  option that match a Option code.
 // If there are multiple options with the same OptionCode it will only return
 // the first one found.  If no matching option is found nil will be returned.
 func (d *DHCPv4) GetOneOption(code OptionCode) Option {
-	return d.Options.GetOneOption(code)
-}
-
-// Options is a collection of options.
-type Options []Option
-
-// GetOption will attempt to get all options that match a DHCPv4 option
-// from its OptionCode.  If the option was not found it will return an
-// empty list.
-//
-// According to RFC 3396, options that are specified more than once are
-// concatenated, and hence this should always just return one option.
-func (o Options) GetOption(code OptionCode) []Option {
-	opts := []Option{}
-	for _, opt := range o {
-		if opt.Code() == code {
-			opts = append(opts, opt)
-		}
-	}
-	return opts
-}
-
-// GetOneOption will attempt to get an  option that match a Option code.
-// If there are multiple options with the same OptionCode it will only return
-// the first one found.  If no matching option is found nil will be returned.
-func (o Options) GetOneOption(code OptionCode) Option {
-	for _, opt := range o {
-		if opt.Code() == code {
-			return opt
-		}
-	}
-	return nil
+	return d.Options.GetOne(code)
 }
 
 // AddOption appends an option to the existing ones. If the last option is an
@@ -590,15 +559,3 @@ func (d *DHCPv4) ToBytes() []byte {
 	}
 	return buf.Data()
 }
-
-// OptionGetter is a interface that knows how to retrieve an option from a
-// structure of options given an OptionCode.
-type OptionGetter interface {
-	GetOption(OptionCode) []Option
-	GetOneOption(OptionCode) Option
-}
-
-// HasOption checks whether the OptionGetter `o` has the given `opcode` Option.
-func HasOption(o OptionGetter, opcode OptionCode) bool {
-	return o.GetOneOption(opcode) != nil
-}
diff --git a/dhcpv4/dhcpv4_test.go b/dhcpv4/dhcpv4_test.go
index 9be2a1a..276b0fb 100644
--- a/dhcpv4/dhcpv4_test.go
+++ b/dhcpv4/dhcpv4_test.go
@@ -305,8 +305,8 @@ func TestNewDiscovery(t *testing.T) {
 	require.Equal(t, iana.HwTypeEthernet, m.HWType)
 	require.Equal(t, hwAddr, m.ClientHWAddr)
 	require.True(t, m.IsBroadcast())
-	require.True(t, HasOption(m, OptionParameterRequestList))
-	require.True(t, HasOption(m, OptionEnd))
+	require.True(t, m.Options.Has(OptionParameterRequestList))
+	require.True(t, m.Options.Has(OptionEnd))
 }
 
 func TestNewInform(t *testing.T) {
diff --git a/dhcpv4/option_relay_agent_information_test.go b/dhcpv4/option_relay_agent_information_test.go
index bff8ced..8722593 100644
--- a/dhcpv4/option_relay_agent_information_test.go
+++ b/dhcpv4/option_relay_agent_information_test.go
@@ -25,9 +25,9 @@ func TestParseOptRelayAgentInformation(t *testing.T) {
 	opt, err = ParseOptRelayAgentInformation(data[2:])
 	require.NoError(t, err)
 	require.Equal(t, len(opt.Options), 2)
-	circuit := opt.Options.GetOneOption(1).(*OptionGeneric)
+	circuit := opt.Options.GetOne(1).(*OptionGeneric)
 	require.NoError(t, err)
-	remote := opt.Options.GetOneOption(2).(*OptionGeneric)
+	remote := opt.Options.GetOne(2).(*OptionGeneric)
 	require.NoError(t, err)
 	require.Equal(t, circuit.Data, []byte("linux"))
 	require.Equal(t, remote.Data, []byte("boot"))
diff --git a/dhcpv4/options.go b/dhcpv4/options.go
index 215da39..d5162f4 100644
--- a/dhcpv4/options.go
+++ b/dhcpv4/options.go
@@ -95,13 +95,48 @@ func ParseOption(code OptionCode, data []byte) (Option, error) {
 	return opt, nil
 }
 
+// Options is a collection of options.
+type Options []Option
+
+// Get will attempt to get all options that match a DHCPv4 option from its
+// OptionCode.  If the option was not found it will return an empty list.
+//
+// According to RFC 3396, options that are specified more than once are
+// concatenated, and hence this should always just return one option.
+func (o Options) Get(code OptionCode) []Option {
+	opts := []Option{}
+	for _, opt := range o {
+		if opt.Code() == code {
+			opts = append(opts, opt)
+		}
+	}
+	return opts
+}
+
+// GetOne will attempt to get an  option that match a Option code.  If there
+// are multiple options with the same OptionCode it will only return the first
+// one found.  If no matching option is found nil will be returned.
+func (o Options) GetOne(code OptionCode) Option {
+	for _, opt := range o {
+		if opt.Code() == code {
+			return opt
+		}
+	}
+	return nil
+}
+
+// Has checks whether o has the given `opcode` Option.
+func (o Options) Has(code OptionCode) bool {
+	return o.GetOne(code) != nil
+}
+
 // OptionsFromBytes parses a sequence of bytes until the end and builds a list
 // of options from it.
 //
 // The sequence should not contain the DHCP magic cookie.
 //
 // Returns an error if any invalid option or length is found.
-func OptionsFromBytes(data []byte) ([]Option, error) {
+func OptionsFromBytes(data []byte) (Options, error) {
 	return OptionsFromBytesWithParser(data, ParseOption, true)
 }