Skip to content

Commit

Permalink
Goreportcart fixes #78 (#82)
Browse files Browse the repository at this point in the history
* Goreportcart fixes #78

* Reduce cyclomatic complexity
  • Loading branch information
danielpoe authored Oct 24, 2019
1 parent e683ad0 commit f99be47
Show file tree
Hide file tree
Showing 11 changed files with 96 additions and 87 deletions.
3 changes: 1 addition & 2 deletions core/cache/httpFrontend.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ func (hf *HTTPFrontend) load(ctx context.Context, key string, loader HTTPLoader)

defer func() {
if err := recover(); err != nil {
//resultErr = errors.WithStack(errors.Errorf("%#v", err))
if err2, ok := err.(error); ok {
resultErr = errors.WithStack(err2) //errors.Errorf("%#v", err)
} else {
Expand Down Expand Up @@ -174,7 +173,7 @@ func (hf *HTTPFrontend) load(ctx context.Context, key string, loader HTTPLoader)
cached = loadedData.(cachedResponse)
}

hf.logger.WithField("category", "httpFrontendCache").Debug("Store in Cache", key, data.(loaderResponse).meta)
hf.logger.WithContext(ctx).WithField("category", "httpFrontendCache").Debug("Store in Cache", key, data.(loaderResponse).meta)
hf.backend.Set(key, &Entry{
Data: cached,
Meta: Meta{
Expand Down
3 changes: 3 additions & 0 deletions core/gotemplate/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,9 @@ func (e *engine) parseLayoutTemplates(functionsMap template.FuncMap, funcs templ
return nil, err
}
templateName, err := filepath.Rel(dir, file)
if err != nil {
return nil, err
}
t := tpl.New(templateName)

_, err = t.Parse(string(tContent))
Expand Down
5 changes: 3 additions & 2 deletions core/locale/domain/dateTime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ var formatTestData = []formatTest{

func TestFormat(t *testing.T) {
for _, testData := range formatTestData {
f := testGetFormatter(testData.timeStamp, testData.location)
f := testGetFormatter(t, testData.timeStamp, testData.location)

assert.Equal(
t,
Expand Down Expand Up @@ -63,7 +63,7 @@ func TestFormat(t *testing.T) {
}
}

func testGetFormatter(timeString string, locationString string) *DateTimeFormatter {
func testGetFormatter(t *testing.T, timeString string, locationString string) *DateTimeFormatter {
f := DateTimeFormatter{
DateFormat: "02 Jan 2006",
TimeFormat: "15:04",
Expand All @@ -76,6 +76,7 @@ func testGetFormatter(timeString string, locationString string) *DateTimeFormatt
panic(e)
}
location, e := time.LoadLocation(locationString)
assert.NoError(t, e)
localTime := dateTime.In(location)

f.SetDateTime(dateTime, localTime)
Expand Down
2 changes: 1 addition & 1 deletion core/oauth/application/authmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func (f *loggingRoundTripper) RoundTrip(req *http.Request) (*http.Response, erro
log.Println("############### OAUTH REQUEST:")
log.Printf("%v %v ", string(b), err)
res, err := f.originalTransport.RoundTrip(req)
b, err = httputil.DumpResponse(res, true)
b, _ = httputil.DumpResponse(res, true)
log.Println("############### OAUTH RESPONSE:")
log.Printf("%v %v ", string(b), err)
log.Println("############### OAUTH Call Stack:")
Expand Down
137 changes: 69 additions & 68 deletions core/requestlogger/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,74 +71,75 @@ func (r *logger) Filter(ctx context.Context, req *web.Request, w http.ResponseWr

webResponse := chain.Next(ctx, req, w)

return &loggedResponse{
result: webResponse,
logCallback: func(rwl *responseWriterLogger) {
var cp func(msg interface{}, styles ...string) string
switch {
case rwl.statusCode >= 200 && rwl.statusCode < 300:
cp = color.Green
case rwl.statusCode >= 300 && rwl.statusCode < 400:
cp = color.Blue
case rwl.statusCode >= 400 && rwl.statusCode < 500:
cp = color.Yellow
case rwl.statusCode == 0 || (rwl.statusCode >= 500 && rwl.statusCode < 600):
cp = color.Red
default:
cp = color.Grey
}

extra := new(strings.Builder)

switch r := webResponse.(type) {
case *web.URLRedirectResponse:
extra.WriteString("-> " + r.URL.String())

case *web.RouteRedirectResponse:
extra.WriteString("-> " + r.To)

case *web.ServerErrorResponse:
extra.WriteString(strings.Split(fmt.Sprintf(`Error: %s`, r.Error.Error()), "\n")[0])
}

var sizeStr string
if rwl.length > 99999 {
sizeStr = strconv.Itoa(rwl.length/1000) + "kb"
} else {
sizeStr = strconv.Itoa(rwl.length) + "b"
}

duration := time.Since(start)

l := r.logger.
WithContext(ctx).
WithFields(
map[flamingo.LogKey]interface{}{
flamingo.LogKeyAccesslog: 1,
flamingo.LogKeyResponseCode: rwl.statusCode,
flamingo.LogKeyResponseTime: duration,
flamingo.LogKeyReferer: req.Request().Referer(),
flamingo.LogKeyClientIP: strings.Join(req.RemoteAddress(), ", "),
flamingo.LogKeyBusinessID: req.Request().Header.Get("X-Business-ID"),
},
)

var extraStr string
if extra.Len() > 0 {
extraStr = " (" + extra.String() + ")"
}

l.Info(
fmt.Sprintf(
cp("%s %s %d: %s in %s%s"),
req.Request().Method,
req.Request().RequestURI,
rwl.statusCode,
sizeStr,
duration,
extraStr,
),
logCallbackFunc := func(rwl *responseWriterLogger) {
var cp func(msg interface{}, styles ...string) string
switch {
case rwl.statusCode >= 200 && rwl.statusCode < 300:
cp = color.Green
case rwl.statusCode >= 300 && rwl.statusCode < 400:
cp = color.Blue
case rwl.statusCode >= 400 && rwl.statusCode < 500:
cp = color.Yellow
case rwl.statusCode == 0 || (rwl.statusCode >= 500 && rwl.statusCode < 600):
cp = color.Red
default:
cp = color.Grey
}

extra := new(strings.Builder)

switch r := webResponse.(type) {
case *web.URLRedirectResponse:
extra.WriteString("-> " + r.URL.String())

case *web.RouteRedirectResponse:
extra.WriteString("-> " + r.To)

case *web.ServerErrorResponse:
extra.WriteString(strings.Split(fmt.Sprintf(`Error: %s`, r.Error.Error()), "\n")[0])
}

var sizeStr string
if rwl.length > 99999 {
sizeStr = strconv.Itoa(rwl.length/1000) + "kb"
} else {
sizeStr = strconv.Itoa(rwl.length) + "b"
}

duration := time.Since(start)

l := r.logger.
WithContext(ctx).
WithFields(
map[flamingo.LogKey]interface{}{
flamingo.LogKeyAccesslog: 1,
flamingo.LogKeyResponseCode: rwl.statusCode,
flamingo.LogKeyResponseTime: duration,
flamingo.LogKeyReferer: req.Request().Referer(),
flamingo.LogKeyClientIP: strings.Join(req.RemoteAddress(), ", "),
flamingo.LogKeyBusinessID: req.Request().Header.Get("X-Business-ID"),
},
)
},

var extraStr string
if extra.Len() > 0 {
extraStr = " (" + extra.String() + ")"
}

l.Info(
fmt.Sprintf(
cp("%s %s %d: %s in %s%s"),
req.Request().Method,
req.Request().RequestURI,
rwl.statusCode,
sizeStr,
duration,
extraStr,
),
)
}
return &loggedResponse{
result: webResponse,
logCallback: logCallbackFunc,
}
}
19 changes: 12 additions & 7 deletions framework/config/area.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,12 +313,7 @@ func (area *Area) GetInitializedInjector() (*dingo.Injector, error) {

if config, ok := area.Configuration.Get("flamingo.modules.disabled"); ok {
for _, disabled := range config.(Slice) {
for i, module := range area.Modules {
tm := reflect.TypeOf(module).Elem()
if tm.PkgPath()+"."+tm.Name() == disabled.(string) {
area.Modules = append(area.Modules[:i], area.Modules[i+1:]...)
}
}
area.Modules = disableModule(area.Modules, disabled.(string))
}
}

Expand All @@ -327,7 +322,17 @@ func (area *Area) GetInitializedInjector() (*dingo.Injector, error) {
return injector, nil
}

// Flat returns a map of name->*Area of contexts, were all values have been inherited (yet overriden) of the parent context tree.
func disableModule(input []dingo.Module, disabled string) []dingo.Module {
for i, module := range input {
tm := reflect.TypeOf(module).Elem()
if tm.PkgPath()+"."+tm.Name() == disabled {
return append(input[:i], input[i+1:]...)
}
}
return input
}

// Flat returns a map of name->*Area of contexts, were all values have been inherited (yet overridden) of the parent context tree.
func (area *Area) Flat() (map[string]*Area, error) {
res := make(map[string]*Area)
res[area.Name] = area
Expand Down
4 changes: 2 additions & 2 deletions framework/config/area_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,10 @@ func TestMap_Get(t *testing.T) {
assert.True(t, ok)
assert.Equal(t, Map{"x": Map{"y": Map{"z": "test"}}}, val)

val, ok = m.Get("foo.bar.baz")
_, ok = m.Get("foo.bar.baz")
assert.False(t, ok)

val, ok = m.Get("unknown")
_, ok = m.Get("unknown")
assert.False(t, ok)
}

Expand Down
2 changes: 1 addition & 1 deletion framework/config/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (

type (
// Module defines a dingo module which automatically binds provided config.
// Normaly this module is not included in your flamingo projects bootstrap.
// Normally this module is not included in your flamingo projects bootstrap.
//
// Its can be useful for testing dingo.Module that require certain configuration to be set before. E.g.:
//
Expand Down
2 changes: 1 addition & 1 deletion framework/opencensus/sampler.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func URLPrefixSampler(whitelist, blacklist []string, allowParentTrace bool) func
}
}

// check sampling decision agains blacklist
// check sampling decision against blacklist
for _, p := range blacklist {
if strings.HasPrefix(path, p) {
sample = false
Expand Down
4 changes: 2 additions & 2 deletions framework/prefixrouter/front_router.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func (fr *FrontRouter) ServeHTTP(w http.ResponseWriter, req *http.Request) {
req = req.WithContext(ctx)
defer span.End()

// process registered primaryHandlers - and if they are sucessfull exist
// process registered primaryHandlers - and if they are successful exist
for _, handler := range fr.primaryHandlers {
proceed, _ := handler.TryServeHTTP(w, req)
if !proceed {
Expand Down Expand Up @@ -151,7 +151,7 @@ func (fr *FrontRouter) ServeHTTP(w http.ResponseWriter, req *http.Request) {
return
}

// process registered fallbackHandlers - and if they are sucessfull exist
// process registered fallbackHandlers - and if they are successful exist
for _, handler := range fr.fallbackHandlers {
proceed, _ := handler.TryServeHTTP(w, req)
if !proceed {
Expand Down
2 changes: 1 addition & 1 deletion framework/web/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ type (
MaxAge int
// SMaxAge defines the maxAge for shared caches. Supposed to override max-age for CDN for example
SMaxAge int
// ETag the key for the Reponse
// ETag the key for the Response
ETag string
// LastModifiedSince indicates the time a document last changed
LastModifiedSince *time.Time
Expand Down

0 comments on commit f99be47

Please sign in to comment.