kit-data-manager / ro-crate-java

Java library to create and modify RO-Crates.
https://kit-data-manager.github.io/webpage/ro-crate-java/
Apache License 2.0
2 stars 3 forks source link

Adding urls/pairs to the context without RoCrateBuilder #36

Closed Code42Cate closed 2 years ago

Code42Cate commented 2 years ago

There is no way to add to the the context without using the RoCrateBuilder (which is not possible when using the RoCrateReader).

There are probably more features missing, and the nicest fix IMO would be to extend the RoCrateBuilder to allow using an existing RoCrate as a template.

RoCrate crate = new RoCrate.RoCrateBuilder(existingCrate)[...]build();
Code42Cate commented 2 years ago

I have not tested everything, but it seems like this constructor would be enough to enable my suggestion:

public RoCrateBuilder(RoCrate crate) {
      this.payload = crate.roCratePayload;
      this.metadataContext = crate.metadataContext;
      this.preview = crate.roCratePreview;
      this.rootDataEntity = crate.rootDataEntity;
      this.jsonDescriptor = crate.jsonDescriptor;
      this.untrackedFiles = crate.untrackedFiles;
}
Pfeil commented 2 years ago

Seems like a good solution to me. If you do this, we could in theory remove all other setters of RoCrate, so modification is only possible from the builder, where we have potentially a better control over what is allowed and what is not. Although removing methods would be a breaking change, so we could mark it as deprecated and remove it in version 2.0, then. Thoughts?

Code42Cate commented 2 years ago

If we are sure that we don't lose any functionality, yes I would do that. Less interfaces that do the same:)

Code42Cate commented 2 years ago

We also need to keep in mind that removing stuff only works on the RoCrate objects, not with the builder.

Pfeil commented 2 years ago

Yeah, we definitely need a long term strategy for that to make things more consistent. This is something for the next major milestone (2.0) I guess. For now, I think, this is fine. Will the PR close this issue, or are other aspects the PR does not fix?

Code42Cate commented 2 years ago

The issue is fixed with #39 ! :)