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

Move projections to separate module #77

Open
lavrukov opened this issue Jun 2, 2024 · 0 comments · May be fixed by #76
Open

Move projections to separate module #77

lavrukov opened this issue Jun 2, 2024 · 0 comments · May be fixed by #76
Assignees
Labels
bug Something isn't working feature New feature or request refactoring Internal improvement that does not change the API or library behavior

Comments

@lavrukov
Copy link
Contributor

lavrukov commented Jun 2, 2024

The projection feature was originally not designed very well and has a number of issues:

  • Unsafe modifying methods issue - the delete and save methods can be called without read, so there will be no objects in the cache, making it impossible to read previous projections, which leads to memory leaks. Due to this behavior, many methods like deleteIfExist have appeared, and because of the use of regular delete, we have repeatedly encountered bugs in production.
  • Projection isn't found in tx cache after save in tx Issue #31
  • Fix Check that read happend before save. #66
  • This is a separable functionality and should not be in the core part. Because of this feature, I personally had to create workarounds, bypass maneuvers, and think a lot about how not to break anything while making changes in the transaction and inMemory parts.

Where we want to go:

  • There is a ProjectionTable which acts as a proxy to Table, with overridden insert, save, and delete methods that implement the logic for working with projections.
  • createProjections becomes a field of ProjectionTable in the form of Supplier<List<Entity<?>>>.
  • ProjectionCache is removed.
  • Since ProjectionTable is no longer a general solution but works only with Entity with projections, we can fail in save and delete if the object is not in the cache. This ensures that working with projections will be safe, and we can forget about unsafe methods and methods like deleteIfExists.

How the migration process will look:

  1. First, implement the new MigrationProjectionsCache, which immediately calls the methods table.{save,delete} without waiting for a commit.
  2. Initially, enable it without a flag and run tests for all projects, ensuring they are green.
  3. Then, leave the feature under a flag and release the service, ensuring that the service also works.
  4. Add a method withoutProjections() to the table that sets a property indicating that projections should not be processed.
  5. Remove ProjectionCache, leaving the logic from MigrationProjectionsCache directly in the Table methods under the withoutProjections flag.
  6. Create a ProjectionsTable proxy that accepts Table<E> in the constructor and sets withoutProjections() on the proxied object.
  7. In ProjectionsTable, override only the insert, save, and delete methods with the projection logic and new error handling.
  8. In the Entity class with the base implementation of createProjections, throw a special CreateProjectionNotOverridedException.
  9. In Table, catch the exception and if withoutProjections is set, fail; otherwise, log an error with a requirement to migrate to the new logic.
  10. After all services migrated remove logic from table, withoutProjections flag and method and move all projection code to separate module

TODO: think how to protect services which can call tx.table(MyEntityWithProjections.class).{insert,save,delete} directly

@lavrukov lavrukov added bug Something isn't working feature New feature or request refactoring Internal improvement that does not change the API or library behavior labels Jun 2, 2024
@lavrukov lavrukov self-assigned this Jun 2, 2024
@lavrukov lavrukov linked a pull request Jun 2, 2024 that will close this issue
@lavrukov lavrukov changed the title Move projections for separate module Move projections to separate module Jun 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feature New feature or request refactoring Internal improvement that does not change the API or library behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant