CMR-11155: Index-set version control -- Part 2/2

[jmaeng72]

Overview

What is the objective?

To allow for disaster recovery and keeping track of historical changes of index-set.

What are the changes?

• Created new index-set table in database as new source of truth
• Created new /sync-with-database api to be able to recover index-sets from database if we lose the index in elastic
• Created new /index-sets/:id?revision_id=X to get different revision versions of the index-set for historical reference
• Index set is treated like a concept now, old revisions are deleted up to 10 versions and there is a new revision created for every create, update, delete. Delete index-set is tombstoned in the db

What areas of the application does this impact?

Indexer

Required Checklist

• New and existing unit and int tests pass locally and remotely
• clj-kondo has been run locally and all errors in changed files are corrected
• I have commented my code, particularly in hard-to-understand areas
• I have made changes to the documentation (if necessary)
• My changes generate no new warnings

Additional Checklist

• I have removed unnecessary/dead code and imports in files I have changed
• I have cleaned up integration tests by doing one or more of the following:
  • migrated any are2 tests to are3 in files I have changed
  • de-duped, consolidated, removed dead int tests
  • transformed applicable int tests into unit tests
  • reduced number of system state resets by updating fixtures. Ex) (use-fixtures :each (ingest/reset-fixture {})) to be :once instead of :each

[jmaeng72]

if the format of the metadata is always the same, do we want this col? I can delete if we assume all metadata is going to be json

[eereiter]

I don't think that we need the format column.

[jmaeng72]

since the indexer uses the concepts flow, format is required so I have decided to keep it

[eereiter]

Why did these get added?

[eereiter]

Is this because when a request comes in the request Authorization token can be checked?

[jmaeng72]

I think this was a remanent of an old change I reverted, so I am going to delete this

[jmaeng72]

Actually, turns out we do need it to gain permissions to look at cache lists and check tokens

The following tests for these permissions under indexer:
cmr.system-int-test.admin.cache-api-test/cache-apis
cmr.system-int-test.admin.admin-permissions-test/ingest-management-permission-test

[zimzoom]

Seems like this is saving a revision to Oracle before the validation 4 lines down (validate-requested-index-set), is that correct? Should we validate before saving?

[jmaeng72]

Yes definitely. I will update this. Good catch

[zimzoom]

Seems like this will result in saving a duplicate revision to the DB during disaster recovery sync. It's reading the latest revision then calling put-index-set which turns around and writes another revision. This could push a genuine revision out of the 10 item limit, up to you if that matters

[jmaeng72]

Should be fine since the new revision will be the same as the old so we'll still have the right copy

[zimzoom]

Suggestion to check the status of this reset call, e.g.: (is (= 204 (:status (index/index-set-reset))))

Because I don't understand how this test is working right now, since calling reset deletes from Oracle as well as removes from ES --> then the sync call below does not restore Oracle tombstones --> then test ends with asserting it is index-set 5555 specifically that was restored.

The suggested line could check for possibility that the reset failed to delete from Oracle

[jmaeng72]

I have added comments and the extra reset call check

[codecov-commenter]

Codecov Report

❌ Patch coverage is 15.85366% with 138 lines in your changes missing coverage. Please review.
✅ Project coverage is 29.30%. Comparing base (1b13525) to head (3a47e83).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...app/src/cmr/indexer/services/index_set_service.clj	2.56%	76 Missing ⚠️
indexer-app/src/cmr/indexer/data/elasticsearch.clj	0.00%	24 Missing ⚠️
indexer-app/src/cmr/indexer/api/routes.clj	0.00%	17 Missing ⚠️
...adata_db/migrations/093_setup_index_sets_table.clj	47.36%	10 Missing ⚠️
...-test/src/cmr/system_int_test/utils/index_util.clj	71.42%	4 Missing ⚠️
...exer-app/src/cmr/indexer/common/index_set_util.clj	0.00%	2 Missing ⚠️
...p/src/cmr/metadata_db/services/concept_service.clj	0.00%	2 Missing ⚠️
...p/src/cmr/indexer/data/index_set_elasticsearch.clj	50.00%	1 Missing ⚠️
...src/cmr/metadata_db/data/oracle/concept_tables.clj	0.00%	0 Missing and 1 partial ⚠️
...b-app/src/cmr/metadata_db/data/oracle/concepts.clj	0.00%	1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (1b13525) and HEAD (3a47e83). Click for more details.

HEAD has 36 uploads less than BASE

Flag	BASE (1b13525)	HEAD (3a47e83)
	40	4

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #2458       +/-   ##
===========================================
- Coverage   58.06%   29.30%   -28.76%     
===========================================
  Files        1071     1012       -59     
  Lines       74364    71000     -3364     
  Branches     2169     1200      -969     
===========================================
- Hits        43180    20809    -22371     
- Misses      29162    49053    +19891     
+ Partials     2022     1138      -884     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.