design & code style fixes after review

Author: SudoBobo

graphql/accessor_general.lua

@@ -827,7 +827,7 @@ local function process_tuple(state, tuple, opts)
 
     -- convert tuple -> object
     local obj = opts.unflatten_tuple(collection_name, tuple,
-        opts.default_unflatten_tuple, opts.is_collection_formatted)
+    { has_space_format = opts.has_space_format }, opts.default_unflatten_tuple)
 
     -- skip all items before pivot (the item pointed by offset)
     if not state.pivot_found and pivot_filter then
@@ -975,7 +975,7 @@ local function select_internal(self, collection_name, from, filter, args, extra)
         fetched_object_cnt_max = self.settings.fetched_object_cnt_max,
         collection_name = collection_name,
         unflatten_tuple = self.funcs.unflatten_tuple,
-        is_collection_formatted = self.formatted_collections[collection_name] or false,
+        has_space_format = self.has_space_format[collection_name] or false,
         default_unflatten_tuple = default_unflatten_tuple,
         pcre = args.pcre,
         resolveField = extra.resolveField,
@@ -1233,7 +1233,6 @@ function accessor_general.new(opts, funcs)
     assert(type(funcs) == 'table',
         'funcs must be a table, got ' .. type(funcs))
 
-    --todo assertions for opt.formatted_collections
     local schemas = opts.schemas
     local collections = opts.collections
     local service_fields = opts.service_fields
@@ -1288,7 +1287,7 @@ function accessor_general.new(opts, funcs)
         indexes = indexes,
         models = models,
         default_unflatten_tuple = default_unflatten_tuple,
-        formatted_collections = opts.formatted_collections,
+        has_space_format = opts.has_space_format or {},
         index_cache = index_cache,
         funcs = funcs,
         settings = {

graphql/accessor_shard.lua

@@ -84,12 +84,19 @@ end
 ---
 --- @tparam string collection_name
 --- @tparam cdata/table tuple
+--- @tparam table opts
+--- * `has_space_format` (boolean, if collection 'collection_name' generated
+--- from space:format() or not).
 --- @tparam function default unflatten action, call it in the following way:
 ---
 --- ```
 --- return default(collection_name, tuple)
 --- ```
-local function unflatten_tuple(collection_name, tuple, default)
+local function unflatten_tuple(collection_name, tuple, opts, default)
+    if opts.has_space_format then
+        return tuple:tomap({ names_only = true })
+    end
+
     return default(collection_name, tuple)
 end
 

graphql/accessor_space.lua

@@ -26,14 +26,17 @@ end
 ---
 --- @tparam string collection_name
 --- @tparam cdata/table tuple
+--- @tparam table opts
+--- * `has_space_format` (boolean, if collection 'collection_name' generated
+--- from space:format() or not).
 --- @tparam function default unflatten action, call it in the following way:
 ---
 --- ```
 --- return default(collection_name, tuple)
 --- ```
-local function unflatten_tuple(collection_name, tuple, default, is_collection_formatted)
-    if is_collection_formatted then
-        return tuple:tomap()
+local function unflatten_tuple(collection_name, tuple, opts, default)
+    if opts.has_space_format then
+        return tuple:tomap({ names_only = true })
     end
     return default(collection_name, tuple)
 end
@@ -51,8 +54,6 @@ function accessor_space.new(opts, funcs)
             'funcs values must be functions, got ' .. type(v))
     end
 
-    -- todo add some note in comments that funcs.unflatten_tuple must
-    -- manage our tuple:tomap or unflatten issue
     local res_funcs = {
         is_collection_exists = funcs.is_collection_exists or
             is_collection_exists,

graphql/config_complement.lua

@@ -1,13 +1,21 @@
+--- Config complement module provide an ability to complement user-defined config
+--- (in a simplified format) to a fully specified format.
+---
+------ Notes:
+---
+--- * Currently the module complements only connections (cfg.connections),
+-- see @{complement_connections}.
+
+local json = require('json')
 local utils = require('graphql.utils')
-local at = utils.assert_type
+local check = utils.check
+local get_spaces_formats = require('graphql.simple_config').get_spaces_formats
 
 local config_complement = {}
 
---- The function determines connection type by fully qualified connection.parts
---- and source collection space format
---- Items in parts must be sorted by destination_field to match field order
---- in connection's index
-local function determine_connection_type(connection_parts, index, source_format)
+--- The function determines connection type by connection.parts
+--- and source collection space format.
+local function determine_connection_type(connection_parts, index, source_space_format)
     local type
 
     if #connection_parts < #(index.fields) then
@@ -22,142 +30,189 @@ local function determine_connection_type(connection_parts, index, source_format)
         end
     end
 
-    -- todo still not clear how to handle nullables
+    local is_all_nullable = true
+    local is_all_not_nullable = true
+
+    for _, connection_part in pairs(connection_parts) do
+        for _,field_format in ipairs(source_space_format) do
+            if connection_part.source_field == field_format.name then
+                if field_format.is_nullable == true then
+                    is_all_not_nullable = false
+                else
+                    is_all_nullable = false
+                end
+            end
+        end
+    end
+
+    if is_all_nullable == is_all_not_nullable then
+        error('source_fields in connection_parts must be all nullable or ' ..
+            'not nullable at the same time')
+    end
+
+    if is_all_nullable then
+        type = type .. '*'
+    end
+
     return type
 end
 
---- XXX Currently if connections_parts have destination_fields that do not
---- exist in connection's index the assertion is raised. Maybe it is not a
---- desirable behavior. Consider the following case: you have working
---- tarantool_graphql, then you try to change configuration a bit and
---- tarantool_graphql crushes.
 local function sort_parts(connection_parts, index_fields)
     local sorted_parts = {}
+
+    -- check if fields in connection_parts exist in index_fields
     for _, part in pairs(connection_parts) do
         local is_found = false
         for i, index_field in ipairs(index_fields) do
             if part.destination_field == index_field then
-                sorted_parts[i] = part
                 is_found = true
             end
         end
         assert(is_found, ('part.destination_field %s was not found in ' ..
-            'connection index'):format(part.destination_field))
+            'connection index %s'):format(part.destination_field,
+                json.encode(index_fields)))
     end
-    return sorted_parts;
+
+    -- sort parts and check that sorted_parts form index prefix
+    -- (including index itself)
+    for i = 1, utils.size(connection_parts) do
+        local index_field = index_fields[i]
+        for _, part in pairs(connection_parts) do
+            if part.destination_field == index_field then
+                sorted_parts[i] = index_field
+                break
+            end
+            -- no match found
+            error(('given parts %s does not form an index or an index ' ..
+                'prefix %s'):format(json.encode(connection_parts),
+                    json.encode(index_fields)))
+        end
+    end
+    return sorted_parts
 end
 
---- The functions determines fully qualified connection's parts based on
---- connection's index and incompletely qualified connection's part given by user
---- @tparam table parts incompletely qualified connection's part given by user
---- @tparam table index connection index (cfg.indexes[collection][index_name]
+--- The functions determines fully defined connection's parts based on
+--- connection's index and partially defined connection's part given by user.
+--- @tparam table parts partially defined connection's part given by user
+--- @tparam table index connection index (cfg.indexes[collection][index_name])
 --- order of items in index.fields must be the same as in index format
 local function determine_connection_parts(parts, index)
-    at(parts, 'parts', 'nil', 'number', 'table')
+    check(parts, 'parts', 'nil', 'number', 'table')
     local result_parts = {}
 
-    -- full index case
+    -- User defined no parts of the connection. All connection's index fields
+    -- are taken as 'parts'
     if type(parts) == 'nil' then
         for i, v in ipairs(index.fields) do
             result_parts[i] = {source_field = v, destination_field = v}
         end
     end
 
-    -- index prefix case
+    -- User defined a number of fields of index which must form index prefix.
+    -- First 'number' index fields are taken as 'parts'
     if type(parts) == 'number' then
         for i = 1, parts do
             local v = index.fields[i]
             result_parts[i] = {source_field = v, destination_field = v}
         end
     end
 
-    -- fully qualified parts case
+    -- User defined parts as pairs of {source_field: foo_field,
+    -- destination_field: boo_field}. These 'parts' may correspond either to full
+    -- index or index prefix
     if type(parts) == 'table' then
-        local sorted_parts = sort_parts(parts, index.fields)
-        for i, v in ipairs(sorted_parts) do
-            result_parts[i] = {source_field = v, destination_field = v}
-        end
+        -- sorting parts is necessary to check if user defined part form an
+        -- index or an index prefix
+        result_parts = sort_parts(parts, index.fields)
     end
 
     return result_parts
 end
 
----
 --- The function complements collections' connections, described in simplified
 --- format, to connections in a fully specified format. Connection's
 --- collection determined on source_collection. Type determined on index type.
 --- Notice an example:
 ---
---- "connections" : [
----     {
----         "name": "order_connection",
----         "source_collection": "user_collection",
----         "destination_collection": "order_collection"
----         "index_name": "user_id_index",
----         "parts" : nil | number | table (destination fields can be omitted)
----     },
----    ...
---- ]
---- will produce following complement in 'user_collection' :
+---     "connections" : [
+---         {
+---             "name": "order_connection",
+---             "source_collection": "user_collection",
+---             "destination_collection": "order_collection"
+---             "index_name": "user_id_index",
+---             "parts" : nil | number | table (destination fields can be omitted)
+---                 in case of 'table' expected format is:
+---                 "parts": [
+---                     {"source_field": "user_id", "destination_field": "user_id"},
+---                     ...
+---                 ]
+---         },
+---         ...
+---     ]
 ---
---- "user_collection": {
----    "schema_name": "user",
----    "connections": [
----    {
----        "type": "1:N",
----        "name": "order_connection",
----        "destination_collection":  "order_collection",
----        "parts": [
----        { "source_field": "user_id", "destination_field": "user_id" }
----        ],
----        "index_name": "user_id_index"
----    },
+--- will produce following complement in 'user_collection' :
 ---
----    ]
---- }
+---     "user_collection": {
+---         "schema_name": "user",
+---         "connections": [
+---             {
+---                 "type": "1:N",
+---                 "name": "order_connection",
+---                 "destination_collection":  "order_collection",
+---                 "parts": [
+---                     { "source_field": "user_id", "destination_field": "user_id" }
+---                 ],
+---                 "index_name": "user_id_index"
+---             },
+---         ]
+---     }
 ---
 --- @tparam table collections cfg.collections
 --- @tparam table connections cfg.connections - user-defined collections
---- @tparam table indexes cfg.indexes - {collection_name = collection_indexes}
+--- @tparam table indexes cfg.indexes - {[collection_name] = collection_indexes, ...}
 local function complement_connections(collections, connections, indexes, schemas)
     if connections == nil then
         return collections
     end
 
-    at(collections, 'collections', 'table')
-    at(connections, 'connections', 'table')
+    check(collections, 'collections', 'table')
+    check(connections, 'connections', 'table')
+
+    local spaces_formats = get_spaces_formats()
 
     for _, c in pairs(connections) do
-        at(c.name, 'connection.name', 'string')
-        at(c.source_collection, 'connection.source_collection', 'string')
-        at(c.destination_collection, 'connection.destination_collection',
-        'string')
-        at(c.index_name, 'connection.index_name', 'string')
-        at(c.parts, 'connection.parts', 'string', 'table', 'nil')
+        check(c.name, 'connection.name', 'string')
+        check(c.source_collection, 'connection.source_collection', 'string')
+        check(c.destination_collection, 'connection.destination_collection',
+            'string')
+        check(c.index_name, 'connection.index_name', 'string')
+        check(c.parts, 'connection.parts', 'string', 'table', 'nil')
 
         local index = indexes[c.source_collection][c.index_name]
         assert(index.unique ~= nil, 'index.unique must not be nil ' ..
             'during connections complementing')
 
         local result_c = {}
+        result_c.source_collection = c.source_collection
+        result_c.destination_collection = c.destination_collection
         result_c.parts = determine_connection_parts(c.parts, index)
-        result_c.type = determine_connection_type(result_c.parts, index)
+
+        local source_space_format = spaces_formats[result_c.source_collection]
+
+        result_c.type = determine_connection_type(result_c.parts, index,
+            source_space_format)
         result_c.index_name = c.index_name
-        result_c.destination_collection = c.destination_collection
         result_c.name = c.name
 
         local collection_connections = collections[c.source_collection].
-            connections
-        if collection_connections == nil then
-            collection_connections = {}
-        end
+            connections or {}
         collection_connections[#collection_connections + 1] = result_c
     end
-
     return collections
 end
 
---- The function complements
+--- The function complements cfg.collection.connections using given
+--- cfg.connections. See @{complement_connections} for details.
 function config_complement.complement_cfg(cfg)
     cfg.collections = complement_connections(cfg.collections, cfg.connections,
         cfg.indexes)