search

sirixdb / sirix

SirixDB is an an embeddable, bitemporal, append-only database system and event store, storing immutable lightweight snapshots. It keeps the full history of each resource. Every commit stores a space-efficient snapshot through structural sharing. It is log-structured and never overwrites data. SirixDB uses a novel page-level versioning approach.
https://sirix.io
BSD 3-Clause "New" or "Revised" License
1.12k stars 253 forks source link

Clean architecture: Replace the `PageReadOnlyTrx` for serialization/deserialization of pages/nodes #519

Open JohannesLichtenberger opened 2 years ago

JohannesLichtenberger commented 2 years ago

It should not be given as a parameter to the serialize/deserialize methods to potentially read other data during serialization/deserialization of nodes for instance as it violates the layered architecture.

For instance, in the enum NodeKind the page read-only trx is used to read a name:

final String uri = pageReadTrx.getName(nameDel.getURIKey(), NodeKind.NAMESPACE);

In most occurrences the trx seems to be used to read the ResourceConfiguration, which might simply be given as a parameter.

pernelkanic commented 1 year ago

can i work on this issue

JohannesLichtenberger commented 1 year ago

@pernelkanic of course :) the enum in question is PageKind plus interface(s).

pernelkanic commented 1 year ago

In the PageKind I've removed the PageReadOnlyTrx parameter and added a ResourceConfiguration parameter for passing the necessary data required for serialization and deserialization? is that correct?

JohannesLichtenberger commented 1 year ago

Yes :-) maybe it would be even better to wrap the configuration in a context (simple data wrapper), but I guess it's ok :-)

JohannesLichtenberger commented 11 months ago

@pernelkanic can you open a PR?

pernelkanic commented 11 months ago

I have opened the PR, requesting review to know if there should be any changes.

JohannesLichtenberger commented 11 months ago

Currently it fails to compile, because I think an export missing.

pernelkanic commented 11 months ago

what should be done?

pernelkanic commented 11 months ago

It seems like org.jcp.xml.dsig.internal.dom.Utils is import that is causing the issue , it is considered internal and used in java.xml.crypto

pernelkanic commented 11 months ago

Does removing it fix the issue?

JohannesLichtenberger commented 11 months ago

Why do you need java.xml.crypto for the changes? Can you remove the import and update your PR?

pernelkanic commented 11 months ago

i have updated the pr