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

serial (auto incremented) Id and deletedAt in BaseEntity #73

Open
m-salman-afzal opened this issue Nov 8, 2024 · 3 comments
Open

serial (auto incremented) Id and deletedAt in BaseEntity #73

m-salman-afzal opened this issue Nov 8, 2024 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@m-salman-afzal
Copy link

  • We often require an auto incremented id or sId that needs to be displayed on the front-end for user experience purposes and it is good to have this id for all our models.
  • In a business, we never hard delete anything, no matter how small it is. So, it makes sense that we have a soft delete column deletedAt like createdAt and updatedAt, that will be just need to be updated by the current date while deleting and we can restore that row whenever we need by just setting the same column to null.

Although, it will require additional work on developer side to make sure that while querying the data, is mode.deletedAt IS NULL filter is applied in where filter or not. Some ORMS like typeORM, Sequelize, Prisma provides auto check of deletedAt is null but for the case of drizzleORM, we'll have to manually check for the deletedAt column, but its a simple enough task.

@m-salman-afzal m-salman-afzal self-assigned this Nov 8, 2024
@m-salman-afzal m-salman-afzal added the enhancement New feature or request label Nov 8, 2024
@volf52
Copy link
Collaborator

volf52 commented Nov 11, 2024

Serial IDs, though easy to start with, weren't built into hexapp as UUIDs (or the "new kids" GUID/CUID/NanoID) are much easier to work with in a distributed context. Serial IDs introduce an implicit dependency on the database to generate the primary key, whereas UUIDs can be generated on the server side without having to rely on the database. This is extremely useful in cases where you are generating multiple entities in a workflow where some entities refer to some other entities in a sequence of operations. With serial ids, you'd have to tie in the repository calls with the entity creation (as you cannot get the ID without the database call), whereas with UUIDs, you can generate the ID on the server and leave the persistence calls till the end (or even push them to the background job queue if required). This also makes working with third party APIs easier in cases where you want a set of records to be generated asynchronously without being tied to the request/response cycle, and persist the records only upon successful processing by the third party platform.
The second biggest benefit of UUIDs I have experienced (or rather, the flaw of serial IDs I had to encounter) is how easy they make migrating and merging databases that are out of sync with each other. With serial IDs, especially in the absence of any other primary key, you cannot just copy over the records from one database to the other as the ID column will create conflicts. The only thing you can do is to add them as new records, which also means that you have to go through all the records in the other tables that depend on this record, and ensure that they reference the correct IDs as well, and on and on it goes. With UUIDs as primary keys, you can just do pgdump ... | pg_restore ... quite reliably (keeping only the reference order in mind on the table level rather than record level).
And ofcourse there are added benefits like easier tests and better information security (as serial ids make it really easy to enumerate other records for a given resource once you get credentials for just one of them). It's a code smell to have the view considerations drive the system design on the backend.

Then we come to the deletedAt column. While it is a good thing, it is not a necessary one. We do hard delete a ton of things, especially in cases where your platform is driven by the users of your client and is not just a fancy admin dashboard for the client themselves. Failing to do so leads to lawsuits for breach of user privacy. And even in cases where all the data is owned by the client organization, not all entities are always going to be soft deleted. As such, it wasn't made part of the BaseEntity spec which is supposed to provide the base for the majority of projects while remaining a base entity and not becoming a base model.

At the end of the day, it's much easier to extend a minimal base for your use case than it is to make extra stuff mandatory and then keep having to remove it on a per-project basis. For both of the above discussed "enhancements", you can very easily create a base class in your project which extends BaseEntity and adds these two properties plus the additional wiring, and use it throughout your project, while remaining compatible with all the other parts of hexapp on a type level.

@m-salman-afzal
Copy link
Author

I agree with your discussion of UUIDs. First, you mentioned that we can get server generated UUID. Of course, we can, but we are not doing it that way in any project at CT. Only Viral Solution's team is doing that. All other projects are doing db generated. So serial ID being a burden on db doesn't make the point. Also, UUID is still going to be our primary key that is being used inside the application as FKs and all the referencing and querying. Serial IDs will only be needed for data display.

2ndly, for the deletedAt, I totally disagree with your point. It is a necessary one. Considering the kind of projects we do here, they are all just fancy dashboards as you put it. For the user privacy problem, no one provides a hard delete on their dashboard. There is always a delete request method that is usually integrated into the system where users can make the ticket to get their data permanently deleted. Also, if we are going to permanently delete user data, it must say on the FE that you are permanently going to delete a certain data (which we are not doing anywhere unless asked from the client).

Also, the problem with these kinds of frameworks that someone builds like .NET, Spring, is there are always changes in newer versions that are not available in the previous versions so they have to make a support timeline of specific versions so that you are maintaining the old version for the projects that depend on it for some time, (or maybe just freeze them) and new projects can still benefit from the added features in newer versions. It does not make sense to do something where you change something in your framework and all the previous projects implemented in that specific version have to change themselves also.

So at the end of day, what matters is the business needs, and how we are going to fulfill it. Making the job a little bit easier on the development side goes a long way. I believe that is what the hexapp aims to solve.

@volf52
Copy link
Collaborator

volf52 commented Nov 12, 2024

It is being done in all the projects I have worked on so far, and is one of the core parts of hexapp. If you're still relying on db generated primary keys, I'm afraid you're not following the principles and architecture outlined in the company guidelines (unless of course it's a project with enough legacy code to make it infeasible to warrant a migration).
You can add serial IDs for data display if required, just like any other entity property. But the concerns of your project don't necessitate adding a mandatory property in a base class in a library which is supposed to be used in most if not all projects moving forward.

You can disagree with my point related to deletedAt, the projects you have worked on so far or are working on might require such an approach, but reiterating my point, it doesn't make it necessary to add it as a required property. If hexapp was a personal library with a collection of utilities that one developer is using in just their own projects, adding or removing things would not incur the same cost. But developing a library requires a different perspective as compared to that for a project, and a good rule of thumb here is to keep the API surface as minimal as possible, as opposed to adding things which might be required in a few projects. This reduces the boilerplate the users of the library will have to deal with.

Also, not all projects, including the ones at Carbonteq, are glorified admin dashboards. Admin dashboard here refers to applications which provide a presentation layer and some custom logic related to the client's data directly, as opposed to a platform where users different from the client/stakeholders sign up, provide their data and benefit from a specific set of services. You can argue about which projects lie in which category, and by how much, but at the end of the day, projects in both of the categories have both kinds of entities, the ones that the client doesn't want to be hard deleted (perhaps a record of actions that might be required for an audit) and records that do need to be hard deleted (e.g. metadata for files related to a user). Keeping this in mind, consider the route where deletedAt is added to core layer. The future projects built with this would have to add extra logic per entity class to remove this property every time they don't need it (or keep it as a redundant property which isn't really used in many entities). On the other hand, if the property is not added to the core layer and a project does require it for multiple entities, the fix is literally as easy as adding a single class which extends the BaseEntity and is then used as the base for other entities in the project which require soft delete.

Business needs matter in a project where you're building a solution for said business, but I would advise against treating everything as a nail with this one hammer in hand. hexapp is meant to provide an opinionated base which softly enforces a specific structure in the projects at Carbonteq while remaining flexible enough to be modified on an ad-hoc basis to suit to the needs of a specific one, and at the same time tying into a larger ecosystem we hope to eventually build for different problems like security, logging, testing, telemetry and so on. Treating the development of this project the same way you would treat the development of a client project at Carbonteq would lead to a very rigid codebase and functional API not adaptable to a wider variety of systems.

I have shared the solution to what you require for your project in my previous comment and this one again. Just a single class in your project which extends from BaseEntity. I do not think it's a difficult enough task, but if required, I can add an example codeblock in the docs which might make it easier on the developer side, considering they'll just need to copy paste.

Asides:

  • Versioning is a whole other topic, and the versioning strategy of a framework is quite different from a library, especially the kind of library hexapp is.
  • hexapp is also not marked as stable yet as it is not mature enough yet to provide a guarantee of backwards compatibility, and because it is not as flexible yet as it perhaps could or should be. Prior to the release of the first major version, it is expected that the projects should ideally upgrade to the latest possible versions even in the presence of breaking changes.
  • Just because things were being done a specific way so far in a specific set of scenarios and projects does not mean that's the best way to do things for all projects. One of the reasons for spending this much time sharing and refining ideas surrounding hexapp is to improve the way things are done. Hopefully, discussions like this one will allow us to discover a better way of doing things, both in specific instances and in general development. Thank you for initiating a discussion.

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

No branches or pull requests

2 participants