Feat/version2.0 feature merges by lfbrehm · Pull Request #97 · arunaengine/aruna

lfbrehm · 2023-07-31T12:31:45Z

Summary

This PR contains delete, relations and archive requests.

Delete

Resources are updated with object_status = DELETED

Relations

The API request ModifyRelations is now implemented

Archive/Snapshot

Projects and all resources belonging to the object are updated to dynamic = false
Collections/Datasets are cloned with a persistent copy

# Conflicts: # Cargo.lock # Cargo.toml # src/database/dsls/object_dsl.rs # src/grpc/collections.rs # src/grpc/datasets.rs # src/grpc/notification.rs # src/grpc/relations.rs # tests/setup.sh

…ections and Datasets.

# Conflicts: # src/grpc/collections.rs

St4NNi

Good work, one thing i really do not like is iterating over DB requests. We should add get / update requests that take a list and process them all at once.

St4NNi · 2023-07-31T12:41:05Z

+        Vec<InternalRelation>, // outbound relations
+        Vec<InternalRelation>, // inbound relations
+    )> {
+        let outbound = "SELECT * FROM internal_relations 


not a fan, we should combine outbound and inbound to one request:

`SELECT * FROM internal_relations WHERE target_pid = $1 OR origin_pid = $1;

And separate the afterwards, especially since you are iterating over them anyway.

Iterator::partition can be used to get two vectors.

St4NNi · 2023-07-31T12:47:45Z

+            "Internal database error"
+        );
+
+        for o in updates {


Not a fan either, would prefer updating them all at once, with one db request.

St4NNi · 2023-07-31T12:51:14Z

+            self.database_handler.snapshot(request).await,
+            "Internal database error."
+        );
+        for resource in &resources {


Again, for loop for updating sounds not like a good idea.

St4NNi · 2023-07-31T12:51:25Z

+            "Internal database error"
+        );
+
+        for o in updates {


Same as col.

St4NNi · 2023-07-31T12:52:57Z

+            "Internal database error"
+        );
+
+        for o in updates {


Again updating them all separately is a bad idea

St4NNi · 2023-07-31T12:57:05Z

+                .remove_external_relation(&transaction_client, external_to_remove)
+                .await?;
+        }
+        for internal_to_remove in labels_to_remove.internal {


St4NNi · 2023-07-31T12:58:13Z

+        client: &Client,
+    ) -> Result<Vec<ObjectWithRelations>> {
+        let mut result: Vec<ObjectWithRelations> = Vec::new();
+        for relation in &project.relation_ids {


St4NNi · 2023-07-31T12:58:21Z

+        for relation in &project.relation_ids {
+            InternalRelation::archive(relation, client).await?;
+        }
+        for resource in &project.resource_ids {


St4NNi · 2023-07-31T12:59:00Z

+        }
+        for resource in &project.resource_ids {
+            Object::archive(resource, client).await?;
+            result.push(Object::get_object_with_relations(resource, client).await?);


updating AND retrieving with two requests in a for loop is really bad

St4NNi · 2023-07-31T13:00:13Z

+        let (mut ids, mut init_in) = InternalRelation::get_all_by_id(project_id, client).await?;
+        ids.append(&mut init_in);
+        for id in ids.iter().map(|i| i.id) {
+            let (outs, ins) = InternalRelation::get_all_by_id(id, client).await?;


get_all in for loop ?

das-Abroxas

Some copy & paste things that should be smoothed out before merge.

das-Abroxas · 2023-07-31T13:04:44Z

+        let collection_id = tonic_invalid!(request.get_id(), "Invalid project id");
+        let ctx = Context::res_ctx(collection_id, DbPermissionLevel::WRITE, true);
+
+        tonic_auth!(
+            self.authorizer.check_permissions(&token, vec![ctx]).await,
+            "Unauthorized"
+        );
+
+        let project = tonic_internal!(
+            self.database_handler.update_name(request).await,
+            "Internal database error."
+        );
+        self.cache
+            .update_object(&project.object.id, project.clone());
+        let project: generic_resource::Resource =
+            tonic_internal!(project.try_into(), "Collection conversion error");


Copy & paste only partially adapted.

Suggested change

let collection_id = tonic_invalid!(request.get_id(), "Invalid project id");

let ctx = Context::res_ctx(collection_id, DbPermissionLevel::WRITE, true);

tonic_auth!(

self.authorizer.check_permissions(&token, vec![ctx]).await,

"Unauthorized"

);

let project = tonic_internal!(

self.database_handler.update_name(request).await,

"Internal database error."

);

self.cache

.update_object(&project.object.id, project.clone());

let project: generic_resource::Resource =

tonic_internal!(project.try_into(), "Collection conversion error");

let project_id = tonic_invalid!(request.get_id(), "Invalid project id");

let ctx = Context::res_ctx(project_id, DbPermissionLevel::WRITE, true);

tonic_auth!(

self.authorizer.check_permissions(&token, vec![ctx]).await,

"Unauthorized"

);

let project = tonic_internal!(

self.database_handler.update_name(request).await,

"Internal database error."

);

self.cache

.update_object(&project.object.id, project.clone());

let project: generic_resource::Resource =

tonic_internal!(project.try_into(), "Project conversion error");

das-Abroxas · 2023-07-31T13:05:46Z

+        let request = DescriptionUpdate::Project(request.into_inner());
+        let collection_id = tonic_invalid!(request.get_id(), "Invalid project id");
+        let ctx = Context::res_ctx(collection_id, DbPermissionLevel::WRITE, true);
+
+        tonic_auth!(
+            self.authorizer.check_permissions(&token, vec![ctx]).await,
+            "Unauthorized"
+        );
+
+        let project = tonic_internal!(
+            self.database_handler.update_description(request).await,
+            "Internal database error."
+        );
+        self.cache
+            .update_object(&project.object.id, project.clone());
+        let project: generic_resource::Resource =
+            tonic_internal!(project.try_into(), "Collection conversion error");


Copy & paste only partially adapted.

Suggested change

let request = DescriptionUpdate::Project(request.into_inner());

let collection_id = tonic_invalid!(request.get_id(), "Invalid project id");

let ctx = Context::res_ctx(collection_id, DbPermissionLevel::WRITE, true);

tonic_auth!(

self.authorizer.check_permissions(&token, vec![ctx]).await,

"Unauthorized"

);

let project = tonic_internal!(

self.database_handler.update_description(request).await,

"Internal database error."

);

self.cache

.update_object(&project.object.id, project.clone());

let project: generic_resource::Resource =

tonic_internal!(project.try_into(), "Collection conversion error");

let request = DescriptionUpdate::Project(request.into_inner());

let project_id = tonic_invalid!(request.get_id(), "Invalid project id");

let ctx = Context::res_ctx(project_id, DbPermissionLevel::WRITE, true);

tonic_auth!(

self.authorizer.check_permissions(&token, vec![ctx]).await,

"Unauthorized"

);

let project = tonic_internal!(

self.database_handler.update_description(request).await,

"Internal database error."

);

self.cache

.update_object(&project.object.id, project.clone());

let project: generic_resource::Resource =

tonic_internal!(project.try_into(), "Project conversion error");

das-Abroxas · 2023-07-31T13:06:51Z

+        let collection_id = tonic_invalid!(request.get_id(), "Invalid project id");
+        let ctx = Context::res_ctx(collection_id, DbPermissionLevel::WRITE, true);
+
+        tonic_auth!(
+            self.authorizer.check_permissions(&token, vec![ctx]).await,
+            "Unauthorized"
+        );
+
+        let project = tonic_internal!(
+            self.database_handler.update_keyvals(request).await,
+            "Internal database error."
+        );
+        self.cache
+            .update_object(&project.object.id, project.clone());
+        let project: generic_resource::Resource =
+            tonic_internal!(project.try_into(), "Collection conversion error");


Copy & paste only partially adapted.

Suggested change

let collection_id = tonic_invalid!(request.get_id(), "Invalid project id");

let ctx = Context::res_ctx(collection_id, DbPermissionLevel::WRITE, true);

tonic_auth!(

self.authorizer.check_permissions(&token, vec![ctx]).await,

"Unauthorized"

);

let project = tonic_internal!(

self.database_handler.update_keyvals(request).await,

"Internal database error."

);

self.cache

.update_object(&project.object.id, project.clone());

let project: generic_resource::Resource =

tonic_internal!(project.try_into(), "Collection conversion error");

let project_id = tonic_invalid!(request.get_id(), "Invalid project id");

let ctx = Context::res_ctx(project_id, DbPermissionLevel::WRITE, true);

tonic_auth!(

self.authorizer.check_permissions(&token, vec![ctx]).await,

"Unauthorized"

);

let project = tonic_internal!(

self.database_handler.update_keyvals(request).await,

"Internal database error."

);

self.cache

.update_object(&project.object.id, project.clone());

let project: generic_resource::Resource =

tonic_internal!(project.try_into(), "Project conversion error");

das-Abroxas · 2023-07-31T13:07:46Z

+        let collection_id = tonic_invalid!(request.get_id(), "Invalid project id");
+        let ctx = Context::res_ctx(collection_id, DbPermissionLevel::WRITE, true);
+
+        tonic_auth!(
+            self.authorizer.check_permissions(&token, vec![ctx]).await,
+            "Unauthorized"
+        );
+
+        let project = tonic_internal!(
+            self.database_handler.update_dataclass(request).await,
+            "Internal database error."
+        );
+        self.cache
+            .update_object(&project.object.id, project.clone());
+        let project: generic_resource::Resource =
+            tonic_internal!(project.try_into(), "Collection conversion error");


Copy & paste only partially adapted.

Suggested change

let collection_id = tonic_invalid!(request.get_id(), "Invalid project id");

let ctx = Context::res_ctx(collection_id, DbPermissionLevel::WRITE, true);

tonic_auth!(

self.authorizer.check_permissions(&token, vec![ctx]).await,

"Unauthorized"

);

let project = tonic_internal!(

self.database_handler.update_dataclass(request).await,

"Internal database error."

);

self.cache

.update_object(&project.object.id, project.clone());

let project: generic_resource::Resource =

tonic_internal!(project.try_into(), "Collection conversion error");

let project_id = tonic_invalid!(request.get_id(), "Invalid project id");

let ctx = Context::res_ctx(project_id, DbPermissionLevel::WRITE, true);

tonic_auth!(

self.authorizer.check_permissions(&token, vec![ctx]).await,

"Unauthorized"

);

let project = tonic_internal!(

self.database_handler.update_dataclass(request).await,

"Internal database error."

);

self.cache

.update_object(&project.object.id, project.clone());

let project: generic_resource::Resource =

tonic_internal!(project.try_into(), "Project conversion error");

das-Abroxas · 2023-07-31T13:13:36Z

+        let project_id = tonic_invalid!(request.get_id(), "Invalid dataset id.");
+        let ctx = Context::res_ctx(project_id, DbPermissionLevel::ADMIN, true);
+
+        tonic_auth!(
+            self.authorizer.check_permissions(&token, vec![ctx]).await,
+            "Unauthorized"
+        );
+
+        // this only contains one entry with a dataset
+        let resources = tonic_internal!(
+            self.database_handler.snapshot(request).await,
+            "Internal database error."
+        );
+        for resource in &resources {
+            self.cache
+                .update_object(&resource.object.id, resource.clone());
+        }
+        let project: generic_resource::Resource =
+            // First entry is always the archived project 
+            tonic_internal!(resources[0].clone().try_into(), "Dataset conversion error");


Copy & paste only partially adapted.

Suggested change

let project_id = tonic_invalid!(request.get_id(), "Invalid dataset id.");

let ctx = Context::res_ctx(project_id, DbPermissionLevel::ADMIN, true);

tonic_auth!(

self.authorizer.check_permissions(&token, vec![ctx]).await,

"Unauthorized"

);

// this only contains one entry with a dataset

let resources = tonic_internal!(

self.database_handler.snapshot(request).await,

"Internal database error."

);

for resource in &resources {

self.cache

.update_object(&resource.object.id, resource.clone());

}

let project: generic_resource::Resource =

// First entry is always the archived project

tonic_internal!(resources[0].clone().try_into(), "Dataset conversion error");

let project_id = tonic_invalid!(request.get_id(), "Invalid project id");

let ctx = Context::res_ctx(project_id, DbPermissionLevel::ADMIN, true);

tonic_auth!(

self.authorizer.check_permissions(&token, vec![ctx]).await,

"Unauthorized"

);

// This contains all the resources the snapshot includes below the project

let resources = tonic_internal!(

self.database_handler.snapshot(request).await,

"Internal database error."

);

for resource in &resources {

self.cache

.update_object(&resource.object.id, resource.clone());

}

let project: generic_resource::Resource =

// First entry is always the archived project

tonic_internal!(resources[0].clone().try_into(), "Project conversion error");

das-Abroxas · 2023-07-31T13:16:29Z

+        let collection_id = tonic_invalid!(request.get_id(), "Invalid dataset id.");
+        let ctx = Context::res_ctx(collection_id, DbPermissionLevel::ADMIN, true);
+
+        tonic_auth!(
+            self.authorizer.check_permissions(&token, vec![ctx]).await,
+            "Unauthorized"
+        );
+
+        // this only contains one entry with a dataset
+        let resources = tonic_internal!(
+            self.database_handler.snapshot(request).await,
+            "Internal database error."
+        );
+        for resource in &resources {
+            self.cache
+                .update_object(&resource.object.id, resource.clone());
+        }
+        let collection: generic_resource::Resource =
+        // First entry is always the snapshot collection
+            tonic_internal!(resources[0].clone().try_into(), "Dataset conversion error");


Copy & paste only partially adapted.

Suggested change

let collection_id = tonic_invalid!(request.get_id(), "Invalid dataset id.");

let ctx = Context::res_ctx(collection_id, DbPermissionLevel::ADMIN, true);

tonic_auth!(

self.authorizer.check_permissions(&token, vec![ctx]).await,

"Unauthorized"

);

// this only contains one entry with a dataset

let resources = tonic_internal!(

self.database_handler.snapshot(request).await,

"Internal database error."

);

for resource in &resources {

self.cache

.update_object(&resource.object.id, resource.clone());

}

let collection: generic_resource::Resource =

// First entry is always the snapshot collection

tonic_internal!(resources[0].clone().try_into(), "Dataset conversion error");

let collection_id = tonic_invalid!(request.get_id(), "Invalid collection id.");

let ctx = Context::res_ctx(collection_id, DbPermissionLevel::ADMIN, true);

tonic_auth!(

self.authorizer.check_permissions(&token, vec![ctx]).await,

"Unauthorized"

);

// This contains all resources the snapshot includes below the collection

let resources = tonic_internal!(

self.database_handler.snapshot(request).await,

"Internal database error."

);

for resource in &resources {

self.cache

.update_object(&resource.object.id, resource.clone());

}

let collection: generic_resource::Resource =

// First entry is always the snapshot collection

tonic_internal!(resources[0].clone().try_into(), "Collection conversion error");

codecov · 2023-07-31T13:33:59Z

Codecov Report

Patch coverage: 20.92% and project coverage change: +3.26% 🎉

Comparison is base (b3377d2) 17.96% compared to head (83a30b6) 21.23%.
Report is 1 commits behind head on feat/version2.0rework.

Additional details and impacted files

@@                    Coverage Diff                    @@
##           feat/version2.0rework      #97      +/-   ##
=========================================================
+ Coverage                  17.96%   21.23%   +3.26%     
=========================================================
  Files                         36       43       +7     
  Lines                       3022     3758     +736     
=========================================================
+ Hits                         543      798     +255     
- Misses                      2479     2960     +481

Files Changed	Coverage Δ
src/auth/permission_handler.rs	`0.00% <0.00%> (ø)`
src/auth/structs.rs	`0.00% <0.00%> (ø)`
src/auth/token_handler.rs	`0.00% <0.00%> (ø)`
src/grpc/collections.rs	`0.00% <0.00%> (ø)`
src/grpc/datasets.rs	`0.00% <0.00%> (ø)`
src/grpc/notification.rs	`0.00% <0.00%> (ø)`
src/grpc/object.rs	`0.00% <0.00%> (ø)`
src/grpc/projects.rs	`0.00% <0.00%> (ø)`
src/grpc/relations.rs	`0.00% <0.00%> (ø)`
src/macros.rs	`14.28% <ø> (ø)`
... and 18 more

... and 2 files with indirect coverage changes

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

St4NNi

Nice, additions for now LGTM. This will surely be modified in further developments.

lfbrehm and others added 30 commits July 27, 2023 16:20

feat: DeleteRequests implementation.

810de5f

feat: Updated cache with deleted info.

7854431

feat: Update requests with cache update.

8ab6bd0

fix: Tests

48a5b76

fix: Codecov action.

372d924

fix: Codecov action.

f54a102

feat: Switched gerneric function to impl Into<String>

c8b16ce

feat: Update user_dsl permission and token handling

b23b3f3

feat: Added external id to user struct

5726e7d

fix: InternalRelation... wrong index

2ea3408

chore: Missing newlines in object

76f7e20

feat: Added blanket cache impl in server

4d17536

feat: Update schema.sql

e6cca22

feat: Added token handling

5ca5807

feat: New Token and auth handling

e2eb332

feat: Update create handling

cf4a80d

feat: Update get_object_with_relations query

c8a605d

feat: Update objects

f65c9b0

chore: Update diesel_ulid and serde

7837a73

chore: Update create_resource

20431f3

feat: Sold my soul to compiler gods

36714dc

chore: Clippy fixes, fmt

33e1d05

fix: Tests

235a839

fix: Relation tests

48c77b2

feat: Added modify labels request.

16eae08

fix: Cargo.lock

6a8c2fa

Merge branch 'feat/v2_relations' into feat/version2.0_feature_merges

4acea25

# Conflicts: # Cargo.lock # Cargo.toml # src/database/dsls/object_dsl.rs # src/grpc/collections.rs # src/grpc/datasets.rs # src/grpc/notification.rs # src/grpc/relations.rs # tests/setup.sh

fix: Fixed compiler errors.

d84894f

fix: Formatting

36550f0

feat: Snapshot impl for collections & datasets.

27d7dcd

lfbrehm added 3 commits July 31, 2023 14:05

feat: Snapshot and Archive functions on GRPC level for Projects, Coll…

f2e1244

…ections and Datasets.

Merge branch 'feat/version2.0rework' into feat/version2.0_feature_merges

6e60f04

# Conflicts: # src/grpc/collections.rs

fix: Updated policy evaluation.

6273be5

lfbrehm requested review from St4NNi and das-Abroxas July 31, 2023 12:31

St4NNi requested changes Jul 31, 2023

View reviewed changes

chore: Applied clippy suggestions.

7a98e76

das-Abroxas requested changes Jul 31, 2023

View reviewed changes

chore: Cargo clippy and cargo fmt

e094dc8

lfbrehm added 7 commits August 1, 2023 15:32

refactor: Applied suggested changes.

c8548f6

refactor: Applied suggested changes.

9259ecb

chore: Applied clippy suggestions.

a077341

fix: Fixed copy paste error.

ba26e3a

fix: Fixed return for snapshot requests.

7871004

fix: Fixed SQL queries with multi parameter queries.

593ccbb

fix: Fixed external_relation requests.

f552ec9

St4NNi requested review from St4NNi and das-Abroxas August 3, 2023 08:47

St4NNi approved these changes Aug 3, 2023

View reviewed changes

lfbrehm added 2 commits August 3, 2023 10:52

fix: Fixed archive request.

99aed7f

style: Fixed formatting for tests.

83a30b6

das-Abroxas approved these changes Aug 3, 2023

View reviewed changes

lfbrehm merged commit 931b4c9 into feat/version2.0rework Aug 3, 2023

lfbrehm deleted the feat/version2.0_feature_merges branch August 3, 2023 09:40

Conversation

lfbrehm commented Jul 31, 2023

Summary

Delete

Relations

Archive/Snapshot

Uh oh!

St4NNi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

das-Abroxas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Jul 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

St4NNi left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Jul 31, 2023 •

edited

Loading