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

Add better Jcloud versioning supports #8512

Merged
merged 2 commits into from
Nov 28, 2024

Conversation

ianwallen
Copy link
Contributor

@ianwallen ianwallen commented Nov 24, 2024

Jcloud does not really support version numbers and will instead create an ETAG which is not comprehensible by the end users. We need to use jcloud storage metadata properties to manage the versions being created.

Before
image

After
image

Added the following new properties for identifying the version property name to be used as well as the strategy.

  • jcloud.external.resource.management.version.property.name
  • jcloud.versioning.strategy

The strategy is to indicate how versioning will be performed based on how the system is configured.

  • ALL - each file uploaded will create a new version (Default)
  • DRAFT - each file updloaded will create a new version - but once approved, it can only create one version for each approval
  • APPROVED - Can only create one version per approval process.

Jcloud does not allow specifying the creation date on the object. It will always assume that it was created on the date loaded and updated on the date loaded. So if loading older data and we want the date to reflect that older date, we need to store the data in a custom property. Added support for specifying the created date property name as well. jcloud.external.resource.management.created.date.property.name Updates the existing change date so that it always specifies the changed date. Prior to this change it would leave the change date property empty when it was equal to the current date.

Also fixed some other minor issue

  • remove finalize() as it was deprecated.

Checklist

  • I have read the contribution guidelines
  • Pull request provided for main branch, backports managed with label
  • Good housekeeping of code, cleaning up comments, tests, and documentation
  • Clean commit history broken into understandable chucks, avoiding big commits with hundreds of files, cautious of reformatting and whitespace changes
  • Clean commit messages, longer verbose messages are encouraged
  • API Changes are identified in commit messages
  • Testing provided for features or enhancements using automatic tests
  • User documentation provided for new features or enhancements in manual
  • Build documentation provided for development instructions in README.md files
  • Library management using pom.xml dependency management. Update build documentation with intended library use and library tutorials or documentation

…e an ETAG which is not comprehensible by the end users. We need to use jcloud storage metadata properties to manage the versions being created.

Added the following new properties for identifying the version property name to be used as well as the strategy.
- jcloud.external.resource.management.version.property.name
- jcloud.versioning.strategy
The strategy is to indicate how versioning will be performed based on how the system is configured.
- ALL - each file uploaded will create a new version (Default)
- DRAFT - each file updloaded will create a new version - but once approved, it can only create one version for each approval
- APPROVED - Can only create one version per approval process.

Jcloud does not allow specifying the creation date on the object.  It will always assume that it was created on the date loaded and updated on the date loaded. So if loading older data and we want the date to reflect that older date, we need to store the data in a custom property.  Added support for specifying the created date property name as well.
jcloud.external.resource.management.created.date.property.name
Updates the existing change date so that it always specifies the changed date. Prior to this change it would leave the change date property empty when it was equal to the current date.

Also fixed some other minor issue
 - remove finalize() as it was deprecated.
@ianwallen ianwallen added this to the 4.4.7 milestone Nov 24, 2024
Copy link
Member

@josegar74 josegar74 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't test the changes, just code review.

Please check the comments.

I notice also that baseMetadataDir definition in https://github.com/geonetwork/core-geonetwork/pull/8512/files#diff-aea5108dfa3cfbf795a7dfe7a199ee09c7da2a13deecb8ba392ed167285ead9cL462 is hiding the instance variable https://github.com/geonetwork/core-geonetwork/pull/8512/files#diff-aea5108dfa3cfbf795a7dfe7a199ee09c7da2a13deecb8ba392ed167285ead9cL81

It's not part of the pull request changes, but would be good to rename it.


return new FilesystemStoreResource(metadataUuid, metadataId, filename,
settingManager.getNodeURL() + "api/records/", visibility, storageMetadata.getSize(), storageMetadata.getLastModified(), versionValue, metadataResourceExternalManagementProperties, approved);
settingManager.getNodeURL() + "api/records/", visibility, storageMetadata.getSize(), changedDate, versionValue, metadataResourceExternalManagementProperties, approved);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to check if changedDate or versionValue are null, if could cause issues in FilesystemStoreResource. In the previous code I have no clear if could be null.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed changeDate so that it will not be null like before.
For versionValues, it was previously null if versioning was not enabled so it should not require any change.

// JCloud does not allow created date to be set so we may supply the value we want as a property so assign the value.
// Only assign the value if we currently don't have a creation date, and we don't have a version assigned either because if either of these exists then
// it will indicate that this is not the first version.
String CreatedDatePropertyName = jCloudConfiguration.getExternalResourceManagementCreatedDatePropertyName();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to createdDatePropertyName

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

// Only assign the value if we currently don't have a creation date, and we don't have a version assigned either because if either of these exists then
// it will indicate that this is not the first version.
String CreatedDatePropertyName = jCloudConfiguration.getExternalResourceManagementCreatedDatePropertyName();
String VersionPropertyName = jCloudConfiguration.getExternalResourceManagementVersionPropertyName();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to versionPropertyName

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

if (StringUtils.hasLength(jCloudConfiguration.getExternalResourceManagementVersionPropertyName())) {
String versionPropertyName = jCloudConfiguration.getExternalResourceManagementVersionPropertyName();

final int approvedMetadataId = approved ? metadataId : canEdit(context, metadataUuid, true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
final int approvedMetadataId = approved ? metadataId : canEdit(context, metadataUuid, true);
final int approvedMetadataId = Boolean.TRUE.equals(approved) ? metadataId : canEdit(context, metadataUuid, true);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

static {
DATE_FORMATTER.setTimeZone(TimeZone.getTimeZone("UTC"));
}

// For azure Blob ADSL hdi_isfolder property name used to identify folders
private final static String AZURE_BLOB_IS_FOLDER_PROPERTY_NAME="hdi_isfolder";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private final static String AZURE_BLOB_IS_FOLDER_PROPERTY_NAME="hdi_isfolder";
private static final String AZURE_BLOB_IS_FOLDER_PROPERTY_NAME="hdi_isfolder";

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

@ianwallen ianwallen requested a review from josegar74 November 28, 2024 15:30
@ianwallen ianwallen merged commit b125070 into geonetwork:main Nov 28, 2024
6 checks passed
@ianwallen ianwallen deleted the jcloud_version branch November 28, 2024 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants