Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FOGL-7274: Asset Tracker not able to cope with assets that include sp… #911

Draft
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

Innovative-ashwin
Copy link
Contributor

…ecial characters

Signed-off-by: Innovative-ashwin [email protected]

Copy link
Contributor

@MarkRiddoch MarkRiddoch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - I assumed it would be simply a case of missing escape calls somewhere between the service and the database insert.

@praveen-garg
Copy link
Member

@Innovative-ashwin with above changes ingestion is good, but error in

Browse assets readings
Screenshot 2022-12-23 at 5 22 20 PM

Dec 23 17:22:13 Fledge Storage[16206]: ERROR: SQLite3 storage plugin raising error: near ""oid"": syntax error

Following are good though,

Stats history dashboard
South page

Screenshot 2022-12-23 at 5 22 11 PM

Screenshot 2022-12-23 at 5 21 58 PM

/cc @MarkRiddoch

Signed-off-by: Innovative-ashwin <[email protected]>
Merge branch 'FOGL-7274_new' of https://github.com/fledge-iot/fledge into FOGL-7274_new
@@ -1614,7 +1614,7 @@ vector<string> asset_codes;
sql_cmd_base += ", asset_code";

sql_cmd_base += ", id, reading, user_ts, ts ";
StringReplaceAll (sql_cmd_base, "asset_code", " \"_assetcode_\" .assetcode. ");
StringReplaceAll (sql_cmd_base, "asset_code", " '_assetcode_' .assetcode. ");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is causing issues now with ''.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please test for following:

sin"1
sin"1"
sin'1
sin'1'
sin "#1'
sin :."',."''""`/1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

failed for sin"1, sin"1", sin'1

@praveen-garg
Copy link
Member

@Innovative-ashwin Note that with commit 3e627b7

SQLIteLB is all good.
Postgres fails only for Sin’ x
SQLite is not good, given the reading catalogue usage in assets browsing query.

@praveen-garg praveen-garg requested a review from gnandan December 25, 2022 15:12
Signed-off-by: Innovative-ashwin <[email protected]>
Signed-off-by: Innovative-ashwin <[email protected]>
Signed-off-by: Innovative-ashwin <[email protected]>
@praveen-garg
Copy link
Member

@Innovative-ashwin with the newest commit 9edfd59

sin"1 appears as 'sin"1' in assets and readings page; hence no graph rendered. Dashboard/Stats history and South page is good. (Same problem with used sin"1"2)

sin'1
sin'1'2
both are okay.

sin1'2" does not work; Asset and reading page breaks with storage plugin errors.

This is for SQLite plugin.
Please test all per last comment for all storage plugins.

@praveen-garg
Copy link
Member

@Innovative-ashwin Note that with commit 3e627b7

SQLIteLB is all good.

Postgres fails only for Sin’ x

SQLite is not good, given the reading catalogue usage in assets browsing query.

@Innovative-ashwin revert to this commit and take care of the reported issues please!

@praveen-garg
Copy link
Member

closed by #970

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants