From 1c33ab4ffca38fe81648bfc8fd8fc218df3d64a1 Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Mon, 30 Jul 2018 16:25:43 -0500 Subject: [PATCH] Rework assets so whether or not they are managed as sets is configured at the type level --- cmd/flowrunner/testdata/default.json | 8 ++-- cmd/flowrunner/testdata/flows/brochure.json | 2 +- .../testdata/flows/dynamic_groups.json | 4 +- cmd/flowrunner/testdata/flows/resthook.json | 2 +- cmd/flowserver/server_test.go | 8 ++-- cmd/flowserver/static/start.json | 10 ++--- flows/assets/assets_test.go | 6 +-- flows/assets/cache.go | 35 ++------------- flows/assets/server.go | 17 ++++---- flows/assets/session.go | 14 +++--- flows/assets/types.go | 43 +++++++++++++++++++ .../definition/testdata/flow_validation.json | 8 ++-- flows/engine/testdata/timeout_test.json | 6 +-- test/session.go | 12 +++--- 14 files changed, 94 insertions(+), 81 deletions(-) create mode 100644 flows/assets/types.go diff --git a/cmd/flowrunner/testdata/default.json b/cmd/flowrunner/testdata/default.json index 9c479bc1c..e475519d5 100644 --- a/cmd/flowrunner/testdata/default.json +++ b/cmd/flowrunner/testdata/default.json @@ -1,6 +1,6 @@ [ { - "type": "field_set", + "type": "field", "url": "http://testserver/assets/field", "content": [ { @@ -31,7 +31,7 @@ ] }, { - "type": "group_set", + "type": "group", "url": "http://testserver/assets/group", "content": [ { @@ -50,7 +50,7 @@ ] }, { - "type": "channel_set", + "type": "channel", "url": "http://testserver/assets/channel", "content": [ { @@ -77,7 +77,7 @@ ] }, { - "type": "label_set", + "type": "label", "url": "http://testserver/assets/label", "content": [ { diff --git a/cmd/flowrunner/testdata/flows/brochure.json b/cmd/flowrunner/testdata/flows/brochure.json index d2e7242fc..019671c52 100644 --- a/cmd/flowrunner/testdata/flows/brochure.json +++ b/cmd/flowrunner/testdata/flows/brochure.json @@ -90,7 +90,7 @@ } }, { - "type": "group_set", + "type": "group", "url": "http://testserver/assets/group", "content": [ { diff --git a/cmd/flowrunner/testdata/flows/dynamic_groups.json b/cmd/flowrunner/testdata/flows/dynamic_groups.json index eb0bcf489..669232eeb 100644 --- a/cmd/flowrunner/testdata/flows/dynamic_groups.json +++ b/cmd/flowrunner/testdata/flows/dynamic_groups.json @@ -64,7 +64,7 @@ } }, { - "type": "field_set", + "type": "field", "url": "http://testserver/assets/field", "content": [ { @@ -90,7 +90,7 @@ ] }, { - "type": "group_set", + "type": "group", "url": "http://testserver/assets/group", "content": [ { diff --git a/cmd/flowrunner/testdata/flows/resthook.json b/cmd/flowrunner/testdata/flows/resthook.json index da5062dcf..8c8adfd70 100644 --- a/cmd/flowrunner/testdata/flows/resthook.json +++ b/cmd/flowrunner/testdata/flows/resthook.json @@ -80,7 +80,7 @@ } }, { - "type": "resthook_set", + "type": "resthook", "url": "http://testserver/assets/resthook", "content": [ { diff --git a/cmd/flowserver/server_test.go b/cmd/flowserver/server_test.go index afd28e124..a4758cd46 100644 --- a/cmd/flowserver/server_test.go +++ b/cmd/flowserver/server_test.go @@ -81,7 +81,7 @@ var testFlowMissingGroupAssets = `[ } }, { - "type": "group_set", + "type": "group", "url": "http://testserver/assets/group", "content": [ { @@ -199,9 +199,9 @@ var startRequestTemplate = `{ "assets": %s, "asset_server": { "type_urls": { - "flow": "http://testserver/assets/flow/{uuid}/", - "field_set": "http://testserver/assets/field/", - "group_set": "http://testserver/assets/group/" + "flow": "http://testserver/assets/flow/", + "field": "http://testserver/assets/field/", + "group": "http://testserver/assets/group/" } }, "trigger": { diff --git a/cmd/flowserver/static/start.json b/cmd/flowserver/static/start.json index 382c9761e..20e37796e 100644 --- a/cmd/flowserver/static/start.json +++ b/cmd/flowserver/static/start.json @@ -22,7 +22,7 @@ } }, { - "type": "field_set", + "type": "field", "url": "http://testserver/assets/field", "content": [ { @@ -33,7 +33,7 @@ ] }, { - "type": "channel_set", + "type": "channel", "url": "http://testserver/assets/channel", "content": [ { @@ -53,9 +53,9 @@ ], "asset_server": { "type_urls": { - "flow": "http://testserver/assets/flow/{uuid}/", - "channel_set": "http://testserver/assets/channel/", - "field_set": "http://testserver/assets/field/" + "flow": "http://testserver/assets/flow/", + "channel": "http://testserver/assets/channel/", + "field": "http://testserver/assets/field/" } }, "trigger": { diff --git a/flows/assets/assets_test.go b/flows/assets/assets_test.go index e0cabfc39..1f036ca43 100644 --- a/flows/assets/assets_test.go +++ b/flows/assets/assets_test.go @@ -21,7 +21,7 @@ func TestAssetCache(t *testing.T) { asset, err := cache.GetAsset(server, assetType("pizza"), "") assert.EqualError(t, err, "asset type 'pizza' not supported by asset server") - asset, err = cache.GetAsset(server, assetTypeLabelSet, "") + asset, err = cache.GetAsset(server, assetTypeLabel, "") assert.NoError(t, err) assert.Equal(t, server.MockedRequests(), []string{"http://testserver/assets/label/"}) @@ -30,7 +30,7 @@ func TestAssetCache(t *testing.T) { assert.NotNil(t, labelSet.FindByName("Spam")) // check that we can refetch without making another server request - asset, err = cache.GetAsset(server, assetTypeLabelSet, "") + asset, err = cache.GetAsset(server, assetTypeLabel, "") assert.NoError(t, err) assert.Equal(t, server.MockedRequests(), []string{"http://testserver/assets/label/"}) } @@ -50,7 +50,7 @@ func TestAssetServer(t *testing.T) { url, err := server.getAssetURL(assetType("pizza"), "") assert.EqualError(t, err, "asset type 'pizza' not supported by asset server") - url, err = server.getAssetURL(assetTypeGroupSet, "") + url, err = server.getAssetURL(assetTypeGroup, "") assert.NoError(t, err) assert.Equal(t, "http://testserver/assets/group/", url) diff --git a/flows/assets/cache.go b/flows/assets/cache.go index a9cb4ab51..581ff35a4 100644 --- a/flows/assets/cache.go +++ b/flows/assets/cache.go @@ -7,25 +7,11 @@ import ( "sync" "time" - "github.com/nyaruka/goflow/flows" - "github.com/nyaruka/goflow/flows/definition" "github.com/nyaruka/goflow/utils" "github.com/karlseguin/ccache" ) -type assetType string - -const ( - assetTypeChannelSet assetType = "channel_set" - assetTypeFieldSet assetType = "field_set" - assetTypeFlow assetType = "flow" - assetTypeGroupSet assetType = "group_set" - assetTypeLabelSet assetType = "label_set" - assetTypeLocationHierarchySet assetType = "location_hierarchy_set" - assetTypeResthookSet assetType = "resthook_set" -) - // AssetCache fetches and caches assets for the engine type AssetCache struct { cache *ccache.Cache @@ -132,25 +118,10 @@ func (c *AssetCache) Include(data json.RawMessage) error { // reads an asset from the given raw JSON data func readAsset(data json.RawMessage, itemType assetType) (interface{}, error) { - var assetReader func(data json.RawMessage) (interface{}, error) - - if itemType == assetTypeLocationHierarchySet { - assetReader = func(data json.RawMessage) (interface{}, error) { return flows.ReadLocationHierarchySet(data) } - } else if itemType == assetTypeChannelSet { - assetReader = func(data json.RawMessage) (interface{}, error) { return flows.ReadChannelSet(data) } - } else if itemType == assetTypeFieldSet { - assetReader = func(data json.RawMessage) (interface{}, error) { return flows.ReadFieldSet(data) } - } else if itemType == assetTypeFlow { - assetReader = func(data json.RawMessage) (interface{}, error) { return definition.ReadFlow(data) } - } else if itemType == assetTypeGroupSet { - assetReader = func(data json.RawMessage) (interface{}, error) { return flows.ReadGroupSet(data) } - } else if itemType == assetTypeLabelSet { - assetReader = func(data json.RawMessage) (interface{}, error) { return flows.ReadLabelSet(data) } - } else if itemType == assetTypeResthookSet { - assetReader = func(data json.RawMessage) (interface{}, error) { return flows.ReadResthookSet(data) } - } else { + cfg := typeConfigs[itemType] + if cfg == nil { return nil, fmt.Errorf("unsupported asset type: %s", itemType) } - return assetReader(data) + return cfg.reader(data) } diff --git a/flows/assets/server.go b/flows/assets/server.go index 1b802fcee..734542895 100644 --- a/flows/assets/server.go +++ b/flows/assets/server.go @@ -5,7 +5,6 @@ import ( "fmt" "io/ioutil" "net/http" - "strings" "github.com/nyaruka/goflow/utils" @@ -58,7 +57,7 @@ func (s *assetServer) getAssetURL(itemType assetType, itemUUID string) (string, } if itemUUID != "" { - url = strings.Replace(url, "{uuid}", itemUUID, -1) + url = fmt.Sprintf("%s%s/", url, itemUUID) } return url, nil @@ -112,13 +111,13 @@ func NewMockAssetServer() *MockAssetServer { return &MockAssetServer{ assetServer: assetServer{ typeURLs: map[assetType]string{ - assetTypeChannelSet: "http://testserver/assets/channel/", - assetTypeFieldSet: "http://testserver/assets/field/", - assetTypeFlow: "http://testserver/assets/flow/{uuid}/", - assetTypeGroupSet: "http://testserver/assets/group/", - assetTypeLabelSet: "http://testserver/assets/label/", - assetTypeLocationHierarchySet: "http://testserver/assets/location_hierarchy/", - assetTypeResthookSet: "http://testserver/assets/resthook/", + assetTypeChannel: "http://testserver/assets/channel/", + assetTypeField: "http://testserver/assets/field/", + assetTypeFlow: "http://testserver/assets/flow/", + assetTypeGroup: "http://testserver/assets/group/", + assetTypeLabel: "http://testserver/assets/label/", + assetTypeLocationHierarchy: "http://testserver/assets/location_hierarchy/", + assetTypeResthook: "http://testserver/assets/resthook/", }, }, mockResponses: map[string]json.RawMessage{}, diff --git a/flows/assets/session.go b/flows/assets/session.go index 203a06969..287619ba6 100644 --- a/flows/assets/session.go +++ b/flows/assets/session.go @@ -21,12 +21,12 @@ func NewSessionAssets(cache *AssetCache, server AssetServer) flows.SessionAssets // HasLocations returns whether locations are supported as an asset item type func (s *sessionAssets) HasLocations() bool { - return s.server.isTypeSupported(assetTypeLocationHierarchySet) + return s.server.isTypeSupported(assetTypeLocationHierarchy) } // GetLocationHierarchy gets the location hierarchy asset for the session func (s *sessionAssets) GetLocationHierarchySet() (*flows.LocationHierarchySet, error) { - asset, err := s.cache.GetAsset(s.server, assetTypeLocationHierarchySet, "") + asset, err := s.cache.GetAsset(s.server, assetTypeLocationHierarchy, "") if err != nil { return nil, err } @@ -52,7 +52,7 @@ func (s *sessionAssets) GetChannel(uuid flows.ChannelUUID) (flows.Channel, error // GetChannelSet gets the set of all channels asset for the session func (s *sessionAssets) GetChannelSet() (*flows.ChannelSet, error) { - asset, err := s.cache.GetAsset(s.server, assetTypeChannelSet, "") + asset, err := s.cache.GetAsset(s.server, assetTypeChannel, "") if err != nil { return nil, err } @@ -78,7 +78,7 @@ func (s *sessionAssets) GetField(key string) (*flows.Field, error) { // GetFieldSet gets the set of all fields asset for the session func (s *sessionAssets) GetFieldSet() (*flows.FieldSet, error) { - asset, err := s.cache.GetAsset(s.server, assetTypeFieldSet, "") + asset, err := s.cache.GetAsset(s.server, assetTypeField, "") if err != nil { return nil, err } @@ -117,7 +117,7 @@ func (s *sessionAssets) GetGroup(uuid flows.GroupUUID) (*flows.Group, error) { // GetGroupSet gets the set of all groups asset for the session func (s *sessionAssets) GetGroupSet() (*flows.GroupSet, error) { - asset, err := s.cache.GetAsset(s.server, assetTypeGroupSet, "") + asset, err := s.cache.GetAsset(s.server, assetTypeGroup, "") if err != nil { return nil, err } @@ -142,7 +142,7 @@ func (s *sessionAssets) GetLabel(uuid flows.LabelUUID) (*flows.Label, error) { } func (s *sessionAssets) GetLabelSet() (*flows.LabelSet, error) { - asset, err := s.cache.GetAsset(s.server, assetTypeLabelSet, "") + asset, err := s.cache.GetAsset(s.server, assetTypeLabel, "") if err != nil { return nil, err } @@ -154,7 +154,7 @@ func (s *sessionAssets) GetLabelSet() (*flows.LabelSet, error) { } func (s *sessionAssets) GetResthookSet() (*flows.ResthookSet, error) { - asset, err := s.cache.GetAsset(s.server, assetTypeResthookSet, "") + asset, err := s.cache.GetAsset(s.server, assetTypeResthook, "") if err != nil { return nil, err } diff --git a/flows/assets/types.go b/flows/assets/types.go new file mode 100644 index 000000000..cd8c1008f --- /dev/null +++ b/flows/assets/types.go @@ -0,0 +1,43 @@ +package assets + +import ( + "encoding/json" + + "github.com/nyaruka/goflow/flows" + "github.com/nyaruka/goflow/flows/definition" +) + +type assetType string + +const ( + assetTypeChannel assetType = "channel" + assetTypeField assetType = "field" + assetTypeFlow assetType = "flow" + assetTypeGroup assetType = "group" + assetTypeLabel assetType = "label" + assetTypeLocationHierarchy assetType = "location_hierarchy" + assetTypeResthook assetType = "resthook" +) + +type assetReader func(data json.RawMessage) (interface{}, error) + +type assetTypeConfig struct { + manageAsSet bool + reader assetReader +} + +var typeConfigs = map[assetType]*assetTypeConfig{} + +func registerAssetType(name assetType, manageAsSet bool, reader assetReader) { + typeConfigs[name] = &assetTypeConfig{manageAsSet: manageAsSet, reader: reader} +} + +func init() { + registerAssetType(assetTypeChannel, true, func(data json.RawMessage) (interface{}, error) { return flows.ReadChannelSet(data) }) + registerAssetType(assetTypeField, true, func(data json.RawMessage) (interface{}, error) { return flows.ReadFieldSet(data) }) + registerAssetType(assetTypeFlow, false, func(data json.RawMessage) (interface{}, error) { return definition.ReadFlow(data) }) + registerAssetType(assetTypeGroup, true, func(data json.RawMessage) (interface{}, error) { return flows.ReadGroupSet(data) }) + registerAssetType(assetTypeLabel, true, func(data json.RawMessage) (interface{}, error) { return flows.ReadLabelSet(data) }) + registerAssetType(assetTypeLocationHierarchy, true, func(data json.RawMessage) (interface{}, error) { return flows.ReadLocationHierarchySet(data) }) + registerAssetType(assetTypeResthook, true, func(data json.RawMessage) (interface{}, error) { return flows.ReadResthookSet(data) }) +} diff --git a/flows/definition/testdata/flow_validation.json b/flows/definition/testdata/flow_validation.json index 7983cc954..d2cfcaf40 100644 --- a/flows/definition/testdata/flow_validation.json +++ b/flows/definition/testdata/flow_validation.json @@ -98,7 +98,7 @@ } }, { - "type": "field_set", + "type": "field", "url": "http://testserver/assets/field", "content": [ { @@ -134,7 +134,7 @@ ] }, { - "type": "group_set", + "type": "group", "url": "http://testserver/assets/group", "content": [ { @@ -153,7 +153,7 @@ ] }, { - "type": "channel_set", + "type": "channel", "url": "http://testserver/assets/channel", "content": [ { @@ -166,7 +166,7 @@ ] }, { - "type": "label_set", + "type": "label", "url": "http://testserver/assets/label", "content": [ { diff --git a/flows/engine/testdata/timeout_test.json b/flows/engine/testdata/timeout_test.json index 9b72c148b..314e895e4 100644 --- a/flows/engine/testdata/timeout_test.json +++ b/flows/engine/testdata/timeout_test.json @@ -95,17 +95,17 @@ } }, { - "type": "group_set", + "type": "group", "url": "http://testserver/assets/group/", "content": [] }, { - "type": "field_set", + "type": "field", "url": "http://testserver/assets/field/", "content": [] }, { - "type": "channel_set", + "type": "channel", "url": "http://testserver/assets/channel/", "content": [ { diff --git a/test/session.go b/test/session.go index f59a0dbb6..167980808 100644 --- a/test/session.go +++ b/test/session.go @@ -15,7 +15,7 @@ import ( var sessionAssets = `[ { - "type": "channel_set", + "type": "channel", "url": "http://testserver/assets/channel", "content": [ { @@ -144,7 +144,7 @@ var sessionAssets = `[ } }, { - "type": "field_set", + "type": "field", "url": "http://testserver/assets/field", "content": [ {"key": "gender", "label": "Gender", "value_type": "text"}, @@ -154,7 +154,7 @@ var sessionAssets = `[ ] }, { - "type": "group_set", + "type": "group", "url": "http://testserver/assets/group", "content": [ {"uuid": "b7cf0d83-f1c9-411c-96fd-c511a4cfa86d", "name": "Testers"}, @@ -163,7 +163,7 @@ var sessionAssets = `[ ] }, { - "type": "label_set", + "type": "label", "url": "http://testserver/assets/label", "content": [ { @@ -173,7 +173,7 @@ var sessionAssets = `[ ] }, { - "type": "location_hierarchy_set", + "type": "location_hierarchy", "url": "http://testserver/assets/location_hierarchy", "content": [ { @@ -212,7 +212,7 @@ var sessionAssets = `[ ] }, { - "type": "resthook_set", + "type": "resthook", "url": "http://testserver/assets/resthook", "content": [ {