Skip to content

Commit

Permalink
Ensure consistency of namedHydrateFunc usage. Closes #651
Browse files Browse the repository at this point in the history
  • Loading branch information
kaidaguerre authored Sep 14, 2023
1 parent 6429d97 commit f187534
Show file tree
Hide file tree
Showing 11 changed files with 71 additions and 72 deletions.
6 changes: 3 additions & 3 deletions plugin/hydrate_call.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
type hydrateCall struct {
namedHydrateFunc
// the dependencies expressed using function name
Depends []string
Depends []namedHydrateFunc
Config *HydrateConfig

queryData *QueryData
Expand All @@ -31,7 +31,7 @@ func newHydrateCall(config *HydrateConfig, d *QueryData) (*hydrateCall, error) {
res.namedHydrateFunc = newNamedHydrateFunc(config.Func)

for _, f := range config.Depends {
res.Depends = append(res.Depends, helpers.GetFunctionName(f))
res.Depends = append(res.Depends, newNamedHydrateFunc(f))
}

return res, nil
Expand Down Expand Up @@ -75,7 +75,7 @@ func (h *hydrateCall) initialiseRateLimiter() error {
func (h *hydrateCall) canStart(rowData *rowData) bool {
// check whether all hydrate functions we depend on have saved their results
for _, dep := range h.Depends {
if !helpers.StringSliceContains(rowData.getHydrateKeys(), dep) {
if !helpers.StringSliceContains(rowData.getHydrateKeys(), dep.Name) {
return false
}
}
Expand Down
17 changes: 11 additions & 6 deletions plugin/hydrate_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"log"
"strings"

"github.com/turbot/go-kit/helpers"
"github.com/turbot/steampipe-plugin-sdk/v5/rate_limiter"
)

Expand Down Expand Up @@ -114,19 +113,21 @@ type HydrateConfig struct {

// Deprecated: use IgnoreConfig
ShouldIgnoreError ErrorPredicate

namedFunc namedHydrateFunc
}

func (c *HydrateConfig) String() string {
var dependsStrings = make([]string, len(c.Depends))
for i, dep := range c.Depends {
dependsStrings[i] = helpers.GetFunctionName(dep)
dependsStrings[i] = newNamedHydrateFunc(dep).Name
}
str := fmt.Sprintf(`Func: %s
RetryConfig: %s
IgnoreConfig: %s
Depends: %s
ScopeValues: %s`,
helpers.GetFunctionName(c.Func),
c.namedFunc.Name,
c.RetryConfig,
c.IgnoreConfig,
strings.Join(dependsStrings, ","),
Expand All @@ -136,7 +137,9 @@ ScopeValues: %s`,
}

func (c *HydrateConfig) initialise(table *Table) {
log.Printf("[TRACE] HydrateConfig.initialise func %s, table %s", helpers.GetFunctionName(c.Func), table.Name)
c.namedFunc = newNamedHydrateFunc(c.Func)

log.Printf("[TRACE] HydrateConfig.initialise func %s, table %s", c.namedFunc.Name, table.Name)

// create RetryConfig if needed
if c.RetryConfig == nil {
Expand All @@ -159,8 +162,10 @@ func (c *HydrateConfig) initialise(table *Table) {
}

// default ignore and retry configs to table defaults
c.RetryConfig.DefaultTo(table.DefaultRetryConfig)
c.IgnoreConfig.DefaultTo(table.DefaultIgnoreConfig)
if table != nil {
c.RetryConfig.DefaultTo(table.DefaultRetryConfig)
c.IgnoreConfig.DefaultTo(table.DefaultIgnoreConfig)
}

log.Printf("[TRACE] HydrateConfig.initialise complete: RetryConfig: %s, IgnoreConfig: %s", c.RetryConfig.String(), c.IgnoreConfig.String())
}
Expand Down
7 changes: 3 additions & 4 deletions plugin/list_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package plugin
import (
"fmt"
"github.com/gertd/go-pluralize"
"github.com/turbot/go-kit/helpers"
"log"
)

Expand Down Expand Up @@ -108,9 +107,9 @@ func (c *ListConfig) Validate(table *Table) []string {
}

// ensure that if there is an explicit hydrate config for the list hydrate, it does not declare dependencies
listHydrateName := helpers.GetFunctionName(table.List.Hydrate)
listHydrateName := table.List.namedHydrateFunc.Name
for _, h := range table.HydrateConfig {
if helpers.GetFunctionName(h.Func) == listHydrateName {
if h.namedFunc.Name == listHydrateName {
if len(h.Depends) > 0 {
validationErrors = append(validationErrors, fmt.Sprintf("table '%s' List hydrate function '%s' defines dependencies in its `HydrateConfig`", table.Name, listHydrateName))
}
Expand All @@ -119,7 +118,7 @@ func (c *ListConfig) Validate(table *Table) []string {
}
// ensure there is no hydrate dependency declared for the list hydrate
for _, h := range table.HydrateDependencies {
if helpers.GetFunctionName(h.Func) == listHydrateName {
if newNamedHydrateFunc(h.Func).Name == listHydrateName {
numDeps := len(h.Depends)
validationErrors = append(validationErrors, fmt.Sprintf("table '%s' List hydrate function '%s' has %d %s - List hydrate functions cannot have dependencies",
table.Name,
Expand Down
4 changes: 3 additions & 1 deletion plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,9 @@ func (p *Plugin) buildHydrateConfigMap() {
// as we are converting into a pointer, we cannot use the array value direct from the range as
// this was causing incorrect values - go must be reusing memory addresses for successive items
h := &p.HydrateConfig[i]
funcName := helpers.GetFunctionName(h.Func)
// initialise before adding to map
h.initialise(nil)
funcName := h.namedFunc.Name
p.hydrateConfigMap[funcName] = h
}
}
Expand Down
23 changes: 12 additions & 11 deletions plugin/query_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,11 +393,10 @@ func (d *QueryData) populateRequiredHydrateCalls() {

// what is the name of the fetch call (i.e. the get/list call)
fetchFunc := t.getFetchFunc(fetchType)
fetchCallName := helpers.GetFunctionName(fetchFunc)

// initialise hydrateColumnMap
d.hydrateColumnMap = make(map[string][]string)
requiredCallBuilder := newRequiredHydrateCallBuilder(d, fetchCallName)
requiredCallBuilder := newRequiredHydrateCallBuilder(d, fetchFunc.Name)

// populate a map keyed by function name to ensure we only store each hydrate function once
for _, column := range t.Columns {
Expand All @@ -413,14 +412,16 @@ func (d *QueryData) populateRequiredHydrateCalls() {
// so there is NO hydrate call registered for the column
// the column is provided by the fetch call
// do not add to map of hydrate functions as the fetch call will always be called
hydrateFunc = fetchFunc
hydrateName = fetchCallName
hydrateFunc = fetchFunc.Func
hydrateName = fetchFunc.Name
} else {
// there is a hydrate call registered
hydrateName = helpers.GetFunctionName(hydrateFunc)
namedFunc := newNamedHydrateFunc(hydrateFunc)
hydrateName = namedFunc.Name

// if this column was requested in query, add the hydrate call to required calls
if helpers.StringSliceContains(colsUsed, column.Name) {
requiredCallBuilder.Add(hydrateFunc, d.connectionCallId)
requiredCallBuilder.Add(namedFunc, d.connectionCallId)
}
}

Expand All @@ -436,7 +437,7 @@ func (d *QueryData) populateRequiredHydrateCalls() {
// build list of all columns returned by the fetch call and required hydrate calls
func (d *QueryData) populateColumns() {
// add columns returned by fetch call
fetchName := helpers.GetFunctionName(d.Table.getFetchFunc(d.FetchType))
fetchName := d.Table.getFetchFunc(d.FetchType).Name
d.addColumnsForHydrate(fetchName)

// add columns returned by required hydrate calls
Expand Down Expand Up @@ -536,13 +537,13 @@ func (d *QueryData) verifyCallerIsListCall(callingFunction string) bool {
if d.Table.List == nil {
return false
}
listFunction := helpers.GetFunctionName(d.Table.List.Hydrate)
listParentFunction := helpers.GetFunctionName(d.Table.List.ParentHydrate)
listFunction := d.Table.List.namedHydrateFunc.Name
listParentFunction := d.Table.List.namedParentHydrateFunc.Name
if callingFunction != listFunction && callingFunction != listParentFunction {
// if the calling function is NOT one of the other registered hydrate functions,
//it must be an anonymous function so let it go
for _, c := range d.Table.Columns {
if c.Hydrate != nil && helpers.GetFunctionName(c.Hydrate) == callingFunction {
if c.Hydrate != nil && newNamedHydrateFunc(c.Hydrate).Name == callingFunction {
return false
}
}
Expand Down Expand Up @@ -650,7 +651,7 @@ func (d *QueryData) streamLeafListItem(ctx context.Context, items ...interface{}
// set the parent item on the row data
rd.parentItem = d.parentItem
// NOTE: add the item as the hydrate data for the list call
rd.set(helpers.GetFunctionName(d.Table.List.Hydrate), item)
rd.set(d.Table.List.namedHydrateFunc.Name, item)

d.rowDataChan <- rd
}
Expand Down
15 changes: 7 additions & 8 deletions plugin/query_data_rate_limiters.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package plugin

import (
"context"
"github.com/turbot/go-kit/helpers"
"github.com/turbot/steampipe-plugin-sdk/v5/grpc"
"github.com/turbot/steampipe-plugin-sdk/v5/plugin/quals"
"github.com/turbot/steampipe-plugin-sdk/v5/rate_limiter"
Expand Down Expand Up @@ -97,7 +96,7 @@ func (d *QueryData) resolveGetRateLimiters() error {
// NOTE: RateLimit cannot be nil as it is initialized to an empty struct if needed
getLimiter, err := d.plugin.getHydrateCallRateLimiter(d.Table.Get.Tags, d)
if err != nil {
log.Printf("[WARN] get call %s getHydrateCallRateLimiter failed: %s (%s)", helpers.GetFunctionName(d.Table.Get.Hydrate), err.Error(), d.connectionCallId)
log.Printf("[WARN] get call %s getHydrateCallRateLimiter failed: %s (%s)", d.Table.Get.named.Name, err.Error(), d.connectionCallId)
return err
}

Expand All @@ -112,7 +111,7 @@ func (d *QueryData) resolveParentChildRateLimiters() error {
// resolve the parent hydrate rate limiter
parentRateLimiter, err := d.plugin.getHydrateCallRateLimiter(d.Table.List.ParentTags, d)
if err != nil {
log.Printf("[WARN] resolveParentChildRateLimiters: %s: getHydrateCallRateLimiter failed: %s (%s)", helpers.GetFunctionName(d.Table.List.ParentHydrate), err.Error(), d.connectionCallId)
log.Printf("[WARN] resolveParentChildRateLimiters: %s: getHydrateCallRateLimiter failed: %s (%s)", d.Table.List.namedParentHydrateFunc.Name, err.Error(), d.connectionCallId)
return err
}
// assign the parent rate limiter to d.fetchLimiters
Expand All @@ -121,7 +120,7 @@ func (d *QueryData) resolveParentChildRateLimiters() error {
// resolve the child hydrate rate limiter
childRateLimiter, err := d.plugin.getHydrateCallRateLimiter(d.Table.List.Tags, d)
if err != nil {
log.Printf("[WARN] resolveParentChildRateLimiters: %s: getHydrateCallRateLimiter failed: %s (%s)", helpers.GetFunctionName(d.Table.List.Hydrate), err.Error(), d.connectionCallId)
log.Printf("[WARN] resolveParentChildRateLimiters: %s: getHydrateCallRateLimiter failed: %s (%s)", d.Table.List.namedHydrateFunc.Name, err.Error(), d.connectionCallId)
return err
}
d.fetchLimiters.childListRateLimiter = childRateLimiter
Expand All @@ -133,7 +132,7 @@ func (d *QueryData) resolveListRateLimiters() error {
// NOTE: RateLimit cannot be nil as it is initialized to an empty struct if needed
listLimiter, err := d.plugin.getHydrateCallRateLimiter(d.Table.List.Tags, d)
if err != nil {
log.Printf("[WARN] get call %s getHydrateCallRateLimiter failed: %s (%s)", helpers.GetFunctionName(d.Table.Get.Hydrate), err.Error(), d.connectionCallId)
log.Printf("[WARN] get call %s getHydrateCallRateLimiter failed: %s (%s)", d.Table.Get.named.Name, err.Error(), d.connectionCallId)
return err
}
d.fetchLimiters.rateLimiter = listLimiter
Expand All @@ -142,7 +141,7 @@ func (d *QueryData) resolveListRateLimiters() error {

func (d *QueryData) setListLimiterMetadata(fetchDelay time.Duration) {
fetchMetadata := &hydrateMetadata{
FuncName: helpers.GetFunctionName(d.listHydrate),
FuncName: d.listHydrate.Name,
RateLimiters: d.fetchLimiters.rateLimiter.LimiterNames(),
ScopeValues: d.fetchLimiters.rateLimiter.ScopeValues,
DelayMs: fetchDelay.Milliseconds(),
Expand All @@ -153,7 +152,7 @@ func (d *QueryData) setListLimiterMetadata(fetchDelay time.Duration) {
} else {
d.fetchMetadata = &hydrateMetadata{
Type: string(fetchTypeList),
FuncName: helpers.GetFunctionName(d.childHydrate),
FuncName: d.childHydrate.Name,
RateLimiters: d.fetchLimiters.childListRateLimiter.LimiterNames(),
ScopeValues: d.fetchLimiters.childListRateLimiter.ScopeValues,
}
Expand All @@ -165,7 +164,7 @@ func (d *QueryData) setListLimiterMetadata(fetchDelay time.Duration) {
func (d *QueryData) setGetLimiterMetadata(fetchDelay time.Duration) {
d.fetchMetadata = &hydrateMetadata{
Type: string(fetchTypeGet),
FuncName: helpers.GetFunctionName(d.Table.Get.Hydrate),
FuncName: d.Table.Get.named.Name,
RateLimiters: d.fetchLimiters.rateLimiter.LimiterNames(),
ScopeValues: d.fetchLimiters.rateLimiter.ScopeValues,
DelayMs: fetchDelay.Milliseconds(),
Expand Down
10 changes: 5 additions & 5 deletions plugin/required_hydrate_calls.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package plugin

import (
"github.com/turbot/go-kit/helpers"
"log"
)

Expand All @@ -20,8 +19,8 @@ func newRequiredHydrateCallBuilder(d *QueryData, fetchCallName string) *required
}
}

func (c requiredHydrateCallBuilder) Add(hydrateFunc HydrateFunc, callId string) error {
hydrateName := helpers.GetFunctionName(hydrateFunc)
func (c requiredHydrateCallBuilder) Add(hydrateFunc namedHydrateFunc, callId string) error {
hydrateName := hydrateFunc.Name

// if the resolved hydrate call is NOT the same as the fetch call, add to the map of hydrate functions to call
if hydrateName != c.fetchCallName {
Expand All @@ -41,8 +40,9 @@ func (c requiredHydrateCallBuilder) Add(hydrateFunc HydrateFunc, callId string)

// now add dependencies (we have already checked for circular dependencies so recursion is fine
for _, dep := range config.Depends {
if err := c.Add(dep, callId); err != nil {
log.Printf("[WARN] failed to add a hydrate call for %s, which is a dependency of %s: %s", helpers.GetFunctionName(dep), hydrateName, err.Error())
namedDep := newNamedHydrateFunc(dep)
if err := c.Add(namedDep, callId); err != nil {
log.Printf("[WARN] failed to add a hydrate call for %s, which is a dependency of %s: %s", namedDep.Name, hydrateName, err.Error())
return err
}
}
Expand Down
30 changes: 10 additions & 20 deletions plugin/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"github.com/turbot/steampipe-plugin-sdk/v5/rate_limiter"
"log"

"github.com/turbot/go-kit/helpers"
"github.com/turbot/steampipe-plugin-sdk/v5/plugin/transform"
)

Expand Down Expand Up @@ -132,7 +131,7 @@ func (t *Table) initialise(p *Plugin) {
// declared for specific columns which do not have config defined
// build a map of all hydrate functions, with empty config if needed
// NOTE: this map also includes information from the legacy HydrateDependencies property
t.initialiseHydrateConfigs()
t.buildHydrateConfigMap()

t.setColumnNameMap()

Expand All @@ -148,17 +147,6 @@ func (t *Table) setColumnNameMap() {
}
}

// build map of all hydrate configs, and initialise them
func (t *Table) initialiseHydrateConfigs() {
// first build a map of all hydrate functions
t.buildHydrateConfigMap()

// initialise all hydrate configs in map
for _, h := range t.hydrateConfigMap {
h.initialise(t)
}
}

// build map of all hydrate configs, including those specified in the legacy HydrateDependencies,
// and those mentioned only in column config
func (t *Table) buildHydrateConfigMap() {
Expand All @@ -167,12 +155,14 @@ func (t *Table) buildHydrateConfigMap() {
// as we are converting into a pointer, we cannot use the array value direct from the range as
// this was causing incorrect values - go must be reusing memory addresses for successive items
h := &t.HydrateConfig[i]
funcName := helpers.GetFunctionName(h.Func)
t.hydrateConfigMap[funcName] = h

// initialise before adding to map
h.initialise(t)
t.hydrateConfigMap[h.namedFunc.Name] = h
}
// add in hydrate config for all hydrate dependencies declared using legacy property HydrateDependencies
for _, d := range t.HydrateDependencies {
hydrateName := helpers.GetFunctionName(d.Func)
hydrateName := newNamedHydrateFunc(d.Func).Name
// if there is already a hydrate config, do nothing here
// (this is a validation error that will be picked up by the validation check later)
if _, ok := t.hydrateConfigMap[hydrateName]; !ok {
Expand All @@ -181,7 +171,7 @@ func (t *Table) buildHydrateConfigMap() {
}
// NOTE: the get config may be used as a column hydrate function so add this into the map
if get := t.Get; get != nil {
hydrateName := helpers.GetFunctionName(get.Hydrate)
hydrateName := get.named.Name
t.hydrateConfigMap[hydrateName] = &HydrateConfig{
Func: get.Hydrate,
IgnoreConfig: get.IgnoreConfig,
Expand All @@ -208,9 +198,9 @@ func (t *Table) buildHydrateConfigMap() {
}
}

func (t *Table) getFetchFunc(fetchType fetchType) HydrateFunc {
func (t *Table) getFetchFunc(fetchType fetchType) namedHydrateFunc {
if fetchType == fetchTypeList {
return t.List.Hydrate
return *t.List.namedHydrateFunc
}
return t.Get.Hydrate
return t.Get.named
}
6 changes: 3 additions & 3 deletions plugin/table_fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func (t *Table) executeGetCall(ctx context.Context, queryData *QueryData) (err e
// we can now close the item chan
queryData.fetchComplete(ctx)
if r := recover(); r != nil {
err = status.Error(codes.Internal, fmt.Sprintf("get call %s failed with panic %v", helpers.GetFunctionName(t.Get.Hydrate), r))
err = status.Error(codes.Internal, fmt.Sprintf("get call %s failed with panic %v", t.Get.named.Name, r))
}
}()

Expand Down Expand Up @@ -179,7 +179,7 @@ func (t *Table) doGetForQualValues(ctx context.Context, queryData *QueryData, ke
// execute a get call for a single key column qual value
// if a matrix is defined, call for every matrix item
func (t *Table) doGet(ctx context.Context, queryData *QueryData) (err error) {
hydrateKey := helpers.GetFunctionName(t.Get.Hydrate)
hydrateKey := t.Get.named.Name
defer func() {
if p := recover(); p != nil {
err = status.Error(codes.Internal, fmt.Sprintf("table '%s': Get hydrate call %s failed with panic %v", t.Name, hydrateKey, p))
Expand Down Expand Up @@ -358,7 +358,7 @@ func (t *Table) executeListCall(ctx context.Context, queryData *QueryData) {
defer log.Printf("[TRACE] executeListCall COMPLETE (%s)", queryData.connectionCallId)
defer func() {
if r := recover(); r != nil {
queryData.streamError(status.Error(codes.Internal, fmt.Sprintf("list call %s failed with panic %v", helpers.GetFunctionName(t.List.Hydrate), r)))
queryData.streamError(status.Error(codes.Internal, fmt.Sprintf("list call %s failed with panic %v", t.List.namedHydrateFunc.Name, r)))
}
// list call will return when it has streamed all items so close rowDataChan
queryData.fetchComplete(ctx)
Expand Down
Loading

0 comments on commit f187534

Please sign in to comment.