Skip to content

Commit

Permalink
Dont log debug output to StdOut (#107)
Browse files Browse the repository at this point in the history 
Logging 'asumming this is handled separately' isn't useful information -
now we log this on our logger which'll only pipe to StdOut in debug
mode.

Additionally makes logger the second argument to our functions as that's
a more common pattern.
  • Loading branch information
@paprikati
paprikati authored Mar 25, 2024
1 parent 166b1bc commit 010950b
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 37 deletions.
4 changes: 2 additions & 2 deletions cmd/catalog-importer/cmd/sync.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -341,14 +341,14 @@ func (opt *SyncOptions) Run(ctx context.Context, logger kitlog.Logger, cfg *conf
OUT("\n ↻ %s", outputType.TypeName)

// Filter source for each of the output types
entries, err := output.Collect(ctx, outputType, sourcedEntries, logger)
entries, err := output.Collect(ctx, logger, outputType, sourcedEntries)
if err != nil {
return errors.Wrap(err, fmt.Sprintf("outputs.%d (type_name='%s')", idx, outputType.TypeName))
}
OUT(" ✔ Building entries... (found %d entries matching filters)", len(entries))

// Marshal entries using the JS expressions.
entryModels, err := output.MarshalEntries(ctx, outputType, entries, logger)
entryModels, err := output.MarshalEntries(ctx, logger, outputType, entries)
if err != nil {
return errors.Wrap(err, fmt.Sprintf("outputs.%d (type_name='%s')", idx, outputType.TypeName))
}
Expand Down
18 changes: 9 additions & 9 deletions expr/js_eval.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func init() {

// EvaluateJavascript can evaluate a source Javascript program having set the given
// subject into the `$` variable.
func EvaluateJavascript(ctx context.Context, source string, subject any, logger kitlog.Logger) (result otto.Value, err error) {
func EvaluateJavascript(ctx context.Context, logger kitlog.Logger, source string, subject any) (result otto.Value, err error) {
var halted bool
defer func() {
if caught := recover(); caught != nil {
Expand Down Expand Up @@ -78,8 +78,8 @@ func EvaluateJavascript(ctx context.Context, source string, subject any, logger

}

func EvaluateArray[ReturnType any](ctx context.Context, source string, subject any, logger kitlog.Logger) ([]ReturnType, error) {
result, err := EvaluateJavascript(ctx, source, subject, logger)
func EvaluateArray[ReturnType any](ctx context.Context, logger kitlog.Logger, source string, subject any) ([]ReturnType, error) {
result, err := EvaluateJavascript(ctx, logger, source, subject)
if err != nil {
return nil, errors.Wrap(err, "evaluating array value")
}
Expand Down Expand Up @@ -115,7 +115,7 @@ func EvaluateArray[ReturnType any](ctx context.Context, source string, subject a
// Now parse each nested value and return the final slice.
resultValues := []ReturnType{}
for _, evaluatedValue := range evaluatedValues {
resultValue, err := EvaluateResultType[ReturnType](ctx, source, evaluatedValue)
resultValue, err := EvaluateResultType[ReturnType](ctx, logger, source, evaluatedValue)
if err != nil {
return nil, nil
}
Expand All @@ -127,25 +127,25 @@ func EvaluateArray[ReturnType any](ctx context.Context, source string, subject a
return resultValues, nil
}

func EvaluateSingleValue[ReturnType any](ctx context.Context, source string, subject any, logger kitlog.Logger) (*ReturnType, error) {
func EvaluateSingleValue[ReturnType any](ctx context.Context, logger kitlog.Logger, source string, subject any) (*ReturnType, error) {
var emptyResult *ReturnType
result, err := EvaluateJavascript(ctx, source, subject, logger)
result, err := EvaluateJavascript(ctx, logger, source, subject)
if err != nil {
return emptyResult, errors.Wrap(err, "evaluating single value")
}
if result.IsNull() || result.IsUndefined() {
return nil, nil
}

resultValue, err := EvaluateResultType[ReturnType](ctx, source, result)
resultValue, err := EvaluateResultType[ReturnType](ctx, logger, source, result)
if err != nil {
return nil, err
}

return resultValue, nil
}

func EvaluateResultType[ReturnType any](ctx context.Context, source string, result otto.Value) (*ReturnType, error) {
func EvaluateResultType[ReturnType any](ctx context.Context, logger kitlog.Logger, source string, result otto.Value) (*ReturnType, error) {
var resultValue *ReturnType
switch {
case result.IsBoolean():
Expand Down Expand Up @@ -217,7 +217,7 @@ func EvaluateResultType[ReturnType any](ctx context.Context, source string, resu
return resultValue, nil

case isArray(result):
fmt.Fprintf(os.Stdout, "\n Source %s evaluates to an array. Assuming this is handled separately\n", source)
logger.Log("\n Source %s evaluates to an array. Assuming this is handled separately\n", source)
return resultValue, nil

default:
Expand Down
26 changes: 13 additions & 13 deletions expr/js_eval_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,63 +44,63 @@ var _ = Describe("Javascript evaluation", func() {
When("parsing attribute sources", func() {
It("returns the correct top-level attribute", func() {
topLevelSrc := "$.name"
evaluatedResult, err := EvaluateSingleValue[string](ctx, topLevelSrc, sourceEntry, logger)
evaluatedResult, err := EvaluateSingleValue[string](ctx, logger, topLevelSrc, sourceEntry)
Expect(err).NotTo(HaveOccurred())
Expect(*evaluatedResult).To(Equal(sourceEntry["name"]))
})

It("returns a bool as expected", func() {
topLevelSrc := "$.important"
evaluatedResult, err := EvaluateSingleValue[bool](ctx, topLevelSrc, sourceEntry, logger)
evaluatedResult, err := EvaluateSingleValue[bool](ctx, logger, topLevelSrc, sourceEntry)
Expect(err).NotTo(HaveOccurred())
Expect(*evaluatedResult).To(Equal(sourceEntry["important"]))
})

It("returns a number as expected", func() {
topLevelSrc := "$.importance_score"
evaluatedResult, err := EvaluateSingleValue[int](ctx, topLevelSrc, sourceEntry, logger)
evaluatedResult, err := EvaluateSingleValue[int](ctx, logger, topLevelSrc, sourceEntry)
Expect(err).NotTo(HaveOccurred())
Expect(*evaluatedResult).To(Equal(sourceEntry["importance_score"]))
})

It("returns a string as expected", func() {
topLevelSrc := "$.description"
evaluatedResult, err := EvaluateSingleValue[string](ctx, topLevelSrc, sourceEntry, logger)
evaluatedResult, err := EvaluateSingleValue[string](ctx, logger, topLevelSrc, sourceEntry)
Expect(err).NotTo(HaveOccurred())
Expect(*evaluatedResult).To(Equal(sourceEntry["description"]))
})

It("does not parse a value if given the wrong type", func() {
topLevelSrc := "$.description"
_, err := EvaluateSingleValue[int](ctx, topLevelSrc, sourceEntry, logger)
_, err := EvaluateSingleValue[int](ctx, logger, topLevelSrc, sourceEntry)
Expect(err).To(HaveOccurred(), "could not convert result of string to int")
})

It("returns nil if the type is not supported", func() {
topLevelSrc := "$.metadata"
evaluatedResult, err := EvaluateSingleValue[string](ctx, topLevelSrc, sourceEntry, logger)
evaluatedResult, err := EvaluateSingleValue[string](ctx, logger, topLevelSrc, sourceEntry)
Expect(err).NotTo(HaveOccurred())
Expect(evaluatedResult).To(BeNil())
})
})

It("manipulates string values as expected", func() {
topLevelSrc := "$.name.replace('Component', 'Replacement')"
evaluatedResult, err := EvaluateSingleValue[string](ctx, topLevelSrc, sourceEntry, logger)
evaluatedResult, err := EvaluateSingleValue[string](ctx, logger, topLevelSrc, sourceEntry)
Expect(err).NotTo(HaveOccurred())
Expect(*evaluatedResult).To(Equal("Replacement name"))
})

It("parses nested values as expected", func() {
topLevelSrc := "$.metadata.namespace"
evaluatedResult, err := EvaluateSingleValue[string](ctx, topLevelSrc, sourceEntry, logger)
evaluatedResult, err := EvaluateSingleValue[string](ctx, logger, topLevelSrc, sourceEntry)
Expect(err).NotTo(HaveOccurred())
Expect(*evaluatedResult).To(Equal(sourceEntry["metadata"].(map[string]any)["namespace"]))
})

It("handles possible null values with _.get", func() {
nestedSrc := "_.get($.metadata, \"badKey\", \"default value\")"
evaluatedResult, err := EvaluateSingleValue[string](ctx, nestedSrc, sourceEntry, logger)
evaluatedResult, err := EvaluateSingleValue[string](ctx, logger, nestedSrc, sourceEntry)
Expect(err).NotTo(HaveOccurred())
Expect(*evaluatedResult).To(Equal("default value"))
})
Expand All @@ -111,14 +111,14 @@ var _ = Describe("Javascript evaluation", func() {
entryName, ok := sourceEntryWithArray["name"].(string)
Expect(ok).To(BeTrue())
expectedResult := []string{entryName}
evaluatedResult, err := EvaluateArray[string](ctx, topLevelSrc, sourceEntryWithArray, logger)
evaluatedResult, err := EvaluateArray[string](ctx, logger, topLevelSrc, sourceEntryWithArray)
Expect(err).NotTo(HaveOccurred())
Expect(evaluatedResult).To(Equal(expectedResult))
})

It("works as expected when given actual array input", func() {
topLevelSrc := "$.domains"
evaluatedResult, err := EvaluateArray[string](ctx, topLevelSrc, sourceEntryWithArray, logger)
evaluatedResult, err := EvaluateArray[string](ctx, logger, topLevelSrc, sourceEntryWithArray)
Expect(err).NotTo(HaveOccurred())
Expect(evaluatedResult).To(Equal(sourceEntryWithArray["domains"]))
})
Expand All @@ -127,14 +127,14 @@ var _ = Describe("Javascript evaluation", func() {
When("sending invalid source javascript", func() {
It("returns nothing if I send a key that isn't present on the entry", func() {
topLevelSrc := "$.ghostkey"
evaluatedResult, err := EvaluateSingleValue[string](ctx, topLevelSrc, sourceEntry, logger)
evaluatedResult, err := EvaluateSingleValue[string](ctx, logger, topLevelSrc, sourceEntry)
Expect(err).NotTo(HaveOccurred())
Expect(evaluatedResult).To(BeNil())
})

It("returns nil if my JS is invalid", func() {
topLevelSrc := "$badKey"
evaluatedResult, err := EvaluateArray[string](ctx, topLevelSrc, sourceEntryWithArray, logger)
evaluatedResult, err := EvaluateArray[string](ctx, logger, topLevelSrc, sourceEntryWithArray)
Expect(err).NotTo(HaveOccurred())
// Expecting an array with an empty string here, as that is the empty state for this function
Expect(evaluatedResult).To(BeNil())
Expand Down
4 changes: 2 additions & 2 deletions output/collect.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

// Collect filters the list of entries against the source filter on the output, returning
// a list of all entries which pass the filter.
func Collect(ctx context.Context, output *Output, entries []source.Entry, logger kitlog.Logger) ([]source.Entry, error) {
func Collect(ctx context.Context, logger kitlog.Logger, output *Output, entries []source.Entry) ([]source.Entry, error) {
if !output.Source.Filter.Valid {
return entries, nil // no-op, the filter is blank
}
Expand All @@ -20,7 +20,7 @@ func Collect(ctx context.Context, output *Output, entries []source.Entry, logger

filteredEntries := []source.Entry{}
for _, entry := range entries {
result, err := expr.EvaluateSingleValue[bool](ctx, src, entry, logger)
result, err := expr.EvaluateSingleValue[bool](ctx, logger, src, entry)
if err != nil {
return nil, errors.Wrap(err, "evaluating filter for entry")
}
Expand Down
16 changes: 8 additions & 8 deletions output/marshal.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func MarshalType(output *Output) (base *CatalogTypeModel, enumTypes []*CatalogTy
//
// The majority of the work comes from compiling and evaluating the JS expressions that
// marshal the catalog entries from source.
func MarshalEntries(ctx context.Context, output *Output, entries []source.Entry, logger kitlog.Logger) ([]*CatalogEntryModel, error) {
func MarshalEntries(ctx context.Context, logger kitlog.Logger, output *Output, entries []source.Entry) ([]*CatalogEntryModel, error) {
nameSource := output.Source.Name
externalIDSource := output.Source.ExternalID
aliasesSource := output.Source.Aliases
Expand All @@ -108,20 +108,20 @@ func MarshalEntries(ctx context.Context, output *Output, entries []source.Entry,

catalogEntryModels := []*CatalogEntryModel{}
for _, entry := range entries {
name, err := expr.EvaluateSingleValue[string](ctx, nameSource, entry, logger)
name, err := expr.EvaluateSingleValue[string](ctx, logger, nameSource, entry)
if err != nil {
return nil, errors.Wrap(err, "evaluating entry name")
}

externalID, err := expr.EvaluateSingleValue[string](ctx, externalIDSource, entry, logger)
externalID, err := expr.EvaluateSingleValue[string](ctx, logger, externalIDSource, entry)
if err != nil {
return nil, errors.Wrap(err, "evaluating entry external ID")
}

var rank *int
if rankSource := output.Source.Rank; rankSource.Valid && rankSource.String != "" {
var err error
rank, err = expr.EvaluateSingleValue[int](ctx, rankSource.String, entry, logger)
rank, err = expr.EvaluateSingleValue[int](ctx, logger, rankSource.String, entry)
if err != nil {
return nil, errors.Wrap(err, "evaluating entry rank")
}
Expand All @@ -132,12 +132,12 @@ func MarshalEntries(ctx context.Context, output *Output, entries []source.Entry,
aliases := []string{}
for idx, aliasSource := range aliasesSource {
toAdd := []string{}
alias, err := expr.EvaluateSingleValue[string](ctx, aliasSource, entry, logger)
alias, err := expr.EvaluateSingleValue[string](ctx, logger, aliasSource, entry)
if err != nil {
return nil, errors.Wrap(err, fmt.Sprintf("aliases.%d: evaluating entry alias", idx))
}
if alias == nil {
aliasArray, arrayErr := expr.EvaluateArray[string](ctx, aliasSource, entry, logger)
aliasArray, arrayErr := expr.EvaluateArray[string](ctx, logger, aliasSource, entry)
if arrayErr != nil {
return nil, errors.Wrap(err, fmt.Sprintf("aliases.%d: evaluating entry alias", idx))
}
Expand All @@ -161,7 +161,7 @@ func MarshalEntries(ctx context.Context, output *Output, entries []source.Entry,
binding := client.CatalogAttributeBindingPayloadV2{}

if attributeByID[attributeID].Array {
valueLiterals, err := expr.EvaluateArray[any](ctx, src, entry, logger)
valueLiterals, err := expr.EvaluateArray[any](ctx, logger, src, entry)
if err != nil {
return catalogEntryModels, errors.Wrap(err, "evaluating attribute")
}
Expand Down Expand Up @@ -251,7 +251,7 @@ func evaluateEntryWithAttributeType(ctx context.Context, src string, entry map[s
}

func evaluateEntryWithType[ReturnType any](ctx context.Context, src string, entry map[string]any, logger kitlog.Logger) (*string, error) {
literal, err := expr.EvaluateSingleValue[ReturnType](ctx, src, entry, logger)
literal, err := expr.EvaluateSingleValue[ReturnType](ctx, logger, src, entry)
if err != nil {
return nil, err
}
Expand Down
6 changes: 3 additions & 3 deletions output/marshal_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ var _ = Describe("Marshalling data", func() {

entries := []source.Entry{sourceEntry}

res, err := MarshalEntries(ctx, catalogTypeOutput, entries, logger)
res, err := MarshalEntries(ctx, logger, catalogTypeOutput, entries)

expectedAliasResult := []string{"aliasInAnArray", "anotherAliasInAnArray"}
Expect(err).NotTo(HaveOccurred())
Expand All @@ -68,7 +68,7 @@ var _ = Describe("Marshalling data", func() {
"aliases": "singleAlias",
}
entries := []source.Entry{sourceEntry}
res, err := MarshalEntries(ctx, catalogTypeOutput, entries, logger)
res, err := MarshalEntries(ctx, logger, catalogTypeOutput, entries)
expectedAliasResult := []string{"singleAlias"}
Expect(err).NotTo(HaveOccurred())
Expect(res[0].Aliases).To(Equal(expectedAliasResult))
Expand Down Expand Up @@ -99,7 +99,7 @@ var _ = Describe("Marshalling data", func() {
"description": "A super important component. A structurally integral component tbh.",
}
entries := []source.Entry{sourceEntry}
res, err := MarshalEntries(ctx, catalogTypeOutput, entries, logger)
res, err := MarshalEntries(ctx, logger, catalogTypeOutput, entries)
Expect(err).NotTo(HaveOccurred())
Expect(res[0].AttributeValues).To(BeEmpty())
})
Expand Down

0 comments on commit 010950b

Please sign in to comment.