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.

Sign up for GitHub

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

Jump to bottom

Use native unicode data types for notification templates #6078

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Use native unicode data types for notification templates #6078

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1894d41
Update MSSQL notification templates content columns to support unicode
darshanasbg Oct 3, 2024
65355b3
Update schema to support unicode for notification templates
darshanasbg Oct 28, 2024
4041f72
Add config to indicate the support of unicode datatypes
darshanasbg Nov 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...es/identity-core/org.wso2.carbon.identity.core.server.feature/resources/dbscripts/db2.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2032,7 +2032,7 @@ CREATE TABLE IDN_NOTIFICATION_ORG_TEMPLATE (
LOCALE VARCHAR(50) NOT NULL,
SUBJECT VARCHAR(4000),
BODY CLOB,
FOOTER CLOB,
FOOTER VARCHAR(4000),
CONTENT_TYPE VARCHAR(50),
TYPE_ID INTEGER NOT NULL,
TENANT_ID INTEGER NOT NULL,
Expand All @@ -2057,7 +2057,7 @@ CREATE TABLE IDN_NOTIFICATION_APP_TEMPLATE (
LOCALE VARCHAR(50) NOT NULL,
SUBJECT VARCHAR(4000),
BODY CLOB,
FOOTER CLOB,
FOOTER VARCHAR(4000),
CONTENT_TYPE VARCHAR(50),
TYPE_ID INTEGER NOT NULL,
APP_ID VARCHAR(255) NOT NULL,
Expand Down
8 changes: 4 additions & 4 deletions ...res/identity-core/org.wso2.carbon.identity.core.server.feature/resources/dbscripts/h2.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1328,8 +1328,8 @@ CREATE TABLE IF NOT EXISTS IDN_NOTIFICATION_ORG_TEMPLATE (
TEMPLATE_KEY VARCHAR(50) NOT NULL,
LOCALE VARCHAR(50) NOT NULL,
SUBJECT VARCHAR(4000),
BODY MEDIUMTEXT,
FOOTER MEDIUMTEXT,
BODY TEXT,
FOOTER VARCHAR(4000),
CONTENT_TYPE VARCHAR(50),
TYPE_ID INTEGER NOT NULL,
TENANT_ID INTEGER NOT NULL,
Expand All @@ -1344,8 +1344,8 @@ CREATE TABLE IF NOT EXISTS IDN_NOTIFICATION_APP_TEMPLATE (
TEMPLATE_KEY VARCHAR(50) NOT NULL,
LOCALE VARCHAR(50) NOT NULL,
SUBJECT VARCHAR(4000),
BODY MEDIUMTEXT,
FOOTER MEDIUMTEXT,
BODY TEXT,
FOOTER VARCHAR(4000),
CONTENT_TYPE VARCHAR(50),
TYPE_ID INTEGER NOT NULL,
APP_ID VARCHAR(255) NOT NULL,
Expand Down
12 changes: 6 additions & 6 deletions .../identity-core/org.wso2.carbon.identity.core.server.feature/resources/dbscripts/mssql.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1473,9 +1473,9 @@ CREATE TABLE IDN_NOTIFICATION_ORG_TEMPLATE (
ID INTEGER IDENTITY,
TEMPLATE_KEY VARCHAR(50) NOT NULL,
LOCALE VARCHAR(50) NOT NULL,
SUBJECT VARCHAR(4000),
BODY TEXT,
FOOTER TEXT,
SUBJECT NVARCHAR(4000),
BODY NVARCHAR(MAX),
Copy link

Choose a reason for hiding this comment

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

Cannot we consider VARBINARY here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

VARBINARY is used to store binary data.. Here we are handling unicode text.. While we can technically use VARBINARY to store this data in the binary format, wouldn't it better to use more native datatype which doesn't require additional binary-text conversion?

Also, can you share what's the problem we can solve, if we are to changing the datatype from NVARCHAR to VARBINARY?

Copy link

@inthirakumaaran inthirakumaaran Oct 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

You're correct; if we only need to support Unicode text (+ HTML), then using NVARCHAR should be sufficient.

FOOTER NVARCHAR(4000),
CONTENT_TYPE VARCHAR(50),
TYPE_ID INTEGER NOT NULL,
TENANT_ID INTEGER NOT NULL,
Expand All @@ -1490,9 +1490,9 @@ CREATE TABLE IDN_NOTIFICATION_APP_TEMPLATE (
ID INTEGER IDENTITY,
TEMPLATE_KEY VARCHAR(50) NOT NULL,
LOCALE VARCHAR(50) NOT NULL,
SUBJECT VARCHAR(4000),
BODY TEXT,
FOOTER TEXT,
SUBJECT NVARCHAR(4000),
BODY NVARCHAR(MAX),
FOOTER NVARCHAR(4000),
CONTENT_TYPE VARCHAR(50),
TYPE_ID INTEGER NOT NULL,
APP_ID VARCHAR(255) NOT NULL,
Expand Down
12 changes: 6 additions & 6 deletions ...y-core/org.wso2.carbon.identity.core.server.feature/resources/dbscripts/mysql-cluster.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1490,9 +1490,9 @@ CREATE TABLE IF NOT EXISTS IDN_NOTIFICATION_ORG_TEMPLATE (
ID INTEGER NOT NULL AUTO_INCREMENT,
TEMPLATE_KEY VARCHAR(50) NOT NULL,
LOCALE VARCHAR(50) NOT NULL,
SUBJECT VARCHAR(4000),
BODY MEDIUMTEXT,
FOOTER MEDIUMTEXT,
SUBJECT VARCHAR(4000) CHARACTER SET utf8mb4,
BODY MEDIUMTEXT CHARACTER SET utf8mb4,
FOOTER VARCHAR(4000) CHARACTER SET utf8mb4,
CONTENT_TYPE VARCHAR(50),
TYPE_ID INTEGER NOT NULL,
TENANT_ID INTEGER NOT NULL,
Expand All @@ -1506,9 +1506,9 @@ CREATE TABLE IF NOT EXISTS IDN_NOTIFICATION_APP_TEMPLATE (
ID INTEGER NOT NULL AUTO_INCREMENT,
TEMPLATE_KEY VARCHAR(50) NOT NULL,
LOCALE VARCHAR(50) NOT NULL,
SUBJECT VARCHAR(4000),
BODY MEDIUMTEXT,
FOOTER MEDIUMTEXT,
SUBJECT VARCHAR(4000) CHARACTER SET utf8mb4,
BODY MEDIUMTEXT CHARACTER SET utf8mb4,
FOOTER VARCHAR(4000) CHARACTER SET utf8mb4,
CONTENT_TYPE VARCHAR(50),
TYPE_ID INTEGER NOT NULL,
APP_ID VARCHAR(255) NOT NULL,
Expand Down
12 changes: 6 additions & 6 deletions .../identity-core/org.wso2.carbon.identity.core.server.feature/resources/dbscripts/mysql.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1358,9 +1358,9 @@ CREATE TABLE IF NOT EXISTS IDN_NOTIFICATION_ORG_TEMPLATE (
ID INTEGER NOT NULL AUTO_INCREMENT,
TEMPLATE_KEY VARCHAR(50) NOT NULL,
LOCALE VARCHAR(50) NOT NULL,
SUBJECT VARCHAR(4000),
BODY MEDIUMTEXT,
FOOTER MEDIUMTEXT,
SUBJECT VARCHAR(4000) CHARACTER SET utf8mb4,
BODY MEDIUMTEXT CHARACTER SET utf8mb4,
FOOTER VARCHAR(4000) CHARACTER SET utf8mb4,
CONTENT_TYPE VARCHAR(50),
TYPE_ID INTEGER NOT NULL,
TENANT_ID INTEGER NOT NULL,
Expand All @@ -1374,9 +1374,9 @@ CREATE TABLE IF NOT EXISTS IDN_NOTIFICATION_APP_TEMPLATE (
ID INTEGER NOT NULL AUTO_INCREMENT,
TEMPLATE_KEY VARCHAR(50) NOT NULL,
LOCALE VARCHAR(50) NOT NULL,
SUBJECT VARCHAR(4000),
BODY MEDIUMTEXT,
FOOTER MEDIUMTEXT,
SUBJECT VARCHAR(4000) CHARACTER SET utf8mb4,
BODY MEDIUMTEXT CHARACTER SET utf8mb4,
FOOTER VARCHAR(4000) CHARACTER SET utf8mb4,
CONTENT_TYPE VARCHAR(50),
TYPE_ID INTEGER NOT NULL,
APP_ID VARCHAR(255) NOT NULL,
Expand Down
12 changes: 6 additions & 6 deletions ...identity-core/org.wso2.carbon.identity.core.server.feature/resources/dbscripts/oracle.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2088,9 +2088,9 @@ CREATE TABLE IDN_NOTIFICATION_ORG_TEMPLATE (
ID INTEGER NOT NULL,
TEMPLATE_KEY VARCHAR(50) NOT NULL,
LOCALE VARCHAR(50) NOT NULL,
SUBJECT VARCHAR(4000),
BODY CLOB,
FOOTER CLOB,
SUBJECT NVARCHAR2(4000),
BODY NCLOB,
FOOTER NVARCHAR2(4000),
CONTENT_TYPE VARCHAR(50),
TYPE_ID INTEGER NOT NULL,
TENANT_ID INTEGER NOT NULL,
Expand All @@ -2115,9 +2115,9 @@ CREATE TABLE IDN_NOTIFICATION_APP_TEMPLATE (
ID INTEGER NOT NULL,
TEMPLATE_KEY VARCHAR(50) NOT NULL,
LOCALE VARCHAR(50) NOT NULL,
SUBJECT VARCHAR(4000),
BODY CLOB,
FOOTER CLOB,
SUBJECT NVARCHAR2(4000),
BODY NCLOB,
FOOTER NVARCHAR2(4000),
CONTENT_TYPE VARCHAR(50),
TYPE_ID INTEGER NOT NULL,
APP_ID VARCHAR(255) NOT NULL,
Expand Down
12 changes: 6 additions & 6 deletions ...tity-core/org.wso2.carbon.identity.core.server.feature/resources/dbscripts/oracle_rac.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2021,9 +2021,9 @@ CREATE TABLE IDN_NOTIFICATION_ORG_TEMPLATE (
ID INTEGER NOT NULL,
TEMPLATE_KEY VARCHAR(50) NOT NULL,
LOCALE VARCHAR(50) NOT NULL,
SUBJECT VARCHAR(4000),
BODY CLOB,
FOOTER CLOB,
SUBJECT NVARCHAR2(4000),
BODY NCLOB,
FOOTER NVARCHAR2(4000),
CONTENT_TYPE VARCHAR(50),
TYPE_ID INTEGER NOT NULL,
TENANT_ID INTEGER NOT NULL,
Expand All @@ -2048,9 +2048,9 @@ CREATE TABLE IDN_NOTIFICATION_APP_TEMPLATE (
ID INTEGER NOT NULL,
TEMPLATE_KEY VARCHAR(50) NOT NULL,
LOCALE VARCHAR(50) NOT NULL,
SUBJECT VARCHAR(4000),
BODY CLOB,
FOOTER CLOB,
SUBJECT NVARCHAR2(4000),
BODY NCLOB,
FOOTER NVARCHAR2(4000),
CONTENT_TYPE VARCHAR(50),
TYPE_ID INTEGER NOT NULL,
APP_ID VARCHAR(255) NOT NULL,
Expand Down
5 changes: 5 additions & 0 deletions features/identity-core/org.wso2.carbon.identity.core.server.feature/resources/identity.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@
<!-- <KeyStores>database</KeyStores>-->
</DataStorageType>

<NotificationTemplates>
<!-- Indicates whether the database uses native Unicode data types (e.g., NVARCHAR, NCHAR) -->
<UseUnicodeDataTypes>false</UseUnicodeDataTypes>
</NotificationTemplates>

<!-- Time configurations are in minutes -->
<TimeConfig>
<SessionIdleTimeout>15</SessionIdleTimeout>
Expand Down
2 changes: 2 additions & 0 deletions ...ures/identity-core/org.wso2.carbon.identity.core.server.feature/resources/identity.xml.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@
<SMSTemplates>
<Apply>{{notification_templates.sms_templates.apply}}</Apply>
</SMSTemplates>
<!-- Indicates whether the database uses native Unicode data types (e.g., NVARCHAR, NCHAR) -->
<UseUnicodeDataTypes>{{notification_templates.use_unicode_data_types}}</UseUnicodeDataTypes>
</NotificationTemplates>

<!-- Time configurations are in minutes -->
Expand Down
2 changes: 2 additions & 0 deletions ...y.core.server.feature/resources/org.wso2.carbon.identity.core.server.feature.default.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@
"remote_fetch.enable": false,
"remote_fetch.working_directory": "${carbon.home}/tmp/",

"notification_templates.use_unicode_data_types": false,

"oauth.token_cleanup.enable": true,
"oauth.token_cleanup.retain_access_tokens_for_auditing": false,

Expand Down
Loading