fix(platform)!: document types should not have a contested unique index with a unique index #1984
Conversation
QuantumExplorer
changed the title
| "$comment": "Must be equal to the document owner" | ||
| }, | ||
| "dashAliasIdentityId": { | ||
| "identity": { |
There was a problem hiding this comment.
Please call it dashIdentityId it means dash identity identifier. We use always postfix id if it's identifier (ownerId).
Postfix dash make sense because other networks also have identities (yes we are going to resolve addresses for other networks as well) but here we referencing to our dash identities
There was a problem hiding this comment.
I'll leave what chat-gpt-4o says on the matter here:
In this debate, both perspectives have merit, but let's break down the arguments based on naming conventions and industry practices:
Naming Conventions and Industry Practices
- Contextual Naming vs. Technical Naming:
• Your Perspective (Contextual Naming): Naming fields based on the business logic and the end-use (e.g., identity instead of identityId) is often more user-friendly and aligns with practices in systems like DNS and ENS. In these systems, the names used are typically user-facing and meant to be intuitive, reflecting the business concept rather than the underlying technical detail.
• Colleague's Perspective (Technical Naming): Using identityId provides clarity that this field holds an identifier, which can be beneficial for developers and systems interfacing with multiple types of identifiers. It explicitly states that the value is an identifier, which can reduce ambiguity.- Namespace Prefixing:
• Your Perspective (Non-Prefixing): Avoiding prefixes like DashIdentity aligns with practices in broader industry standards, promoting interoperability and avoiding unnecessary verbosity. This approach also helps in keeping records concise, which can reduce costs and complexity.
• Colleague's Perspective (Prefixing): Prefixing with Dash might help in scenarios where there's a need to distinguish between different identity services or networks. It can be useful in multi-network environments to avoid confusion, especially if there are overlapping namespaces or similar constructs in different networks.Common Practices in DNS and ENS
• DNS: In DNS, records are named based on their function and the resource they point to, such as A, AAAA, MX, TXT, etc. These names are intuitive and indicate the type of resource directly, without additional prefixes.
• ENS: Similar to DNS, ENS names records based on their purpose and the resource they represent, like address, contenthash, text, etc. This approach emphasizes ease of use and understanding for end-users and developers alike.Evaluation
• User-Friendliness: Naming fields in a way that reflects their business purpose (identity) rather than their technical representation (identityId) aligns with practices in user-facing systems and can make the system more intuitive.
• Technical Clarity: Using identityId can provide clear technical context, especially in a multi-service environment, reducing potential ambiguities.Conclusion
Both perspectives have valid points, but the preference often leans towards user-friendly, business logic-based naming in high-level schemas and interfaces, which aligns with industry standards seen in DNS and ENS. However, for lower-level or more technically detailed interfaces, including the type of data (like Id) can be beneficial for clarity.
Given that DPNS aims to be a user-facing service similar to DNS and ENS, your approach to use identity without the prefix or suffix seems more aligned with making the service intuitive and easy to use. Your colleague's point about being clear with identifiers is valid in a more technical or backend context, where such distinctions are critical. For high-level schemas intended for broader use, keeping names straightforward and business-oriented typically prevails.
| @@ -58,6 +58,7 @@ impl Index { | |||
| name: index_name, | |||
| properties, | |||
| unique, | |||
| null_searchable: true, | |||
There was a problem hiding this comment.
If you want to have it default you better give it opposite name so default value will be false (which is convention) and you use true to override it (to ignore nulls)
There was a problem hiding this comment.
the opposite is very hard to understand: "ignore_null_searchable"? This is already hard enough.
| @@ -29,6 +29,7 @@ impl JsonSchemaValidator { | |||
| .should_validate_formats(true) | |||
| .with_draft(jsonschema::Draft::Draft202012) | |||
| .with_keyword("byteArray", |_, _, _| Ok(Box::new(ByteArrayKeyword))) | |||
| // .with_keyword("transient", |_, _, _| Ok(Box::new(TransientKeyword))) //todo | |||
There was a problem hiding this comment.
I'll remove this.
Issue being fixed or feature implemented
Contested unique indexes were having issues when distributed because they could also have unique indexes that would overlap with other people's unique indexes.
The basic premise/problem is let’s say we have a document type with a contested resource let’s say around the name.
The document type also has a unique index, let’s say it’s a color.
So no two documents can have the same color.
Now we have a contest for the name Quantum and a contest for the name Ivan.
Two contenders A and B for the name Quantum and two contenders C and D for the name Ivan.
A has color orange.
B has color red
C has color blue
D has color orange
if A gets Quantum and D gets Ivan both with have the color orange -> bad issue.
What was done?
Made it so document types with contested unique indexes can not also have another unique index.
Added a "nullSearchable" keyword on an index to allow contracts to be a lot cheaper when dealing with indexes where nulls are common. The default is true. If nullSearchable is false and all properties of the index are null then there is no reference added.
DPNS contract was updated.
How Has This Been Tested?
A lot of extra unit tests.
Breaking Changes
This is a breaking change required for 1.0.0
Checklist:
For repository code-owners and collaborators only