Skip to content

Commit

Permalink
Merge pull request #70 from ironsmile/bugfix/handlersSettingsPointing…
Browse files Browse the repository at this point in the history 
…ToSameUnderlyingArray

Bugfix/handlers settings pointing to same underlying array
  • Loading branch information
@na--
na-- committed Sep 11, 2015
2 parents 85294e0 + 90f1723 commit 3c9cc3f
Show file tree
Hide file tree
Showing 10 changed files with 222 additions and 104 deletions.
4 changes: 2 additions & 2 deletions app/init.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (a *Application) initFromConfig() (err error) {
}
a.virtualHosts[cfgVhost.Name] = &vhost

if vhost.Logger, err = logger.New(cfgVhost.Logger); err != nil {
if vhost.Logger, err = logger.New(&cfgVhost.Logger); err != nil {
return err
}

Expand Down Expand Up @@ -94,7 +94,7 @@ func (a *Application) initFromConfigLocationsForVHost(cfgLocations []*config.Loc
CacheKey: locCfg.CacheKey,
}

if locations[index].Logger, err = logger.New(locCfg.Logger); err != nil {
if locations[index].Logger, err = logger.New(&locCfg.Logger); err != nil {
return nil, err
}

Expand Down
211 changes: 143 additions & 68 deletions config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func getNormalConfig() *Config {
//!TODO: split into different test case composable builders
c := new(Config)
c.System = System{Pidfile: filepath.Join(os.TempDir(), "nedomi.pid")}
c.Logger = Logger{Type: "nillogger"}
c.Logger = *NewLogger("nillogger", nil)

cz := &CacheZone{
ID: "test1",
Expand All @@ -79,8 +79,8 @@ func getNormalConfig() *Config {
Name: "localhost",
UpstreamType: "simple",
CacheKey: "test",
Handlers: []Handler{Handler{Type: "proxy"}},
Logger: &c.Logger,
Handlers: []Handler{*NewHandler("proxy", nil)},
Logger: c.Logger,
},
CacheZone: cz,
},
Expand Down Expand Up @@ -138,57 +138,71 @@ func TestDuplicateCacheSettings(t *testing.T) {
func TestHandlersParsing(t *testing.T) {
cfg, err := parseBytes([]byte(`
{
"system": {
"pidfile": "/tmp/nedomi_pidfile.pid",
"workdir": "/tmp/"
},
"default_cache_type": "disk",
"default_cache_algorithm": "lru",
"cache_zones": {
"default": {
"path": "/tmp/test/cache1",
"storage_objects": 1023123,
"part_size": "2m"
}
},
"http": {
"listen": ":8282",
"max_headers_size": 1231241212,
"read_timeout": 12312310,
"write_timeout": 213412314,
"default_handlers": [
{ "type" : "via" },
{ "type" : "proxy" }
],
"default_upstream_type": "simple",
"default_cache_zone": "default",
"virtual_hosts": {
"localhost": {
"upstream_address": "http://upstream.com/",
"cache_key": "1.1",
"locations": {
"/status": {
"handlers": [
{ "type" : "gzip"},
{ "type" : "via"},
{ "type" : "status"}
]
},
"~ \\.mp4$": {
},
"~* \\.mp4$": {
"handlers": [ { "type" : "proxy"}]
}
}
},
"127.0.0.2": {
"handlers": [{ "type" : "status" }]
}
}
},
"logger": {
"type": "nillogger"
}
"system": {
"pidfile": "/tmp/nedomi_pidfile.pid",
"workdir": "/tmp/"
},
"default_cache_type": "disk",
"default_cache_algorithm": "lru",
"cache_zones": {
"default": {
"path": "/tmp/test/cache1",
"storage_objects": 1023123,
"part_size": "2m"
}
},
"http": {
"listen": ":8282",
"max_headers_size": 1231241212,
"read_timeout": 12312310,
"write_timeout": 213412314,
"default_handlers": [
{ "type" : "via" },
{ "type" : "proxy", "settings" : {"field" : "proxySetting"}}
],
"default_upstream_type": "simple",
"default_cache_zone": "default",
"virtual_hosts": {
"localhost": {
"upstream_address": "http://upstream.com/",
"cache_key": "1.1",
"locations": {
"/status": {
"handlers": [
{ "type" : "gzip"},
{ "type" : "via", "settings": {"field": "viasetting"}},
{ "type" : "status"}
]
},
"~ \\.mp4$": {
"logger": {
"type": "notLogger",
"settings" : {"field" : "notField"}
}
},
"~* \\.mp4$": {
"handlers": [ { "type" : "proxy"}]
}
},
"logger": {
"type": "localhostLogger",
"settings" : {"field" : "loalField"}
}
},
"127.0.0.2": {
"handlers": [{ "type" : "status" }]
}
},
"logger": {
"type": "httplogger",
"settings" : {"field" : "nilLogger"}
}
},
"logger": {
"type": "nillogger",
"settings" : {"field" : "nilLogger"}
}
}
`))
//!TODO do it with http section only
Expand All @@ -199,26 +213,45 @@ func TestHandlersParsing(t *testing.T) {

// don't touch it works
var mat struct {
Def []string `json:"def"`
Def []DefStruct `json:"def"`
Logger DefStruct `json:"logger"`
Servers map[string]struct {
Def []string `json:"def"`
Locations map[string][]string `json:"locations"`
Def []DefStruct `json:"def"`
Logger DefStruct `json:"logger"`
Locations map[string]struct {
Handlers []DefStruct `json:"handlers"`
Logger DefStruct `json:"logger"`
} `json:"locations"`
} `json:"servers"`
}
matText := []byte(`
{
"def": ["via", "proxy"],
"def": [
{ "type": "via"} ,
{ "type" : "proxy", "setting": "proxySetting"} ],
"logger" : {"type" : "httplogger", "setting" : "nilLogger"},
"servers": {
"localhost": {
"def": ["via", "proxy"],
"def": [{ "type": "via"}, {"type": "proxy", "setting" : "proxySetting"}],
"logger": {"type": "localhostLogger", "setting": "loalField"},
"locations": {
"/status": ["gzip", "via", "status"],
"~ \\.mp4$": ["via", "proxy"],
"~* \\.mp4$": [ "proxy"]
"/status": {
"handlers":[ {"type" :"gzip"}, {"type" : "via", "setting": "viasetting"}, {"type" : "status"}],
"logger": {"type": "localhostLogger", "setting": "loalField"}
},
"~ \\.mp4$": {
"handlers" :[{"type" :"via"}, {"type": "proxy", "setting": "proxySetting"}],
"logger": {"type": "notLogger", "setting": "notField"}
},
"~* \\.mp4$": {
"handlers" : [ {"type":"proxy"}],
"logger": {"type": "localhostLogger", "setting": "loalField"}
}
}
},
"127.0.0.2": {
"def": ["status" ]
"def": [ {"type":"status"} ],
"logger" : {"type" : "httplogger", "setting" : "nilLogger"}
}
}
}`)
Expand All @@ -228,6 +261,7 @@ func TestHandlersParsing(t *testing.T) {
}

checkHandlerTypes(t, "Default Handlers are wrong", cfg.HTTP.DefaultHandlers, mat.Def)
checkLogger(t, "Default Logger is wrong", cfg.HTTP.Logger, mat.Logger)
if len(cfg.HTTP.Servers) != len(mat.Servers) {
t.Errorf("expected %d server cfgs got %d", len(mat.Servers), len(cfg.HTTP.Servers))
}
Expand All @@ -238,14 +272,15 @@ func TestHandlersParsing(t *testing.T) {
continue
}
checkHandlerTypes(t, fmt.Sprintf("Default Handlers for server %s are wrong", server), server.Handlers, serverExpect.Def)
checkLogger(t, fmt.Sprintf("Default logger for server %s is wrong", server), server.Logger, serverExpect.Logger)
for _, location := range server.Locations {
locationExpect, ok := serverExpect.Locations[location.Name]
if !ok {
t.Errorf("got a location %s that is not expected", location)
continue
}
checkHandlerTypes(t, fmt.Sprintf("Handlers for location %s are wrong", location), location.Handlers, locationExpect)

checkHandlerTypes(t, fmt.Sprintf("Handlers for location %s are wrong", location), location.Handlers, locationExpect.Handlers)
checkLogger(t, fmt.Sprintf("Logger for location %s is wrong", location), location.Logger, locationExpect.Logger)
delete(serverExpect.Locations, location.Name)
}
if len(serverExpect.Locations) != 0 {
Expand All @@ -259,15 +294,55 @@ func TestHandlersParsing(t *testing.T) {
}
}

func checkHandlerTypes(t *testing.T, msg string, handlers []Handler, handlerTypes []string) {
func checkLogger(t *testing.T, msg string, logger Logger, loggerExpect DefStruct) {
if loggerExpect.Type != logger.Type {
t.Errorf("%s expected `%s`, got `%s`", msg, loggerExpect.Type, logger.Type)
return
}
var s struct {
Field string `json:"field"`
}
if len(logger.Settings) != 0 {
err := json.Unmarshal(logger.Settings, &s)
if err != nil {
t.Errorf("got error while parsing Settings for logger %s with raw settings `%s` - %s", logger.Type, string(logger.Settings), err)
}
if s.Field != loggerExpect.Setting {
t.Errorf("%s expected to have Setting `%s`, got `%s`", msg, loggerExpect.Setting, s.Field)
}
} else if len(loggerExpect.Setting) > 0 {
t.Errorf("%s expected to have Setting `%s`, but no Setting was found", msg, loggerExpect.Setting)
}
}

func checkHandlerTypes(t *testing.T, msg string, handlers []Handler, handlerTypes []DefStruct) {
if len(handlers) != len(handlerTypes) {
t.Errorf("%s expected `%s`, got `%s`", msg, handlerTypes, handlers)
return
}
for index := range handlerTypes {
if handlers[index].Type != handlerTypes[index] {
t.Errorf("%s expected `%s`, got `%s`", msg, handlerTypes, handlers)
if handlers[index].Type != handlerTypes[index].Type {
t.Errorf("%s expected `%s`, got `%s`", msg, handlerTypes[index].Type, handlers[index].Type)
return
}
var s struct {
Field string `json:"field"`
}
if len(handlers[index].Settings) != 0 {
err := json.Unmarshal(handlers[index].Settings, &s)
if err != nil {
t.Errorf("got error while parsing Settings for handler %s with raw settings `%s` - %s", handlers[index].Type, string(handlers[index].Settings), err)
}
if s.Field != handlerTypes[index].Setting {
t.Errorf("%s expected to have Setting `%s`, got `%s`", msg, handlerTypes[index].Setting, s.Field)
}
} else if len(handlerTypes[index].Setting) > 0 {
t.Errorf("%s expected to have Setting `%s`, but no Setting was found", msg, handlerTypes[index].Setting)
}
}
}

type DefStruct struct {
Type string `json:"type"`
Setting string `json:"setting"`
}
30 changes: 25 additions & 5 deletions config/section_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,35 @@ import (
// ErrHandlerWithNoType is returned when a Handler without a type is validated
var ErrHandlerWithNoType = errors.New("handler must have a type")

// Handler contains handler options.
type Handler struct {
type handlerBase struct {
Type string `json:"type"`
Settings json.RawMessage `json:"settings"`
}

// Handler contains handler options.
type Handler struct {
handlerBase
}

// NewHandler creates a new Handler with the provided type name and settings
func NewHandler(name string, setting json.RawMessage) *Handler {
return &Handler{
handlerBase: handlerBase{
Type: name,
Settings: setting,
},
}
}

// UnmarshalJSON is a custom JSON unmarshalling where custom stands for resetting the Settings field.
func (h *Handler) UnmarshalJSON(buff []byte) error {
h.Settings = append(json.RawMessage{}, h.Settings...)
return json.Unmarshal(buff, &h.handlerBase)
}

// Validate checks a Handler config section config for errors.
func (l Handler) Validate() error {
if l.Type == "" {
func (h Handler) Validate() error {
if h.Type == "" {
return ErrHandlerWithNoType
}

Expand All @@ -25,6 +45,6 @@ func (l Handler) Validate() error {
}

// GetSubsections returns nil (Handler has no subsections).
func (l Handler) GetSubsections() []Section {
func (h Handler) GetSubsections() []Section {
return nil
}
2 changes: 1 addition & 1 deletion config/section_location.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ type baseLocation struct {
CacheZone string `json:"cache_zone"`
CacheKey string `json:"cache_key"`
Handlers []Handler `json:"handlers"`
Logger *Logger `json:"logger"`
Logger Logger `json:"logger"`
}

// Location contains all configuration options for virtual host's location.
Expand Down
24 changes: 22 additions & 2 deletions config/section_logger.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,32 @@ import (
"errors"
)

// Logger contains logger options.
type Logger struct {
type loggerBase struct {
Type string `json:"type"`
Settings json.RawMessage `json:"settings"`
}

// Logger contains logger options.
type Logger struct {
loggerBase
}

// UnmarshalJSON is a custom JSON unmarshalling where custom stands for resetting the Settings field.
func (l *Logger) UnmarshalJSON(buff []byte) error {
l.Settings = append(json.RawMessage{}, l.Settings...)
return json.Unmarshal(buff, &l.loggerBase)
}

// NewLogger creates a new Logger with the provided type name and settings
func NewLogger(name string, settings json.RawMessage) *Logger {
return &Logger{
loggerBase: loggerBase{
Type: name,
Settings: settings,
},
}
}

// Validate checks a Logger config section config for errors.
func (l Logger) Validate() error {

Expand Down
Loading

0 comments on commit 3c9cc3f

Please sign in to comment.