Altinn / altinn-storage

Altinn platform microservice for handling instance storage
2 stars 3 forks source link

Encapsulating query parameters within a new type #488

Closed Ahmed-Ghanam closed 2 months ago

Ahmed-Ghanam commented 2 months ago

Description

Move the query parameters used to retrieve instance to their new type

Related Issue(s)

Verification

Documentation

SandGrainOne commented 2 months ago

Found a change: Request: storage/api/v1/instances?appId=ttd/apps-test&size=1000&lastChanged=gt:2024-08-14T12:58:11.672345846&process.currentTask=Task_4&excludeConfirmedBy=skd&status.isArchived=false&status.isSoftDeleted=false&status.isHardDeleted=false

Old logic result: {select * from storage.readinstancefromquery_v4 (_appId => 'ttd/apps-test', _appIds => NULL, _archiveReference => NULL, _continue_idx => -1, _created_eq => NULL, _created_gt => NULL, _created_gte => NULL, _created_lt => NULL, _created_lte => NULL, _dueBefore_eq => NULL, _dueBefore_gt => NULL, _dueBefore_gte => NULL, _dueBefore_lt => NULL, _dueBefore_lte => NULL, _excludeConfirmedBy => ARRAY ['[{"StakeholderId":"skd"}]']::jsonb[], _includeElements => True, _instanceOwner_partyId => NULL, _instanceOwner_partyIds => NULL, _lastChanged_eq => NULL, _lastChanged_gt => '2024-08-14T12:58:11.11Z'::timestamptz, _lastChanged_gte => NULL, _lastChanged_idx => NULL, _lastChanged_lt => NULL, _lastChanged_lte => NULL, _msgBoxInterval_eq => NULL, _msgBoxInterval_gt => NULL, _msgBoxInterval_gte => NULL, _msgBoxInterval_lt => NULL, _msgBoxInterval_lte => NULL, _org => NULL , _process_currentTask => 'Task_4', _process_ended_eq => NULL, _process_ended_gt => NULL, _process_ended_gte => NULL, _process_ended_lt => NULL, _process_ended_lte => NULL, _process_isComplete => NULL, _search_string => NULL, _size => 1000, _sort_ascending => False, _status_isActiveOrSoftDeleted => NULL, _status_isArchived => False, _status_isArchivedOrSoftDeleted => NULL, _status_isHardDeleted => False, _status_isSoftDeleted => False, _visibleAfter_eq => NULL, _visibleAfter_gt => NULL, _visibleAfter_gte => NULL, _visibleAfter_lt => NULL, _visibleAfter_lte => NULL, }

New logic results: {select * from storage.readinstancefromquery_v4 (_appId => 'ttd/apps-test', _appIds => NULL, _archiveReference => NULL, _continue_idx => -1, _created_eq => NULL, _created_gt => NULL, _created_gte => NULL, _created_lt => NULL, _created_lte => NULL, _dueBefore_eq => NULL, _dueBefore_gt => NULL, _dueBefore_gte => NULL, _dueBefore_lt => NULL, _dueBefore_lte => NULL, _excludeConfirmedBy => ARRAY ['[{"StakeholderId":"skd"}]']::jsonb[], _includeElements => True, _instanceOwner_partyId => NULL, _instanceOwner_partyIds => NULL, _lastChanged_eq => NULL, _lastChanged_gt => '2024-08-14T12:58:11.11Z'::timestamptz, _lastChanged_gte => NULL, _lastChanged_idx => NULL, _lastChanged_lt => NULL, _lastChanged_lte => NULL, _msgBoxInterval_eq => NULL, _msgBoxInterval_gt => NULL, _msgBoxInterval_gte => NULL, _msgBoxInterval_lt => NULL, _msgBoxInterval_lte => NULL, _org => 'ttd', _process_currentTask => 'Task_4', _process_ended_eq => NULL, _process_ended_gt => NULL, _process_ended_gte => NULL, _process_ended_lt => NULL, _process_ended_lte => NULL, _process_isComplete => NULL, _search_string => NULL, _size => 1000, _sort_ascending => False, _status_isActiveOrSoftDeleted => NULL, _status_isArchived => False, _status_isArchivedOrSoftDeleted => NULL, _status_isHardDeleted => False, _status_isSoftDeleted => False, _visibleAfter_eq => NULL, _visibleAfter_gt => NULL, _visibleAfter_gte => NULL, _visibleAfter_lt => NULL, _visibleAfter_lte => NULL, }

New logic sets the _org parameter. This might be a good thing depending on how the query works.

SandGrainOne commented 2 months ago

Found a change Request: storage/api/v1/instances?instanceOwner.partyId=66522906&appId=skd%2Fformueinntekt-skattemelding-v2

Old logic: {select * from storage.readinstancefromquery_v4 (_appId => 'skd/formueinntekt-skattemelding-v2', _appIds => NULL, _archiveReference => NULL, _continue_idx => -1, _created_eq => NULL, _created_gt => NULL, _created_gte => NULL, _created_lt => NULL, _created_lte => NULL, _dueBefore_eq => NULL, _dueBefore_gt => NULL, _dueBefore_gte => NULL, _dueBefore_lt => NULL, _dueBefore_lte => NULL, _excludeConfirmedBy => NULL, _includeElements => True, _instanceOwner_partyId => 66522906, _instanceOwner_partyIds => NULL, _lastChanged_eq => NULL, _lastChanged_gt => NULL, _lastChanged_gte => NULL, _lastChanged_idx => NULL, _lastChanged_lt => NULL, _lastChanged_lte => NULL, _msgBoxInterval_eq => NULL, _msgBoxInterval_gt => NULL, _msgBoxInterval_gte => NULL, _msgBoxInterval_lt => NULL, _msgBoxInterval_lte => NULL, _org => NULL, _process_currentTask => NULL, _process_ended_eq => NULL, _process_ended_gt => NULL, _process_ended_gte => NULL, _process_ended_lt => NULL, _process_ended_lte => NULL, _process_isComplete => NULL, _search_string => NULL, _size => 100, _sort_ascending => False, _status_isActiveOrSoftDeleted => NULL, _status_isArchived => NULL, _status_isArchivedOrSoftDeleted => NULL, _status_isHardDeleted => False, _status_isSoftDeleted => NULL, _visibleAfter_eq => NULL, _visibleAfter_gt => NULL, _visibleAfter_gte => NULL, _visibleAfter_lt => NULL, _visibleAfter_lte => NULL, }

New logic: {select * from storage.readinstancefromquery_v4 (_appId => 'skd/formueinntekt-skattemelding-v2', _appIds => NULL, _archiveReference => NULL, _continue_idx => -1, _created_eq => NULL, _created_gt => NULL, _created_gte => NULL, _created_lt => NULL, _created_lte => NULL, _dueBefore_eq => NULL, _dueBefore_gt => NULL, _dueBefore_gte => NULL, _dueBefore_lt => NULL, _dueBefore_lte => NULL, _excludeConfirmedBy => ARRAY]::jsonb[], _includeElements => True, _instanceOwner_partyId => 66522906, _instanceOwner_partyIds => NULL, _lastChanged_eq => NULL, _lastChanged_gt => NULL, _lastChanged_gte => NULL, _lastChanged_idx => NULL, _lastChanged_lt => NULL, _lastChanged_lte => NULL, _msgBoxInterval_eq => NULL, _msgBoxInterval_gt => NULL, _msgBoxInterval_gte => NULL, _msgBoxInterval_lt => NULL, _msgBoxInterval_lte => NULL, _org => NULL, _process_currentTask => NULL, _process_ended_eq => NULL, _process_ended_gt => NULL, _process_ended_gte => NULL, _process_ended_lt => NULL, _process_ended_lte => NULL, _process_isComplete => NULL, _search_string => NULL, _size => 100, _sort_ascending => False, _status_isActiveOrSoftDeleted => NULL, _status_isArchived => NULL, _status_isArchivedOrSoftDeleted => NULL, _status_isHardDeleted => False, _status_isSoftDeleted => NULL, _visibleAfter_eq => NULL, _visibleAfter_gt => NULL, _visibleAfter_gte => NULL, _visibleAfter_lt => NULL, _visibleAfter_lte => NULL, }

The new logic produce an empty array for _excludeConfirmedBy instead of producing NULL. That is something I believe we need to fix.

Ahmed-Ghanam commented 2 months ago

@SandGrainOne I updated the new logic and it will not replace empty arrays with NULLs.

SandGrainOne commented 2 months ago

Found a change that must be fixed:

MessageBoxController: POST:

{
    "language": "nb",
    "instanceOwnerPartyIdList": [50809924],
    "fromCreated": "2024-05-31",
    "toCreated": "9999-05-31T14:44:00"
}

In the new logic we populate _instanceOwner_partyIds and leave _instanceOwner_partyId NULL. For performance reasons it is important that we use the _instanceOwner_partyIdparameter when there is a single partyId. Use _instanceOwner_partyIds only if there are more than one party.

NEW: _instanceOwner_partyId => NULL, _instanceOwner_partyIds => '{50809924}',
OLD: _instanceOwner_partyId => 50809924, _instanceOwner_partyIds => NULL,

I also tested OLD code with more than one party as input and this is the result:

OLD: _includeElements => False, _instanceOwner_partyId => NULL, _instanceOwner_partyIds => '{50809924, 60809925}'
Ahmed-Ghanam commented 2 months ago

Found a change that must be fixed:

MessageBoxController: POST:

{
    "language": "nb",
    "instanceOwnerPartyIdList": [50809924],
    "fromCreated": "2024-05-31",
    "toCreated": "9999-05-31T14:44:00"
}

In the new logic we populate _instanceOwner_partyIds and leave _instanceOwner_partyId NULL. For performance reasons it is important that we use the _instanceOwner_partyIdparameter when there is a single partyId. Use _instanceOwner_partyIds only if there are more than one party.

NEW: _instanceOwner_partyId => NULL, _instanceOwner_partyIds => '{50809924}',
OLD: _instanceOwner_partyId => 50809924, _instanceOwner_partyIds => NULL,

I also tested OLD code with more than one party as input and this is the result:

OLD: _includeElements => False, _instanceOwner_partyId => NULL, _instanceOwner_partyIds => '{50809924, 60809925}'

FYI: This is fixed

sonarcloud[bot] commented 2 months ago

Quality Gate Passed Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
81.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud