Add AppSessionId #793

nieznanysprawiciel · 2020-11-17T12:41:04Z

Event API - Agreement events
Add AppSessionId

resolves: #726

Contains changes from: #754
which resolves: #729

…reement-events

…ocedural macro for generating ToSql/FromSql implementation

…reement-events

…a-client repo

…ession-id

tworec

part 1

core/market/migrations/2020-10-04-190200_agreement_events/up.sql

core/market/src/db/dao.rs

core/market/src/db/dao/agreement.rs

core/market/src/db/dao/agreement_events.rs

tworec · 2020-11-20T14:49:54Z

core/market/src/db/model/agreement.rs


 use crate::db::model::{OwnerType, Proposal, ProposalId, SubscriptionId};
 use crate::db::schema::market_agreement;

 pub type AgreementId = ProposalId;
+pub type AppSessionId = Option<String>;


as this is user input, we need to check if this is at most 100 chars. Is there a unit test for such a case?

I didn't know, we have such a limit. Is it documented?

you have implicitly set this limit via up.sql ;)

core/market/src/db/model/agreement.rs

tworec · 2020-11-20T14:53:11Z

core/market/src/db/model/agreement_events.rs

+                log::warn!(
+                    "Agreement Event with not parsable Reason in database. Error: {}. Shouldn't happen \
+                     because market is responsible for rejecting invalid Reasons.", e
+                )


minor: I'd also return this warning mesage as a proper JsonReason for the clients to be aware and can report if it happens one day

You mean, that you would set this error message in JsonResponse?

yes, and extend this message to contain whole reason contents

Cargo.toml

tworec

uhh, it was big. But here it is

core/market/src/db/model/proposal_id.rs

core/market/src/negotiation/common.rs

core/market/src/utils/display.rs

core/market/src/market.rs

core/market/src/rest_api.rs

core/market/tests/test_app_session_id.rs

Co-authored-by: Piotr Chromiec <tworec@golem.network>

…est case fixes

…d-app-session-id

core/market/src/negotiation/common.rs

tworec · 2020-11-23T14:29:23Z

core/market/src/rest_api.rs

-const DEFAULT_EVENT_TIMEOUT: f32 = 0.0; // seconds
-const DEFAULT_QUERY_TIMEOUT: f32 = 12.0;
+const DEFAULT_EVENT_TIMEOUT: f32 = 5.0; // seconds
+const DEFAULT_QUERY_TIMEOUT: f32 = 5.0;


below you can find examlpe how to handle invalid user input for appSessionId ie. len > 100 chars in this case

core/market/tests/test_app_session_id.rs

nieznanysprawiciel added 25 commits November 2, 2020 20:18

Create database models and dao

fd50f14

Partial implementation of rest endpoint and client Events

5cf4231

Get Agreement events parameters from rest call

e594718

Added database migrations

343ad13

Merge master

ce4756c

Add AgreementApproved events

7765c58

[Test] AgreementApproved event works

e08b6f8

Display Identity rewritten to new trait

580573f

AgreementApproved on provider side

4832a80

[Test] Check if wait_for_approval works simultaneously with events

605e426

[Test] AgreementApproved on Provider side

75715d1

Merge branch 'master' of github.com:golemfactory/yagna into market/ag…

f6b73fc

…reement-events

Clean Agreement events for removed Agreements

4490fef

Merge branch 'master' of github.com:golemfactory/yagna into market/ag…

abc271a

…reement-events

Terminate signature work in progress

4ba96c1

Add reason, terminator and signature to AgreementTerminated model; Pr…

567fa43

…ocedural macro for generating ToSql/FromSql implementation

Change database enums to strings for better readability

89aa18e

Add proposal events reason; Fix database migration

82c6c10

Fix migrations after trying to run them with yagna on real database

b7781a9

Chnage query agreement events endpoint parameters from path to query

e82d8a4

Use ya-client AgreementEvents implementation instead of mock

a0df87d

Change 'Propagating Offers...' log level to debug

9ad8f92

Merge branch 'master' of github.com:golemfactory/yagna into market/ag…

cd96e0b

…reement-events

Fix test compilation after changes in events

76a5383

Simplify matching agreement state

58176cc

nieznanysprawiciel self-assigned this Nov 17, 2020

nieznanysprawiciel added the t-mkt label Nov 17, 2020

nieznanysprawiciel added 2 commits November 17, 2020 14:09

Confirm Agreement endpoint with optional appSessionId

a196069

Add appSessionId in ApproveAgreement endpoint

bf25ce7

nieznanysprawiciel changed the base branch from master to market/agreement-events November 17, 2020 13:30

nieznanysprawiciel removed request for a team and prekucki November 19, 2020 14:44

Remove mock ya_client - it's unnecessary because all changes are in y…

40cd74f

…a-client repo

nieznanysprawiciel force-pushed the market/add-app-session-id branch from 2b14035 to 40cd74f Compare November 19, 2020 14:46

tworec added 2 commits November 20, 2020 13:49

Merge branch 'master' into prekucki/decimal-as-real-decimal

6f4c5fb

Merge branch 'prekucki/decimal-as-real-decimal' into market/add-app-s…

324068a

…ession-id

nieznanysprawiciel requested review from a team and mfranciszkiewicz November 20, 2020 14:55

tworec suggested changes Nov 20, 2020

View reviewed changes

tworec suggested changes Nov 21, 2020

View reviewed changes

Wiezzel previously approved these changes Nov 23, 2020

View reviewed changes

nieznanysprawiciel dismissed stale reviews from Wiezzel via 7e0b5d4 November 23, 2020 11:35

Apply suggestions from code review

7e1d011

Co-authored-by: Piotr Chromiec <tworec@golem.network>

nieznanysprawiciel force-pushed the market/add-app-session-id branch from 7e0b5d4 to 7e1d011 Compare November 23, 2020 11:44

nieznanysprawiciel added 2 commits November 23, 2020 14:38

Review fixes: max events config values; approve_data -> approve_ts; T…

561272a

…est case fixes

Merge branch 'master' of github.com:golemfactory/yagna into market/ad…

e8d8afd

…d-app-session-id

tworec suggested changes Nov 23, 2020

View reviewed changes

nieznanysprawiciel mentioned this pull request Nov 23, 2020

Check AppSessionId length - should be < 100 #819

Open

Final review fixes

bb6c573

nieznanysprawiciel removed request for a team and mfranciszkiewicz November 23, 2020 16:00

tworec approved these changes Nov 23, 2020

View reviewed changes

nieznanysprawiciel merged commit 966e9ea into master Nov 24, 2020

nieznanysprawiciel deleted the market/add-app-session-id branch November 24, 2020 10:11

This was referenced Nov 24, 2020

send warning + original reason when JsonReason parsing fails #820

Closed

decrease Offer TTL to 1h #800

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add AppSessionId #793

Add AppSessionId #793

nieznanysprawiciel commented Nov 17, 2020 •

edited

Loading

tworec left a comment

tworec Nov 20, 2020

nieznanysprawiciel Nov 23, 2020

tworec Nov 23, 2020

tworec Nov 20, 2020

nieznanysprawiciel Nov 23, 2020

tworec Nov 23, 2020

tworec left a comment

tworec Nov 23, 2020

Add AppSessionId #793

Add AppSessionId #793

Conversation

nieznanysprawiciel commented Nov 17, 2020 • edited Loading

tworec left a comment

Choose a reason for hiding this comment

tworec Nov 20, 2020

Choose a reason for hiding this comment

nieznanysprawiciel Nov 23, 2020

Choose a reason for hiding this comment

tworec Nov 23, 2020

Choose a reason for hiding this comment

tworec Nov 20, 2020

Choose a reason for hiding this comment

nieznanysprawiciel Nov 23, 2020

Choose a reason for hiding this comment

tworec Nov 23, 2020

Choose a reason for hiding this comment

tworec left a comment

Choose a reason for hiding this comment

tworec Nov 23, 2020

Choose a reason for hiding this comment

nieznanysprawiciel commented Nov 17, 2020 •

edited

Loading