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

Update database schema with more NOT NULL columns #5613

Open
1 task done
htdvisser opened this issue Jul 19, 2022 · 4 comments
Open
1 task done

Update database schema with more NOT NULL columns #5613

htdvisser opened this issue Jul 19, 2022 · 4 comments
Assignees
Labels
c/identity server This is related to the Identity Server compat/db This could affect Database compatibility technical debt Not necessarily broken, but could be done better/cleaner
Milestone

Comments

@htdvisser
Copy link
Contributor

Summary

A number of columns in our database schema allow NULL values when in fact NULLs shoud not be possible. We should fix that.

Current Situation

Many columns (at least the ones marked as nullzero in pkg/identityserver/bunstore, but there are more) allow NULL values that are equivalent to empty values in The Things Stack.

Desired Situation

We should not allow NULLs in those columns. We can set a DEFAULT for INSERT compatibility.

Code of Conduct

@htdvisser htdvisser added c/identity server This is related to the Identity Server compat/db This could affect Database compatibility needs/triage We still need to triage this labels Jul 19, 2022
@NicolasMrad NicolasMrad added technical debt Not necessarily broken, but could be done better/cleaner and removed needs/triage We still need to triage this labels Jul 26, 2022
@NicolasMrad NicolasMrad added this to the 2022 Q4 milestone Jul 26, 2022
@NicolasMrad NicolasMrad modified the milestones: 2022 Q4, 2023 Q1 Jan 2, 2023
@nicholaspcr
Copy link
Contributor

As pointed out by @cvetkovski98 we do have the notnull annotation which creates the restriction on the tables but at the same time we are not using the nullzero which marshals the Go zero values into NULL. Ref: https://bun.uptrace.dev/guide/models.html#struct-tags

Besides this I should write it in here what else shouldn't be NULL in the database.

@NicolasMrad NicolasMrad modified the milestones: 2023 Q1, 2023 Q2 Apr 12, 2023
@adriansmares
Copy link
Contributor

Besides this I should write it in here what else shouldn't be NULL in the database.

Could you take a look for this ? I can then check the production databases for conflicts and we can write the migration and store changes.

@adriansmares adriansmares modified the milestones: 2023 Q2, v3.26.2 May 23, 2023
@nicholaspcr nicholaspcr modified the milestones: v3.26.2, v3.27.0 Jun 27, 2023
@nicholaspcr nicholaspcr modified the milestones: v3.27.0, v3.27.1 Jul 7, 2023
@nicholaspcr nicholaspcr modified the milestones: v3.27.1, v3.27.2 Aug 16, 2023
@nicholaspcr nicholaspcr added the in progress We're working on it label Sep 6, 2023
@KrishnaIyer KrishnaIyer modified the milestones: v3.27.2, v3.28.0 Sep 6, 2023
@nicholaspcr
Copy link
Contributor

The initial idea was to find more columns which would benefit from having more NOT NULL columns. By going through columns which already have the nullzero restriction we can get a good idea where the addition is applicable. Hylke mentioned that there are other fields which would benefit from this addition but since the current list is extensive enough, we can develop on top of them after agreeing on the changes on the fields with nullzero.

I went a bit all over the place while working on this, so there are two sections in this comment:

  1. The fields that have nullzero but would benefit from having notnull
  2. The fields that have notnull but would benefit from having nullzero

Each section is divided in:

  • What columns can have the addition
    • Which of them ensures no NULL or zero values due to request validation and business logic.
    • Which of them probably have NULL or zero values.
  • What columns should not have the addition
  • Some additional questions regarding the topic

Overall most fields which have notnull or nullzero either enforce the value not being null via request validation and business logic or the current behavior is adequate. But for those who require the change, imply in a different migration for each, where we would set the default value and add the constraint to the column on the table.

@adriansmares I left the queries for the fields (just counting amount of NULLs) for each fields which I believe might have the NULL or zero value on production. There are some questions regarding some specific fields which I would appreciate your take.

Notes related to `nullzero`

List of fields that could have notnull added:

Do not have NULL values

This means that adding the notnull restriction should not cause problems

  • pkg/identityserver/bunstore/api_key_store.go: APIKeyID string bun:"api_key_id,nullzero"

    • Uses is.GenerateAPIKey to fill the value, there should be no NULL value
  • pkg/identityserver/bunstore/api_key_store.go: Key string bun:"key,nullzero"

    • Uses is.GenerateAPIKey to fill the value, there should be no NULL value
  • pkg/identityserver/bunstore/api_key_store.go: Rights []int bun:"rights,array,nullzero"

    • message validation ensures at least one right, should never be NULL
  • pkg/networkoperationscenter/store/model/gateway.tti.go: DataRate string bun:",type:varchar(20),nullzero"

  • IDs have regex enforcing a value

    • pkg/identityserver/bunstore/gateway_store.go: BrandID string bun:"brand_id,nullzero"
    • pkg/identityserver/bunstore/gateway_store.go: ModelID string bun:"model_id,nullzero"
  • pkg/identityserver/bunstore/end_device_location_store.go: Service string bun:"service,nullzero"

    • Regex validation makes it so its always non empty
  • pkg/identityserver/bunstore/contact_info_store.go: Reference string bun:"reference,nullzero"

    • Regex validation makes it so its always non empty
    • Reference is the ID
  • pkg/identityserver/bunstore/contact_info_store.go: Token string bun:"token,nullzero"

    • Regex validation makes it so its always non empty
  • pkg/identityserver/bunstore/notification_store.go: Receivers []int bun:"receivers,array,nullzero"

    • validation makes it so its always non empty
Might HAVE NULL values

This means that adding the notnull restriction requires checking on prod if it is truly the case and setting a default if so.

  • pkg/networkoperationscenter/store/model/end_device.tti.go: SessionKeyID []byte bun:",type:bytea,nullzero"

    • It should not but it might have a NULL value since there is no validation for the field.
  • pkg/identityserver/bunstore/user_session_store.go: SessionSecret string bun:"session_secret,nullzero"

    • No validation on making it necessary or specifying minimum value
    • It says it will never be returned but FindSessions does return a list of sessions with the secret included.
      • // The session secret is used to compose an authorization key and is never returned.
  • pkg/identityserver/bunstore/client_store.go: ClientSecret string bun:"client_secret,nullzero"

    • No validation on making it necessary or specifying minimum value
  • pkg/identityserver/bunstore/gateway_store.go: GatewayServerAddress string bun:"gateway_server_address,nullzero"

    • There is validation but was introduced on v3.20, so might have NULL values
  • pkg/identityserver/bunstore/application_store.go: NetworkServerAddress string bun:"network_server_address,nullzero"

    • There is validation but was introduced on v3.20, so might have NULL values
  • pkg/identityserver/bunstore/application_store.go: ApplicationServerAddress string bun:"application_server_address,nullzero"

    • There is validation but was introduced on v3.20, so might have NULL values
  • pkg/identityserver/bunstore/application_store.go: JoinServerAddress string bun:"join_server_address,nullzero"

    • There is validation but was introduced on v3.20, so might have NULL values
  • pkg/identityserver/bunstore/contact_info_store.go: Value string bun:"value,nullzero"

  • pkg/identityserver/bunstore/end_device_store.go: NetworkServerAddress string bun:"network_server_address,nullzero"

    • There is validation but was introduced on v3.20, so might have NULL values
  • pkg/identityserver/bunstore/end_device_store.go: ApplicationServerAddress string bun:"application_server_address,nullzero"

    • There is validation but was introduced on v3.20, so might have NULL values
  • pkg/identityserver/bunstore/end_device_store.go: JoinServerAddress string bun:"join_server_address,nullzero"

    • There is validation but was introduced on v3.20, so might have NULL values
  • pkg/identityserver/bunstore/gateway_store.go: FrequencyPlanID string bun:"frequency_plan_id,nullzero"

    • API doesn't enforce having a frequency plan, just the Console. Possible to create a gateway without one via CLI.

Queries to run on production to see if there are any NULL values:

SELECT COUNT(1) FROM user_sessions WHERE session_secret IS NULL;

SELECT COUNT(1) FROM clients WHERE client_secret IS NULL;

SELECT COUNT(1) FROM gateways WHERE gateway_server_address IS NULL;


SELECT COUNT(1) FROM applications WHERE network_server_address IS NULL;
SELECT COUNT(1) FROM applications WHERE application_server_address IS NULL;
SELECT COUNT(1) FROM applications WHERE join_server_address IS NULL;

SELECT COUNT(1) FROM contact_infos WHERE value IS NULL;

SELECT COUNT(1) FROM end_devices WHERE network_server_address IS NULL;
SELECT COUNT(1) FROM end_devices WHERE application_server_address IS NULL;
SELECT COUNT(1) FROM end_devices WHERE join_server_address IS NULL;

SELECT COUNT(1) FROM gateways WHERE frequency_plan_id IS NULL;

Should NOT have notnull added

  • pkg/identityserver/bunstore/api_key_store.go: Name string bun:"name,nullzero"
  • pkg/identityserver/bunstore/client_store.go: Name string bun:"name,nullzero"
  • pkg/identityserver/bunstore/client_store.go: Description string bun:"description,nullzero"
  • pkg/identityserver/bunstore/client_store.go: RedirectURIs []string bun:"redirect_uris,array,nullzero"
    • I believe redirect_uris are not mandated, so being null is permissable
  • pkg/identityserver/bunstore/client_store.go: LogoutRedirectURIs []string bun:"logout_redirect_uris,array,nullzero"
    • I believe logout_redirect_uris are not mandated, so being null is permissable
  • pkg/identityserver/bunstore/client_store.go: StateDescription string bun:"state_description,nullzero"
  • pkg/identityserver/bunstore/client_store.go: Grants []int bun:"grants,array,nullzero"
    • A bit weird not having any grants but I believe its permitted by design so I don't think we should enforce grants
  • pkg/identityserver/bunstore/client_store.go: Rights []int bun:"rights,array,nullzero"
    • A bit weird not having any rights but I believe its permitted by design so I don't think we should enforce rights
  • pkg/identityserver/bunstore/gateway_store.go: Name string bun:"name,nullzero"
  • pkg/identityserver/bunstore/gateway_store.go: Description string bun:"description,nullzero"
  • pkg/identityserver/bunstore/gateway_store.go: HardwareVersion string bun:"hardware_version,nullzero"
    • I believe we don't enforce this intentionally
  • pkg/identityserver/bunstore/gateway_store.go: FirmwareVersion string bun:"firmware_version,nullzero"
    • I believe we don't enforce this intentionally
  • pkg/identityserver/bunstore/gateway_store.go: UpdateChannel string bun:"update_channel,nullzero"
    • Most likely NULL for most cases
  • pkg/identityserver/bunstore/gateway_store.go: LBSLNSSecret []byte bun:"lbs_lns_secret,nullzero"
  • pkg/identityserver/bunstore/gateway_store.go: TargetCUPSURI string bun:"target_cups_uri,nullzero"
  • ClientAuthorization struct
    • pkg/identityserver/bunstore/oauth_store.go: Rights []int bun:"rights,array,nullzero"
  • AuthorizationCode struct
    • pkg/identityserver/bunstore/oauth_store.go: Rights []int bun:"rights,array,nullzero"
    • pkg/identityserver/bunstore/oauth_store.go: UserSessionID string bun:"user_session_id,nullzero"
    • pkg/identityserver/bunstore/oauth_store.go: RedirectURI string bun:"redirect_uri,nullzero"
    • pkg/identityserver/bunstore/oauth_store.go: State string bun:"state,nullzero"
  • AccessToken struct
    • pkg/identityserver/bunstore/oauth_store.go: UserSessionID string bun:"user_session_id,nullzero"
    • pkg/identityserver/bunstore/oauth_store.go: Rights []int bun:"rights,array,nullzero"
    • pkg/identityserver/bunstore/oauth_store.go: PreviousID string bun:"previous_id,nullzero"
  • pkg/identityserver/bunstore/gateway_antenna_store.go: Gain float32 bun:"gain,nullzero"
  • pkg/identityserver/bunstore/application_store.go: Name string bun:"name,nullzero"
  • pkg/identityserver/bunstore/application_store.go: Description string bun:"description,nullzero"
  • Membership struct
    • pkg/identityserver/bunstore/membership_store.go: Rights []int bun:"rights,array,nullzero"
  • directEntityMembership struct
    • pkg/identityserver/bunstore/membership_store.go: Rights []int bun:"rights,array,nullzero"
  • indirectEntityMembership struct
    • pkg/identityserver/bunstore/membership_store.go: UserRights []int bun:"user_rights,array,nullzero"
    • pkg/identityserver/bunstore/membership_store.go: EntityRights []int bun:"entity_rights,array,nullzero"
  • pkg/identityserver/bunstore/authentication_provider_store.tti.go: Name string bun:"name,nullzero"
  • pkg/identityserver/bunstore/notification_store.go: Data json.RawMessage bun:"data,nullzero"
  • pkg/identityserver/bunstore/organization_store.go: Name string bun:"name,nullzero"
  • pkg/identityserver/bunstore/organization_store.go: Description string bun:"description,nullzero"
  • pkg/identityserver/bunstore/tenant_store.tti.go: Name string bun:"name,nullzero"
  • pkg/identityserver/bunstore/tenant_store.tti.go: Description string bun:"description,nullzero"
  • pkg/identityserver/bunstore/tenant_store.tti.go: StateDescription string bun:"state_description,nullzero"
  • pkg/identityserver/bunstore/user_store.go: Name string bun:"name,nullzero"
  • pkg/identityserver/bunstore/user_store.go: Description string bun:"description,nullzero"
  • pkg/identityserver/bunstore/user_store.go: StateDescription string bun:"state_description,nullzero"
  • pkg/identityserver/bunstore/user_store.go: TemporaryPassword string bun:"temporary_password,nullzero"
  • pkg/identityserver/bunstore/end_device_store.go: Name string bun:"name,nullzero"
  • pkg/identityserver/bunstore/end_device_store.go: Description string bun:"description,nullzero"
  • pkg/identityserver/bunstore/end_device_store.go: BrandID string bun:"brand_id,nullzero"
  • pkg/identityserver/bunstore/end_device_store.go: ModelID string bun:"model_id,nullzero"
  • pkg/identityserver/bunstore/end_device_store.go: HardwareVersion string bun:"hardware_version,nullzero"
  • pkg/identityserver/bunstore/end_device_store.go: FirmwareVersion string bun:"firmware_version,nullzero"
  • pkg/identityserver/bunstore/end_device_store.go: BandID string bun:"band_id,nullzero"
  • pkg/identityserver/bunstore/end_device_store.go: SerialNumber string bun:"serial_number,nullzero"
  • pkg/identityserver/bunstore/end_device_store.go: ServiceProfileID string bun:"service_profile_id,nullzero"
  • pkg/identityserver/bunstore/end_device_store.go: VendorID uint32 bun:"vendor_id,nullzero"
  • pkg/identityserver/bunstore/end_device_store.go: VendorProfileID uint32 bun:"vendor_profile_id,nullzero"

Should it have nullzero?

  • pkg/identityserver/bunstore/location.go: Latitude float64 bun:"latitude,nullzero"
    • zero value is a valid option, should it have nullzero?
  • pkg/identityserver/bunstore/location.go: Longitude float64 bun:"longitude,nullzero"
    • zero value is a valid option, should it have nullzero?
  • pkg/identityserver/bunstore/location.go: Altitude int32 bun:"altitude,nullzero"
    • zero value is a valid option, should it have nullzero?
  • pkg/identityserver/bunstore/location.go: Accuracy int32 bun:"accuracy,nullzero"
    • zero value is a valid option, should it have nullzero?
  • pkg/identityserver/bunstore/gateway_store.go: DownlinkPathConstraint int bun:"downlink_path_constraint,nullzero"
    • zero value is a valid option, should it have nullzero?
    // Indicates that the gateway can be selected for downlink without constraints by the Network Server.
    DownlinkPathConstraint_DOWNLINK_PATH_CONSTRAINT_NONE DownlinkPathConstraint = 0
  • pkg/identityserver/bunstore/gateway_store.go: RequireAuthenticatedConnection bool bun:"require_authenticated_connection,nullzero"
    • zero value is a valid option, should it have nullzero?
  • pkg/identityserver/bunstore/login_token_store.go: Used bool bun:"used,nullzero"
    • zero value is a valid option, should it have nullzero?
  • pkg/identityserver/bunstore/gateway_antenna_store.go: Placement int bun:"placement,nullzero"
    • zero value is a valid option, should it have nullzero?
  • pkg/identityserver/bunstore/application_store.go: DevEUICounter int bun:"dev_eui_counter,nullzero"
    • zero value is a valid option, should it have nullzero?
  • pkg/identityserver/bunstore/contact_info_store.go: Used bool bun:"used,nullzero"
    • zero value is a valid option, should it have nullzero?
  • pkg/identityserver/bunstore/authentication_provider_store.tti.go: AllowRegistrations bool bun:"allow_registrations,nullzero"
    • zero value is a valid option, should it have nullzero?
Notes related to `notnull`

In addition to that I took the liberty to check places where notnull was already added but nullzero was not present, meaning that we possibly have zero values on the database instead of NULL and the restriction initially imposed is not applied properly.

List of fields that could have nullzero added:

Do not have zero values values

The items below don't have zero values, this is mostly enforced by either business logic on the registry or a validation on the request message.

  • identityserver/bunstore/notification_store.go: NotificationType bun:"notification_type,notnull"
  • identityserver/bunstore/login_token_store.go: Token bun:"token,notnull"
  • identityserver/bunstore/attribute_store.go: Key bun:"key,notnull"
  • jdentityserver/bunstore/attribute_store.go: Value bun:"value,notnull"
  • identityserver/bunstore/user_store.go: PrimaryEmailAddress bun:"primary_email_address,notnull"
  • identityserver/bunstore/user_store.go: Password bun:"password,notnull"
  • identityserver/bunstore/account_store.go: AccountType bun:"account_type,notnull" // user or organization
  • EmbeddedAccount struct
    • identityserver/bunstore/membership_store.go: AccountType bun:"account_type,notnull"
  • ContactInfo struct
    • identityserver/bunstore/contact_info_store.go: Value bun:"value,notnull"
  • ContactInfoValidation struct
    • identityserver/bunstore/invitation_store.go: Email bun:"email,notnull"
  • identityserver/bunstore/eui_store.go: StartEUI bun:"start_eui,notnull"
  • identityserver/bunstore/eui_store.go: EndCounter bun:"end_counter,notnull"
  • identityserver/bunstore/oauth_store.go: Code bun:"code,notnull"
    • Code in this case is a string
  • identityserver/bunstore/oauth_store.go: AccessToken bun:"access_token,notnull"
  • identityserver/bunstore/oauth_store.go: RefreshToken bun:"refresh_token,notnull"
  • applicationserver/io/packages/storage/bunstore/model.tti.go: Timestamp bun:"timestamp,notnull,type:timestamptz"
  • identityserver/bunstore/invitation_store.go: Token bun:"token,notnull"
  • pkg/identityserver/bunstore/contact_info_store.go: Reference string bun:"reference,nullzero"
    • ContactInfoValidation struct
    • Regex validation makes it so its always non empty
  • pkg/identityserver/bunstore/contact_info_store.go: Token string bun:"token,nullzero"
    • Regex validation makes it so its always non empty
  • identityserver/bunstore/authentication_provider_store.tti.go: ProviderID bun:"provider_id,notnull"
    • Regex validation enforces minimum size of 2
  • identityserver/bunstore/authentication_provider_store.tti.go: Configuration bun:"configuration,notnull"
    • OneOf and required field

OperationAt is never be zero value:

  • identityserver/bunstore/account_store.go: CreatedAt bun:"created_at,notnull,scanonly"
  • identityserver/bunstore/account_store.go: UpdatedAt bun:"updated_at,notnull,scanonly"
  • identityserver/bunstore/user_store.go: PasswordUpdatedAt bun:"password_updated_at,notnull"
  • identityserver/bunstore/notification_store.go: StatusUpdatedAt bun:"status_updated_at,notnull"
  • identityserver/bunstore/model.go: CreatedAt bun:"created_at,notnull"
  • identityserver/bunstore/model.go: UpdatedAt bun:"updated_at,notnull"

Entity Types require an existing entity, so it is always filled

  • identityserver/bunstore/contact_info_store.go: EntityType bun:"entity_type,notnull"
  • identityserver/bunstore/contact_info_store.go: EntityType bun:"entity_type,notnull"
  • identityserver/bunstore/membership_store.go: EntityType bun:"entity_type,notnull"
  • identityserver/bunstore/membership_store.go: EntityType bun:"entity_type,notnull"
  • identityserver/bunstore/membership_store.go: EntityType bun:"entity_type,notnull"
  • identityserver/bunstore/attribute_store.go: EntityType bun:"entity_type,notnull"
  • identityserver/bunstore/notification_store.go: EntityType bun:"entity_type,notnull"
  • identityserver/bunstore/api_key_store.go: EntityType bun:"entity_type,notnull"

Just usage of entity's ID, so it would result in error before writing to the DB

  • applicationserver/io/packages/storage/bunstore/model.tti.go: DeviceID bun:"device_id,notnull,type:varchar(36)"
  • applicationserver/io/packages/storage/bunstore/model.tti.go: ApplicationID bun:"application_id,notnull,type:varchar(36)"
  • applicationserver/io/packages/storage/bunstore/model.tti.go: TenantID bun:"tenant_id,notnull,type:varchar(36)"
  • identityserver/bunstore/oauth_store.go: TokenID bun:"token_id,notnull"
  • identityserver/bunstore/oauth_store.go: UserID bun:"user_id,notnull"
  • identityserver/bunstore/oauth_store.go: ClientID bun:"client_id,notnull"
  • identityserver/bunstore/oauth_store.go: UserID bun:"user_id,notnull"
  • identityserver/bunstore/oauth_store.go: ClientID bun:"client_id,notnull"
  • identityserver/bunstore/oauth_store.go: UserID bun:"user_id,notnull"
  • identityserver/bunstore/oauth_store.go: ClientID bun:"client_id,notnull"
  • identityserver/bunstore/contact_info_store.go: EntityID bun:"entity_id,notnull"
  • identityserver/bunstore/contact_info_store.go: EntityID bun:"entity_id,notnull"
  • identityserver/bunstore/tenant_store.tti.go: TenantID bun:"tenant_id,notnull"
  • identityserver/bunstore/membership_store.go: EntityFriendlyID bun:"entity_friendly_id,notnull"
  • identityserver/bunstore/membership_store.go: EntityID bun:"entity_id,notnull"
  • identityserver/bunstore/membership_store.go: OrganizationAccountFriendlyID bun:"organization_account_friendly_id,notnull"
  • identityserver/bunstore/membership_store.go: OrganizationAccountID bun:"organization_account_id,notnull"
  • identityserver/bunstore/membership_store.go: UserAccountFriendlyID bun:"user_account_friendly_id,notnull"
  • identityserver/bunstore/membership_store.go: UserAccountID bun:"user_account_id,notnull"
  • identityserver/bunstore/membership_store.go: EntityFriendlyID bun:"entity_friendly_id,notnull"
  • identityserver/bunstore/membership_store.go: EntityID bun:"entity_id,notnull"
  • identityserver/bunstore/membership_store.go: AccountFriendlyID bun:"account_friendly_id,notnull"
  • identityserver/bunstore/membership_store.go: AccountID bun:"account_id,notnull"
  • identityserver/bunstore/membership_store.go: EntityID bun:"entity_id,notnull"
  • identityserver/bunstore/membership_store.go: AccountID bun:"account_id,notnull"
  • identityserver/bunstore/account_store.go: UID bun:"uid,notnull,scanonly"
  • identityserver/bunstore/account_store.go: TenantID bun:"tenant_id,notnull,scanonly"
  • identityserver/bunstore/account_store.go: ID bun:"id,notnull,scanonly"
  • identityserver/bunstore/external_user_store.tti.go: AuthenticationProviderID bun:"authentication_provider_id,notnull"
  • identityserver/bunstore/external_user_store.tti.go: UserID bun:"user_id,notnull"
  • identityserver/bunstore/account_store.go: AccountID bun:"account_id,notnull"
  • identityserver/bunstore/account_store.go: UID bun:"uid,notnull"
  • identityserver/bunstore/end_device_location_store.go: EndDeviceID bun:"end_device_id,notnull"
  • identityserver/bunstore/end_device_store.go: DeviceID bun:"device_id,notnull"
  • identityserver/bunstore/end_device_store.go: ApplicationID bun:"application_id,notnull"
  • identityserver/bunstore/gateway_antenna_store.go: GatewayID bun:"gateway_id,notnull"
  • identityserver/bunstore/attribute_store.go: EntityID bun:"entity_id,notnull"
  • identityserver/bunstore/notification_store.go: ReceiverID bun:"receiver_id,notnull"
  • identityserver/bunstore/notification_store.go: NotificationID bun:"notification_id,notnull"
  • identityserver/bunstore/notification_store.go: SenderUID bun:"sender_uid,notnull"
  • identityserver/bunstore/notification_store.go: EntityUID bun:"entity_uid,notnull"
  • identityserver/bunstore/notification_store.go: EntityID bun:"entity_id,notnull"
  • identityserver/bunstore/user_session_store.go: UserID bun:"user_id,notnull"
  • identityserver/bunstore/api_key_store.go: EntityID bun:"entity_id,notnull"
  • identityserver/bunstore/external_user_store.tti.go: ExternalID bun:"external_id,notnull"
  • identityserver/bunstore/model.tti.go: TenantID bun:"tenant_id,notnull"
  • identityserver/bunstore/client_store.go: ClientID bun:"client_id,notnull"
  • identityserver/bunstore/application_store.go: ApplicationID bun:"application_id,notnull"
  • identityserver/bunstore/gateway_store.go: GatewayID bun:"gateway_id,notnull"
  • identityserver/bunstore/model.go: ID bun:"id,type:uuid,pk,notnull,default:gen_random_uuid()"
Might have the Zero values on prod
  • identityserver/bunstore/eui_store.go: Type bun:"type,notnull"
    • Allows for null values where it shouldn't
  • applicationserver/io/packages/storage/bunstore/model.tti.go: Type bun:"type,notnull,type:varchar(36)"
    • Proto definition does not contain validation around it
SELECT COUNT(1) FROM eui_blocks WHERE type='';

# ApplicationServer DB
SELECT COUNT(1) FROM stored_application_ups WHERE type='';

Should not have the nullzero tag

The list bellow are of fields which the zero value is an appropriate value, mostly are boolean fields:

  • identityserver/bunstore/authentication_provider_store.tti.go: Name string bun:"name,nullzero"
  • identityserver/bunstore/client_store.go: Endorsed bun:"endorsed,notnull"
  • identityserver/bunstore/client_store.go: State bun:"state,notnull"
  • identityserver/bunstore/client_store.go: SkipAuthorization bun:"skip_authorization,notnull"
  • identityserver/bunstore/gateway_store.go: AutoUpdate bun:"auto_update,notnull"
  • identityserver/bunstore/gateway_store.go: StatusPublic bun:"status_public,notnull"
  • identityserver/bunstore/gateway_store.go: LocationPublic bun:"location_public,notnull"
  • identityserver/bunstore/gateway_store.go: ScheduleDownlinkLate bun:"schedule_downlink_late,notnull"
  • identityserver/bunstore/gateway_store.go: EnforceDutyCycle bun:"enforce_duty_cycle,notnull"
  • identityserver/bunstore/gateway_store.go: UpdateLocationFromStatus bun:"update_location_from_status,notnull"
  • identityserver/bunstore/gateway_store.go: SupportsLRFHSS bun:"supports_lrfhss,notnull"
  • identityserver/bunstore/gateway_store.go: DisablePacketBrokerForwarding bun:"disable_packet_broker_forwarding,notnull"
  • identityserver/bunstore/notification_store.go: Email bun:"email,notnull"
  • identityserver/bunstore/notification_store.go: Status bun:"status,notnull"
  • identityserver/bunstore/tenant_store.tti.go: State bun:"state,notnull"
  • identityserver/bunstore/user_store.go: State bun:"state,notnull"
  • identityserver/bunstore/user_store.go: RequirePasswordUpdate bun:"require_password_update,notnull"
  • identityserver/bunstore/user_store.go: Admin bun:"admin,notnull"
  • ContactInfo struct
    • identityserver/bunstore/contact_info_store.go: ContactType bun:"contact_type,notnull"
    • identityserver/bunstore/contact_info_store.go: ContactMethod bun:"contact_method,notnull"
  • ContactInfoValidation struct
    • identityserver/bunstore/contact_info_store.go: ContactMethod bun:"contact_method,notnull"
    • identityserver/bunstore/eui_store.go: CurrentCounter bun:"current_counter,notnull"

Question regarding certain notnull fields

  • identityserver/bunstore/gateway_store.go: ScheduleAnytimeDelay int64 bun:"schedule_anytime_delay,notnull"
    • Does ScheduleAnytimeDelay gets invalidated if set to zero?

And lastly these lists are useful for asking questions regarding specific fields but their main purpose is also to serve as a record of things to change. This way the issue can be fragmented into smaller PRs. While it these changes have to be merged on a minor release I believe it is important to not attempt to do everything at once, specially considering the overhead generated by a mistake on the identification in one of these columns behavior. Will break the list into sections done by minor releases but I'm posting this in order to have a discussion around the questions that appeared.

@KrishnaIyer KrishnaIyer modified the milestones: v3.28.0, v3.29.0 Sep 27, 2023
@nicholaspcr
Copy link
Contributor

This requires a migration, therefore I'm moving it to the v3.29.0

@nicholaspcr nicholaspcr modified the milestones: v3.28.2, v3.29.0 Oct 18, 2023
@nicholaspcr nicholaspcr removed the in progress We're working on it label Dec 21, 2023
@KrishnaIyer KrishnaIyer modified the milestones: v3.29.0, v3.30.0 Jan 18, 2024
@nicholaspcr nicholaspcr modified the milestones: v3.30.0, Backlog Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c/identity server This is related to the Identity Server compat/db This could affect Database compatibility technical debt Not necessarily broken, but could be done better/cleaner
Projects
None yet
Development

No branches or pull requests

5 participants