From 93a7d84ab230f59d5dccf907fb4ce63502cb7de6 Mon Sep 17 00:00:00 2001 From: reindexer-bot <@> Date: Sun, 8 Sep 2024 22:39:13 +0000 Subject: [PATCH] Merge branch 'fix/go-index-tags' into 'develop' Disable indexing for private fields and json:- fields #1829 See merge request itv-backend/reindexer!1666 --- clang-tidy/.clang-tidy | 31 -- clang-tidy/.clang-tidy-ignore | 10 - clang-tidy/run-clang-tidy-18.py | 429 ------------------ .../test/test_storage_compatibility.sh | 195 ++++++++ readme.md | 19 +- reflect.go | 79 ++-- reindexer_impl.go | 21 +- test/index_struct_test.go | 145 ++++++ test/uuid_test.go | 12 +- 9 files changed, 419 insertions(+), 522 deletions(-) delete mode 100644 clang-tidy/.clang-tidy delete mode 100644 clang-tidy/.clang-tidy-ignore delete mode 100755 clang-tidy/run-clang-tidy-18.py create mode 100755 cpp_src/cmd/reindexer_server/test/test_storage_compatibility.sh create mode 100644 test/index_struct_test.go diff --git a/clang-tidy/.clang-tidy b/clang-tidy/.clang-tidy deleted file mode 100644 index 063df74f1..000000000 --- a/clang-tidy/.clang-tidy +++ /dev/null @@ -1,31 +0,0 @@ -Checks: 'clang-diagnostic-*, - clang-analyzer-*, - performance-*, - bugprone-*, - -bugprone-exception-escape, - -bugprone-branch-clone, - -bugprone-easily-swappable-parameters, - -bugprone-macro-parentheses, - -bugprone-signed-char-misuse, - -bugprone-narrowing-conversions, - -bugprone-reserved-identifier, - -bugprone-implicit-widening-of-multiplication-result, - -bugprone-assignment-in-if-condition, - -bugprone-parent-virtual-call, - -bugprone-integer-division, - -bugprone-unhandled-self-assignment, - -bugprone-inc-dec-in-conditions, - -clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling, - -performance-no-int-to-ptr, - -performance-enum-size, - -performance-avoid-endl' -# clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling - too many unnecessary warning in vendored code -# performance-no-int-to-ptr - consider how to fix this -# bugprone-macro-parentheses - consider fixing -WarningsAsErrors: '*' -HeaderFilterRegex: '.*(?= 4.0.0 are given under - # the top level key 'Diagnostics' in the output yaml files - mergekey = "Diagnostics" - merged=[] - for replacefile in glob.iglob(os.path.join(tmpdir, '*.yaml')): - content = yaml.safe_load(open(replacefile, 'r')) - if not content: - continue # Skip empty files. - merged.extend(content.get(mergekey, [])) - - if merged: - # MainSourceFile: The key is required by the definition inside - # include/clang/Tooling/ReplacementsYaml.h, but the value - # is actually never used inside clang-apply-replacements, - # so we set it to '' here. - output = {'MainSourceFile': '', mergekey: merged} - with open(mergefile, 'w') as out: - yaml.safe_dump(output, out) - else: - # Empty the file: - open(mergefile, 'w').close() - - -def find_binary(arg, name, build_path): - """Get the path for a binary or exit""" - if arg: - if shutil.which(arg): - return arg - else: - raise SystemExit( - "error: passed binary '{}' was not found or is not executable" - .format(arg)) - - built_path = os.path.join(build_path, "bin", name) - binary = shutil.which(name) or shutil.which(built_path) - if binary: - return binary - else: - raise SystemExit( - "error: failed to find {} in $PATH or at {}" - .format(name, built_path)) - - -def apply_fixes(args, clang_apply_replacements_binary, tmpdir): - """Calls clang-apply-fixes on a given directory.""" - invocation = [clang_apply_replacements_binary] - invocation.append('-ignore-insert-conflict') - if args.format: - invocation.append('-format') - if args.style: - invocation.append('-style=' + args.style) - invocation.append(tmpdir) - subprocess.call(invocation) - - -def run_tidy(args, clang_tidy_binary, tmpdir, build_path, queue, lock, - failed_files): - """Takes filenames out of queue and runs clang-tidy on them.""" - while True: - name = queue.get() - invocation = get_tidy_invocation(name, clang_tidy_binary, args.checks, - tmpdir, build_path, args.header_filter, - args.allow_enabling_alpha_checkers, - args.extra_arg, args.extra_arg_before, - args.quiet, args.config_file, args.config, - args.line_filter, args.use_color, - args.plugins) - - proc = subprocess.Popen(invocation, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - output, err = proc.communicate() - if proc.returncode != 0: - if proc.returncode < 0: - msg = "%s: terminated by signal %d\n" % (name, -proc.returncode) - err += msg.encode('utf-8') - failed_files.append(name) - with lock: - sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8')) - if len(err) > 0: - sys.stdout.flush() - sys.stderr.write(err.decode('utf-8')) - queue.task_done() - - -def main(): - parser = argparse.ArgumentParser(description='Runs clang-tidy over all files ' - 'in a compilation database. Requires ' - 'clang-tidy and clang-apply-replacements in ' - '$PATH or in your build directory.') - parser.add_argument('-allow-enabling-alpha-checkers', - action='store_true', help='allow alpha checkers from ' - 'clang-analyzer.') - parser.add_argument('-clang-tidy-binary', metavar='PATH', - default='clang-tidy-18', - help='path to clang-tidy binary') - parser.add_argument('-clang-apply-replacements-binary', metavar='PATH', - default='clang-apply-replacements-18', - help='path to clang-apply-replacements binary') - parser.add_argument('-checks', default=None, - help='checks filter, when not specified, use clang-tidy ' - 'default') - config_group = parser.add_mutually_exclusive_group() - config_group.add_argument('-config', default=None, - help='Specifies a configuration in YAML/JSON format: ' - ' -config="{Checks: \'*\', ' - ' CheckOptions: {x: y}}" ' - 'When the value is empty, clang-tidy will ' - 'attempt to find a file named .clang-tidy for ' - 'each source file in its parent directories.') - config_group.add_argument('-config-file', default=None, - help='Specify the path of .clang-tidy or custom config ' - 'file: e.g. -config-file=/some/path/myTidyConfigFile. ' - 'This option internally works exactly the same way as ' - '-config option after reading specified config file. ' - 'Use either -config-file or -config, not both.') - parser.add_argument('-header-filter', default=None, - help='regular expression matching the names of the ' - 'headers to output diagnostics from. Diagnostics from ' - 'the main file of each translation unit are always ' - 'displayed.') - parser.add_argument('-line-filter', default=None, - help='List of files with line ranges to filter the' - 'warnings.') - if yaml: - parser.add_argument('-export-fixes', metavar='filename', dest='export_fixes', - help='Create a yaml file to store suggested fixes in, ' - 'which can be applied with clang-apply-replacements.') - parser.add_argument('-j', type=int, default=0, - help='number of tidy instances to be run in parallel.') - parser.add_argument('files', nargs='*', default=['.*'], - help='files to be processed (regex on path)') - parser.add_argument('-fix', action='store_true', help='apply fix-its') - parser.add_argument('-format', action='store_true', help='Reformat code ' - 'after applying fixes') - parser.add_argument('-style', default='file', help='The style of reformat ' - 'code after applying fixes') - parser.add_argument('-use-color', type=strtobool, nargs='?', const=True, - help='Use colors in diagnostics, overriding clang-tidy\'s' - ' default behavior. This option overrides the \'UseColor' - '\' option in .clang-tidy file, if any.') - parser.add_argument('-p', dest='build_path', - help='Path used to read a compile command database.') - parser.add_argument('-extra-arg', dest='extra_arg', - action='append', default=[], - help='Additional argument to append to the compiler ' - 'command line.') - parser.add_argument('-extra-arg-before', dest='extra_arg_before', - action='append', default=[], - help='Additional argument to prepend to the compiler ' - 'command line.') - parser.add_argument('-ignore', default=DEFAULT_CLANG_TIDY_IGNORE, - help='File path to clang-tidy-ignore') - parser.add_argument('-quiet', action='store_true', - help='Run clang-tidy in quiet mode') - parser.add_argument('-load', dest='plugins', - action='append', default=[], - help='Load the specified plugin in clang-tidy.') - args = parser.parse_args() - - db_path = 'compile_commands.json' - - if args.build_path is not None: - build_path = args.build_path - else: - # Find our database - build_path = find_compilation_database(db_path) - - clang_tidy_binary = find_binary(args.clang_tidy_binary, "clang-tidy", - build_path) - - tmpdir = None - if args.fix or (yaml and args.export_fixes): - clang_apply_replacements_binary = find_binary( - args.clang_apply_replacements_binary, "clang-apply-replacements", - build_path) - tmpdir = tempfile.mkdtemp() - - try: - invocation = get_tidy_invocation("", clang_tidy_binary, args.checks, - None, build_path, args.header_filter, - args.allow_enabling_alpha_checkers, - args.extra_arg, args.extra_arg_before, - args.quiet, args.config_file, args.config, - args.line_filter, args.use_color, - args.plugins) - invocation.append('-list-checks') - invocation.append('-') - if args.quiet: - # Even with -quiet we still want to check if we can call clang-tidy. - with open(os.devnull, 'w') as dev_null: - subprocess.check_call(invocation, stdout=dev_null) - else: - subprocess.check_call(invocation) - except: - print("Unable to run clang-tidy.", file=sys.stderr) - sys.exit(1) - - # Load the database and extract all files. - database = json.load(open(os.path.join(build_path, db_path))) - files = set([make_absolute(entry['file'], entry['directory']) - for entry in database]) - files, excluded = filter_files(args.ignore, files) - if excluded: - print("Excluding the following files:\n" + "\n".join(excluded) + "\n") - - max_task = args.j - if max_task == 0: - max_task = multiprocessing.cpu_count() - - # Build up a big regexy filter from all command line arguments. - file_name_re = re.compile('|'.join(args.files)) - - return_code = 0 - try: - # Spin up a bunch of tidy-launching threads. - task_queue = queue.Queue(max_task) - # List of files with a non-zero return code. - failed_files = [] - lock = threading.Lock() - for _ in range(max_task): - t = threading.Thread(target=run_tidy, - args=(args, clang_tidy_binary, tmpdir, build_path, - task_queue, lock, failed_files)) - t.daemon = True - t.start() - - # Fill the queue with files. - for name in files: - if file_name_re.search(name): - task_queue.put(name) - - # Wait for all threads to be done. - task_queue.join() - if len(failed_files): - return_code = 1 - - except KeyboardInterrupt: - # This is a sad hack. Unfortunately subprocess goes - # bonkers with ctrl-c and we start forking merrily. - print('\nCtrl-C detected, goodbye.') - if tmpdir: - shutil.rmtree(tmpdir) - os.kill(0, 9) - - if yaml and args.export_fixes: - print('Writing fixes to ' + args.export_fixes + ' ...') - try: - merge_replacement_files(tmpdir, args.export_fixes) - except: - print('Error exporting fixes.\n', file=sys.stderr) - traceback.print_exc() - return_code=1 - - if args.fix: - print('Applying fixes ...') - try: - apply_fixes(args, clang_apply_replacements_binary, tmpdir) - except: - print('Error applying fixes.\n', file=sys.stderr) - traceback.print_exc() - return_code = 1 - - if tmpdir: - shutil.rmtree(tmpdir) - sys.exit(return_code) - - -if __name__ == '__main__': - main() diff --git a/cpp_src/cmd/reindexer_server/test/test_storage_compatibility.sh b/cpp_src/cmd/reindexer_server/test/test_storage_compatibility.sh new file mode 100755 index 000000000..d189d3841 --- /dev/null +++ b/cpp_src/cmd/reindexer_server/test/test_storage_compatibility.sh @@ -0,0 +1,195 @@ +#!/bin/bash +# Task: https://github.com/restream/reindexer/-/issues/1188 +set -e + +function KillAndRemoveServer { + local pid=$1 + kill $pid + wait $pid + yum remove -y 'reindexer*' > /dev/null +} + +function WaitForDB { + # wait until DB is loaded + set +e # disable "exit on error" so the script won't stop when DB's not loaded yet + is_connected=$(reindexer_tool --dsn $ADDRESS --command '\databases list'); + while [[ $is_connected != "test" ]] + do + sleep 2 + is_connected=$(reindexer_tool --dsn $ADDRESS --command '\databases list'); + done + set -e +} + +function CompareNamespacesLists { + local ns_list_actual=$1 + local ns_list_expected=$2 + local pid=$3 + + diff=$(echo ${ns_list_actual[@]} ${ns_list_expected[@]} | tr ' ' '\n' | sort | uniq -u) # compare in any order + if [ "$diff" == "" ]; then + echo "## PASS: namespaces list not changed" + else + echo "##### FAIL: namespaces list was changed" + echo "expected: $ns_list_expected" + echo "actual: $ns_list_actual" + KillAndRemoveServer $pid; + exit 1 + fi +} + +function CompareMemstats { + local actual=$1 + local expected=$2 + local pid=$3 + diff=$(echo ${actual[@]} ${expected[@]} | tr ' ' '\n' | sed 's/\(.*\),$/\1/' | sort | uniq -u) # compare in any order + if [ "$diff" == "" ]; then + echo "## PASS: memstats not changed" + else + echo "##### FAIL: memstats was changed" + echo "expected: $expected" + echo "actual: $actual" + KillAndRemoveServer $pid; + exit 1 + fi +} + + +RX_SERVER_CURRENT_VERSION_RPM="$(basename build/reindexer-*server*.rpm)" +VERSION_FROM_RPM=$(echo "$RX_SERVER_CURRENT_VERSION_RPM" | grep -o '.*server-..') +VERSION=$(echo ${VERSION_FROM_RPM: -2:1}) # one-digit version + +echo "## choose latest release rpm file" +if [ $VERSION == 3 ]; then + LATEST_RELEASE=$(python3 cpp_src/cmd/reindexer_server/test/get_last_rx_version.py -v 3) + namespaces_list_expected=$'purchase_options_ext_dict\nchild_account_recommendations\n#config\n#activitystats\nradio_channels\ncollections\n#namespaces\nwp_imports_tasks\nepg_genres\nrecom_media_items_personal\nrecom_epg_archive_default\n#perfstats\nrecom_epg_live_default\nmedia_view_templates\nasset_video_servers\nwp_tasks_schedule\nadmin_roles\n#clientsstats\nrecom_epg_archive_personal\nrecom_media_items_similars\nmenu_items\naccount_recommendations\nkaraoke_items\nmedia_items\nbanners\n#queriesperfstats\nrecom_media_items_default\nrecom_epg_live_personal\nservices\n#memstats\nchannels\nmedia_item_recommendations\nwp_tasks_tasks\nepg' +elif [ $VERSION == 4 ]; then + LATEST_RELEASE=$(python3 cpp_src/cmd/reindexer_server/test/get_last_rx_version.py -v 4) + # replicationstats ns added for v4 + namespaces_list_expected=$'purchase_options_ext_dict\nchild_account_recommendations\n#config\n#activitystats\n#replicationstats\nradio_channels\ncollections\n#namespaces\nwp_imports_tasks\nepg_genres\nrecom_media_items_personal\nrecom_epg_archive_default\n#perfstats\nrecom_epg_live_default\nmedia_view_templates\nasset_video_servers\nwp_tasks_schedule\nadmin_roles\n#clientsstats\nrecom_epg_archive_personal\nrecom_media_items_similars\nmenu_items\naccount_recommendations\nkaraoke_items\nmedia_items\nbanners\n#queriesperfstats\nrecom_media_items_default\nrecom_epg_live_personal\nservices\n#memstats\nchannels\nmedia_item_recommendations\nwp_tasks_tasks\nepg' +else + echo "Unknown version" + exit 1 +fi + +echo "## downloading latest release rpm file: $LATEST_RELEASE" +curl "http://repo.itv.restr.im/itv-api-ng/7/x86_64/$LATEST_RELEASE" --output $LATEST_RELEASE; +echo "## downloading example DB" +curl "https://git.restream.ru/MaksimKravchuk/reindexer_testdata/-/raw/master/big.zip" --output big.zip; +unzip -o big.zip # unzips into mydb_big.rxdump; + +ADDRESS="cproto://127.0.0.1:6534/" +DB_NAME="test" + +memstats_expected=$'[ +{"replication":{"data_hash":24651210926,"data_count":3}}, +{"replication":{"data_hash":6252344969,"data_count":1}}, +{"replication":{"data_hash":37734732881,"data_count":28}}, +{"replication":{"data_hash":0,"data_count":0}}, +{"replication":{"data_hash":1024095024522,"data_count":1145}}, +{"replication":{"data_hash":8373644068,"data_count":1315}}, +{"replication":{"data_hash":0,"data_count":0}}, +{"replication":{"data_hash":0,"data_count":0}}, +{"replication":{"data_hash":0,"data_count":0}}, +{"replication":{"data_hash":0,"data_count":0}}, +{"replication":{"data_hash":7404222244,"data_count":97}}, +{"replication":{"data_hash":94132837196,"data_count":4}}, +{"replication":{"data_hash":1896088071,"data_count":2}}, +{"replication":{"data_hash":0,"data_count":0}}, +{"replication":{"data_hash":-672103903,"data_count":33538}}, +{"replication":{"data_hash":0,"data_count":0}}, +{"replication":{"data_hash":6833710705,"data_count":1}}, +{"replication":{"data_hash":5858155773472,"data_count":4500}}, +{"replication":{"data_hash":-473221280268823592,"data_count":65448}}, +{"replication":{"data_hash":0,"data_count":0}}, +{"replication":{"data_hash":8288213744,"data_count":3}}, +{"replication":{"data_hash":0,"data_count":0}}, +{"replication":{"data_hash":0,"data_count":0}}, +{"replication":{"data_hash":354171024786967,"data_count":3941}}, +{"replication":{"data_hash":-6520334670,"data_count":35886}}, +{"replication":{"data_hash":112772074632,"data_count":281}}, +{"replication":{"data_hash":-12679568198538,"data_count":1623116}} +] +Returned 27 rows' + +echo "##### Forward compatibility test #####" + +DB_PATH=$(pwd)"/rx_db" + +echo "Database: "$DB_PATH + +echo "## installing latest release: $LATEST_RELEASE" +yum install -y $LATEST_RELEASE > /dev/null; +# run RX server with disabled logging +reindexer_server -l warning --httplog=none --rpclog=none --db $DB_PATH & +server_pid=$! +sleep 2; + +reindexer_tool --dsn $ADDRESS$DB_NAME -f mydb_big.rxdump --createdb; +sleep 1; + +namespaces_1=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command '\namespaces list'); +echo $namespaces_1; +CompareNamespacesLists "${namespaces_1[@]}" "${namespaces_list_expected[@]}" $server_pid; + +memstats_1=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command 'select replication.data_hash, replication.data_count from #memstats'); +CompareMemstats "${memstats_1[@]}" "${memstats_expected[@]}" $server_pid; + +KillAndRemoveServer $server_pid; + +echo "## installing current version: $RX_SERVER_CURRENT_VERSION_RPM" +yum install -y build/*.rpm > /dev/null; +reindexer_server -l0 --corelog=none --httplog=none --rpclog=none --db $DB_PATH & +server_pid=$! +sleep 2; + +WaitForDB + +namespaces_2=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command '\namespaces list'); +echo $namespaces_2; +CompareNamespacesLists "${namespaces_2[@]}" "${namespaces_1[@]}" $server_pid; + +memstats_2=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command 'select replication.data_hash, replication.data_count from #memstats'); +CompareMemstats "${memstats_2[@]}" "${memstats_1[@]}" $server_pid; + +KillAndRemoveServer $server_pid; +rm -rf $DB_PATH; +sleep 1; + +echo "##### Backward compatibility test #####" + +echo "## installing current version: $RX_SERVER_CURRENT_VERSION_RPM" +yum install -y build/*.rpm > /dev/null; +reindexer_server -l warning --httplog=none --rpclog=none --db $DB_PATH & +server_pid=$! +sleep 2; + +reindexer_tool --dsn $ADDRESS$DB_NAME -f mydb_big.rxdump --createdb; +sleep 1; + +namespaces_3=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command '\namespaces list'); +echo $namespaces_3; +CompareNamespacesLists "${namespaces_3[@]}" "${namespaces_list_expected[@]}" $server_pid; + +memstats_3=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command 'select replication.data_hash, replication.data_count from #memstats'); +CompareMemstats "${memstats_3[@]}" "${memstats_expected[@]}" $server_pid; + +KillAndRemoveServer $server_pid; + +echo "## installing latest release: $LATEST_RELEASE" +yum install -y $LATEST_RELEASE > /dev/null; +reindexer_server -l warning --httplog=none --rpclog=none --db $DB_PATH & +server_pid=$! +sleep 2; + +WaitForDB + +namespaces_4=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command '\namespaces list'); +echo $namespaces_4; +CompareNamespacesLists "${namespaces_4[@]}" "${namespaces_3[@]}" $server_pid; + +memstats_4=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command 'select replication.data_hash, replication.data_count from #memstats'); +CompareMemstats "${memstats_4[@]}" "${memstats_3[@]}" $server_pid; + +KillAndRemoveServer $server_pid; +rm -rf $DB_PATH; diff --git a/readme.md b/readme.md index 93e615986..8720a66b0 100644 --- a/readme.md +++ b/readme.md @@ -457,10 +457,12 @@ Fields with regular indexes are not nullable. Condition `is NULL` is supported o ### Nested Structs By default, Reindexer scans all nested structs and adds their fields to the namespace (as well as indexes specified). +During indexes scan private (unexported fields), fields tagged with `reindex:"-"` and fields tagged with `json:"-"` will be skipped. ```go type Actor struct { Name string `reindex:"actor_name"` + Age int `reindex:"age"` } type BaseItem struct { @@ -469,13 +471,16 @@ type BaseItem struct { } type ComplexItem struct { - BaseItem // Index fields of BaseItem will be added to reindex - Actor []Actor // Index fields of Actor will be added to reindex as arrays - Name string `reindex:"name"` // Hash-index for "name" - Year int `reindex:"year,tree"` // Tree-index for "year" - Value int `reindex:"value,-"` // Store(column)-index for "value" - Metainfo int `json:"-"` // Field "MetaInfo" will not be stored in reindexer - Parent *Item `reindex:"-"` // Index fields of parent will NOT be added to reindex + BaseItem // Index fields of BaseItem will be added to reindex + Actor []Actor // Index fields ("name" and "age") of Actor will be added to reindex as array-indexes + Name string `reindex:"name"` // Hash-index for "name" + Year int `reindex:"year,tree"` // Tree-index for "year" + Value int `reindex:"value,-"` // Store(column)-index for "value" + Metainfo int `json:"-"` // Field "MetaInfo" will not be stored in reindexer + Parent *Item `reindex:"-"` // Index fields of "Parent" will NOT be added to reindex and all of the "Parent" exported content will be stored as non-indexed data + ParentHidden *Item `json:"-"` // Indexes and fields of "ParentHidden" will NOT be added to reindexer + privateParent *Item // Indexes and fields of "ParentHidden" will NOT be added to reindexer (same as with `json:"-"`) + AnotherActor Actor `reindex:"actor"` // Index fields of "AnotherActor" will be added to reindexer with prefix "actor." (in this example two indexes will be created: "actor.actor_name" and "actor.age") } ``` diff --git a/reflect.go b/reflect.go index 7b1394878..e23197a69 100644 --- a/reflect.go +++ b/reflect.go @@ -35,6 +35,8 @@ type indexOptions struct { isSparse bool rtreeType string isUuid bool + isJoined bool + isComposite bool } func parseRxTags(field reflect.StructField) (idxName string, idxType string, expireAfter string, idxSettings []string) { @@ -56,7 +58,7 @@ func parseRxTags(field reflect.StructField) (idxName string, idxType string, exp return } -func parseIndexes(namespace string, st reflect.Type, joined *map[string][]int) (indexDefs []bindings.IndexDef, err error) { +func parseIndexes(st reflect.Type, joined *map[string][]int) (indexDefs []bindings.IndexDef, err error) { if err = parseIndexesImpl(&indexDefs, st, false, "", "", joined, nil); err != nil { return nil, err } @@ -64,7 +66,7 @@ func parseIndexes(namespace string, st reflect.Type, joined *map[string][]int) ( return indexDefs, nil } -func parseSchema(namespace string, st reflect.Type) *bindings.SchemaDef { +func parseSchema(st reflect.Type) *bindings.SchemaDef { reflector := &jsonschema.Reflector{} reflector.FieldIsInScheme = func(f reflect.StructField) bool { _, _, _, idxSettings := parseRxTags(f) @@ -102,19 +104,20 @@ func parseIndexesImpl(indexDefs *[]bindings.IndexDef, st reflect.Type, subArray } for i := 0; i < st.NumField(); i++ { - t := st.Field(i).Type + field := st.Field(i) + t := field.Type if t.Kind() == reflect.Ptr { t = t.Elem() } // Get and parse tags - jsonPath := strings.Split(st.Field(i).Tag.Get("json"), ",")[0] + jsonTag := strings.Split(field.Tag.Get("json"), ",")[0] - if len(jsonPath) == 0 && !st.Field(i).Anonymous { - jsonPath = st.Field(i).Name + if len(jsonTag) == 0 && !field.Anonymous { + jsonTag = field.Name } - jsonPath = jsonBasePath + jsonPath + jsonPath := jsonBasePath + jsonTag - idxName, idxType, expireAfter, idxSettings := parseRxTags(st.Field(i)) + idxName, idxType, expireAfter, idxSettings := parseRxTags(field) if idxName == "-" { continue } @@ -126,17 +129,33 @@ func parseIndexesImpl(indexDefs *[]bindings.IndexDef, st reflect.Type, subArray } if opts.isPk && strings.TrimSpace(idxName) == "" { - return fmt.Errorf("No index name is specified for primary key in field %s", st.Field(i).Name) + return fmt.Errorf("no index name is specified for primary key in field '%s'; jsonpath: '%s'", field.Name, jsonPath) } if idxType == "rtree" { if t.Kind() != reflect.Array || t.Len() != 2 || t.Elem().Kind() != reflect.Float64 { - return fmt.Errorf("'rtree' index allowed only for [2]float64 or reindexer.Point field type") + return fmt.Errorf("'rtree' index allowed only for [2]float64 or reindexer.Point field type (index name: '%s', field name: '%s', jsonpath: '%s')", + reindexPath, field.Name, jsonPath) } } - if parseByKeyWord(&idxSettings, "composite") { + + if jsonTag == "-" && !opts.isComposite && !opts.isJoined { + if reindexTag := field.Tag.Get("reindex"); reindexTag != "" { + return fmt.Errorf("non-composite/non-joined field ('%s'), marked with `json:-` can not have explicit reindex tags, but it does ('%s')", field.Name, reindexTag) + } + continue + } + if !opts.isComposite && !field.IsExported() { + if reindexTag := field.Tag.Get("reindex"); reindexTag != "" { + return fmt.Errorf("unexported non-composite field ('%s') can not have reindex tags, but it does ('%s')", field.Name, reindexTag) + } + continue + } + + if opts.isComposite { if t.Kind() != reflect.Struct || t.NumField() != 0 { - return fmt.Errorf("'composite' tag allowed only on empty on structs: Invalid tags %v on field %s", strings.SplitN(st.Field(i).Tag.Get("reindex"), ",", 3), st.Field(i).Name) + return fmt.Errorf("'composite' tag allowed only on empty on structs: Invalid tags '%v' on field '%s'", + strings.SplitN(field.Tag.Get("reindex"), ",", 3), field.Name) } indexDef := makeIndexDef(parseCompositeName(reindexPath), parseCompositeJsonPaths(reindexPath), idxType, "composite", opts, CollateNone, "", parseExpireAfter(expireAfter)) @@ -144,13 +163,17 @@ func parseIndexesImpl(indexDefs *[]bindings.IndexDef, st reflect.Type, subArray return err } } else if t.Kind() == reflect.Struct { + if opts.isJoined { + return fmt.Errorf("joined index must be a slice of structs/pointers, but it is a single struct (index name: '%s', field name: '%s', jsonpath: '%s')", + reindexPath, field.Name, jsonPath) + } if err := parseIndexesImpl(indexDefs, t, subArray, reindexPath, jsonPath, joined, parsed); err != nil { return err } } else if (t.Kind() == reflect.Slice || t.Kind() == reflect.Array) && (t.Elem().Kind() == reflect.Struct || (t.Elem().Kind() == reflect.Ptr && t.Elem().Elem().Kind() == reflect.Struct)) { // Check if field nested slice of struct - if parseByKeyWord(&idxSettings, "joined") && len(idxName) > 0 { + if opts.isJoined && len(idxName) > 0 { (*joined)[idxName] = st.Field(i).Index } else if err := parseIndexesImpl(indexDefs, t.Elem(), true, reindexPath, jsonPath, joined, parsed); err != nil { return err @@ -165,17 +188,22 @@ func parseIndexesImpl(indexDefs *[]bindings.IndexDef, st reflect.Type, subArray } if opts.isUuid { if fieldType != "string" { - return fmt.Errorf("UUID index is not applicable with '%v' field, only with 'string'", fieldType) + return fmt.Errorf("UUID index is not applicable with '%v' field, only with 'string' (index name: '%s', field name: '%s', jsonpath: '%s')", + fieldType, reindexPath, field.Name, jsonPath) } fieldType = "uuid" } + if opts.isJoined { + return fmt.Errorf("joined index must be a slice of objects/pointers, but it is a scalar value (index name: '%s', field name: '%s', jsonpath: '%s')", + reindexPath, field.Name, jsonPath) + } indexDef := makeIndexDef(reindexPath, []string{jsonPath}, idxType, fieldType, opts, collateMode, sortOrderLetters, parseExpireAfter(expireAfter)) if err := indexDefAppend(indexDefs, indexDef, opts.isAppenable); err != nil { return err } } if len(idxSettings) > 0 { - return fmt.Errorf("Unknown index settings are found: %v", idxSettings) + return fmt.Errorf("unknown index settings are found: '%v'", idxSettings) } } @@ -202,6 +230,10 @@ func parseOpts(idxSettingsBuf *[]string) indexOptions { opts.rtreeType = idxSetting case "uuid": opts.isUuid = true + case "joined": + opts.isJoined = true + case "composite": + opts.isComposite = true default: newIdxSettingsBuf = append(newIdxSettingsBuf, idxSetting) } @@ -238,7 +270,7 @@ func parseCollate(idxSettingsBuf *[]string) (int, string) { if newCollateMode, ok := collateModes[kvIdxSettings[0]]; ok { if collateMode != CollateNone { - panic(fmt.Errorf("Collate mode is already set to %d. Misunderstanding %s", collateMode, idxSetting)) + panic(fmt.Errorf("collate mode is already set to '%d'. Misunderstanding '%s'", collateMode, idxSetting)) } collateMode = newCollateMode @@ -324,14 +356,6 @@ func getJoinedField(val reflect.Value, joined map[string][]int, name string) (re return ret } -func makeFieldDef(JSONPath string, fieldType string, isArray bool) bindings.FieldDef { - return bindings.FieldDef{ - JSONPath: JSONPath, - Type: fieldType, - IsArray: isArray, - } -} - func makeIndexDef(index string, jsonPaths []string, indexType, fieldType string, opts indexOptions, collateMode int, sortOrder string, expireAfter int) bindings.IndexDef { cm := "" switch collateMode { @@ -373,19 +397,17 @@ func indexDefAppend(indexDefs *[]bindings.IndexDef, indexDef bindings.IndexDef, indexDefExists = true foundIndexDef = indexDef foundIndexPos = pos - break } } if !indexDefExists { *indexDefs = append(*indexDefs, indexDef) - return nil } if indexDef.IndexType != foundIndexDef.IndexType { - return fmt.Errorf("Index %s has another type: %+v", name, indexDef) + return fmt.Errorf("index '%s' has another type: found index def is '%+v' and new index def is '%+v'", name, foundIndexDef, indexDef) } if len(indexDef.JSONPaths) > 0 && indexDef.IndexType != "composite" { @@ -400,9 +422,8 @@ func indexDefAppend(indexDefs *[]bindings.IndexDef, indexDef bindings.IndexDef, if !isPresented { if !isAppendable { - return fmt.Errorf("Index %s is not appendable", name) + return fmt.Errorf("index '%s' is not appendable. Attempt to create array index with multiple JSON-paths: %v", name, indexDef.JSONPaths) } - foundIndexDef.JSONPaths = append(foundIndexDef.JSONPaths, indexDef.JSONPaths[0]) } diff --git a/reindexer_impl.go b/reindexer_impl.go index b4763e32f..f7958f27a 100644 --- a/reindexer_impl.go +++ b/reindexer_impl.go @@ -310,6 +310,7 @@ func (db *reindexerImpl) openNamespace(ctx context.Context, namespace string, op db.binding.DropNamespace(ctx, namespace) continue } + db.unregisterNamespaceImpl(namespace) db.binding.CloseNamespace(ctx, namespace) break } @@ -338,6 +339,12 @@ func (db *reindexerImpl) registerNamespace(ctx context.Context, namespace string return db.registerNamespaceImpl(namespace, opts, s) } +func (db *reindexerImpl) unregisterNamespaceImpl(namespace string) { + db.lock.Lock() + defer db.lock.Unlock() + delete(db.ns, namespace) +} + // registerNamespace Register go type against namespace. There are no data and indexes changes will be performed func (db *reindexerImpl) registerNamespaceImpl(namespace string, opts *NamespaceOptions, s interface{}) (err error) { t := reflect.TypeOf(s) @@ -392,10 +399,10 @@ func (db *reindexerImpl) registerNamespaceImpl(namespace string, opts *Namespace if err = validator.Validate(s); err != nil { return err } - if ns.indexes, err = parseIndexes(namespace, ns.rtype, &ns.joined); err != nil { + if ns.indexes, err = parseIndexes(ns.rtype, &ns.joined); err != nil { return err } - if schema := parseSchema(namespace, ns.rtype); schema != nil { + if schema := parseSchema(ns.rtype); schema != nil { ns.schema = *schema } @@ -418,10 +425,7 @@ func (db *reindexerImpl) dropNamespace(ctx context.Context, namespace string) er db.nsModifySlowLock.Lock() defer db.nsModifySlowLock.Unlock() - db.lock.Lock() - delete(db.ns, namespace) - db.lock.Unlock() - + db.unregisterNamespaceImpl(namespace) return db.binding.DropNamespace(ctx, namespace) } @@ -485,10 +489,7 @@ func (db *reindexerImpl) closeNamespace(ctx context.Context, namespace string) e db.nsModifySlowLock.Lock() defer db.nsModifySlowLock.Unlock() - db.lock.Lock() - delete(db.ns, namespace) - db.lock.Unlock() - + db.unregisterNamespaceImpl(namespace) return db.binding.CloseNamespace(ctx, namespace) } diff --git a/test/index_struct_test.go b/test/index_struct_test.go new file mode 100644 index 000000000..ffd37db08 --- /dev/null +++ b/test/index_struct_test.go @@ -0,0 +1,145 @@ +package reindexer + +import ( + "testing" + + "github.com/restream/reindexer/v3" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type Account struct { + Id int64 `json:"id" reindex:"id,,pk"` + SAN string `json:"san" reindex:"san,hash"` + Count string `json:"-" reindex:"count,hash"` +} + +type Orders struct { + OrderId int64 `json:"-"` + OrderOwnerFName string + OrderTime uint32 `reindex:"ordertime"` + OrderCity string `reindex:"ordercity,tree"` +} + +type TestOpenNamespace struct { + Id int64 `json:"id" reindex:"id,,pk"` + First int `reindex:"first,-" json:"first"` + Age1 int64 `json:"-"` + Age2 int64 `reindex:"-"` + Field int64 + Parent *Orders `reindex:"-"` + + privateSimple string `json:"-"` + privateSlice string `json:"-"` + privateStruct struct{} `json:"-"` + + Accounts1 []Account `json:"-" reindex:"accounts,,joined"` + Accounts2 []Account `json:"-"` + Accounts3 []Account `reindex:"accounts,,joined"` + + Obj []Orders `json:"Ord1,omitempty"` + privateObj []Orders `json:"Ord2,omitempty"` + + _ struct{} `json:"-" reindex:"id+first,,composite"` + privateComposite1 struct{} `reindex:"first+id,,composite"` +} + +type FailSimple struct { + Id int64 `json:"id" reindex:"id,,pk"` + Age string `json:"-" reindex:"age,hash"` +} + +type FailPrivate struct { + Id int64 `json:"id" reindex:"id,,pk"` + private string `json:"private" reindex:"private,hash"` +} + +type FailPrivateJoin struct { + Id int64 `json:"id" reindex:"id,,pk"` + privateAccounts []Account `json:"-" reindex:"accounts,,joined"` +} + +type FailJoinScalar struct { + Id int64 `json:"id" reindex:"id,,pk"` + Accounts string `json:"-" reindex:"accounts,,joined"` +} + +type FailJoinSingleStruct struct { + Id int64 `json:"id" reindex:"id,,pk"` + Accounts Account `json:"-" reindex:"accounts,,joined"` +} + +type FailComposite struct { + Id int64 `json:"id" reindex:"id,,pk"` + Composite []Orders `json:"-" reindex:"ordertime+ordercity,,composite"` +} + +type ExpectedIndexDef struct { + Name string + JSONPaths []string + IndexType string + FieldType string +} + +func TestOpenNs(t *testing.T) { + t.Parallel() + + t.Run("open namespase: check indexes creation", func(t *testing.T) { + if len(DB.slaveList) > 0 { + t.Skip() // This test contains ns open/close and won't work with our replication testing logic + } + + const ns = "test_namespace_open" + err := DB.OpenNamespace(ns, reindexer.DefaultNamespaceOptions(), TestOpenNamespace{}) + defer DB.CloseNamespace(ns) + require.NoError(t, err) + desc, err := DB.DescribeNamespace(ns) + require.NoError(t, err) + actual := make([]ExpectedIndexDef, 0) + for _, idx := range desc.Indexes { + actual = append(actual, ExpectedIndexDef{ + idx.IndexDef.Name, + idx.IndexDef.JSONPaths, + idx.IndexDef.IndexType, + idx.IndexDef.FieldType, + }) + } + expected := []ExpectedIndexDef{ + {Name: "id", JSONPaths: []string{"id"}, IndexType: "hash", FieldType: "int64"}, + {Name: "first", JSONPaths: []string{"first"}, IndexType: "-", FieldType: "int64"}, + {Name: "ordertime", JSONPaths: []string{"Ord1.OrderTime"}, IndexType: "hash", FieldType: "int"}, + {Name: "ordercity", JSONPaths: []string{"Ord1.OrderCity"}, IndexType: "tree", FieldType: "string"}, + {Name: "id+first", JSONPaths: []string{"id", "first"}, IndexType: "hash", FieldType: "composite"}, + {Name: "first+id", JSONPaths: []string{"first", "id"}, IndexType: "hash", FieldType: "composite"}, + } + assert.Equal(t, expected, actual) + }) + + t.Run("no open namespace: check indexes are not created", func(t *testing.T) { + const ns = "test_no_namespace_open" + + err := DB.OpenNamespace(ns, reindexer.DefaultNamespaceOptions(), FailSimple{}) + assert.ErrorContains(t, err, + "non-composite/non-joined field ('Age'), marked with `json:-` can not have explicit reindex tags, but it does ('age,hash')") + + err = DB.OpenNamespace(ns, reindexer.DefaultNamespaceOptions(), FailPrivate{}) + assert.ErrorContains(t, err, + "unexported non-composite field ('private') can not have reindex tags, but it does ('private,hash')") + + err = DB.OpenNamespace(ns, reindexer.DefaultNamespaceOptions(), FailPrivateJoin{}) + assert.ErrorContains(t, err, + "unexported non-composite field ('privateAccounts') can not have reindex tags, but it does ('accounts,,joined')") + + err = DB.OpenNamespace(ns, reindexer.DefaultNamespaceOptions(), FailJoinScalar{}) + assert.ErrorContains(t, err, + "joined index must be a slice of objects/pointers, but it is a scalar value") + + err = DB.OpenNamespace(ns, reindexer.DefaultNamespaceOptions(), FailJoinSingleStruct{}) + assert.ErrorContains(t, err, + "joined index must be a slice of structs/pointers, but it is a single struct") + + err = DB.OpenNamespace(ns, reindexer.DefaultNamespaceOptions(), FailComposite{}) + assert.ErrorContains(t, err, + "'composite' tag allowed only on empty on structs: Invalid tags '[ordertime+ordercity composite]' on field 'Composite'") + }) +} diff --git a/test/uuid_test.go b/test/uuid_test.go index 4316030e3..dc7c3dca8 100644 --- a/test/uuid_test.go +++ b/test/uuid_test.go @@ -240,25 +240,25 @@ func TestNotStringItemsWithUuidTag(t *testing.T) { nsOpts := reindexer.DefaultNamespaceOptions() t.Run("cant open ns when int field with uuid tag", func(t *testing.T) { - errExpr := "UUID index is not applicable with 'int64' field, only with 'string'" + errExpr := "UUID index is not applicable with 'int64' field, only with 'string' (index name: 'uuid', field name: 'Uuid', jsonpath: 'uuid')" assert.EqualError(t, DB.OpenNamespace(TestIntItemWithUuidTagNs, nsOpts, TestIntItemWithUuidTagStruct{}), errExpr) }) t.Run("cant open ns when int64 field with uuid tag", func(t *testing.T) { - errExpr := "UUID index is not applicable with 'int64' field, only with 'string'" + errExpr := "UUID index is not applicable with 'int64' field, only with 'string' (index name: 'uuid', field name: 'Uuid', jsonpath: 'uuid')" assert.EqualError(t, DB.OpenNamespace(TestInt64ItemWithUuidTagNs, nsOpts, TestInt64ItemWithUuidTagStruct{}), errExpr) }) t.Run("cant open ns when float field with uuid tag", func(t *testing.T) { - errExpr := "UUID index is not applicable with 'double' field, only with 'string'" + errExpr := "UUID index is not applicable with 'double' field, only with 'string' (index name: 'uuid', field name: 'Uuid', jsonpath: 'uuid')" assert.EqualError(t, DB.OpenNamespace(TestFloatItemWithUuidTagNs, nsOpts, TestFloatItemWithUuidTagStruct{}), errExpr) }) t.Run("cant open ns when double field with uuid tag", func(t *testing.T) { - errExpr := "UUID index is not applicable with 'double' field, only with 'string'" + errExpr := "UUID index is not applicable with 'double' field, only with 'string' (index name: 'uuid', field name: 'Uuid', jsonpath: 'uuid')" assert.EqualError(t, DB.OpenNamespace(TestDoubleItemWithUuidTagNs, nsOpts, TestDoubleItemWithUuidTagStruct{}), errExpr) }) @@ -270,13 +270,13 @@ func TestNotStringItemsWithUuidTag(t *testing.T) { }) t.Run("cant open ns when byte field with uuid tag", func(t *testing.T) { - errExpr := "UUID index is not applicable with 'int' field, only with 'string'" + errExpr := "UUID index is not applicable with 'int' field, only with 'string' (index name: 'uuid', field name: 'Uuid', jsonpath: 'uuid')" assert.EqualError(t, DB.OpenNamespace(TestByteItemWithUuidTagNs, nsOpts, TestByteItemWithUuidTagStruct{}), errExpr) }) t.Run("cant open ns when bool field with uuid tag", func(t *testing.T) { - errExpr := "UUID index is not applicable with 'bool' field, only with 'string'" + errExpr := "UUID index is not applicable with 'bool' field, only with 'string' (index name: 'uuid', field name: 'Uuid', jsonpath: 'uuid')" assert.EqualError(t, DB.OpenNamespace(TestBoolItemWithUuidTagNs, nsOpts, TestBoolItemWithUuidTagStruct{}), errExpr) })