File Retention Properties #399

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.

Sign up for GitHub

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

Jump to bottom

Merged

GraciesPadre merged 11 commits into SpectraLogic:master from sulabhjain1991:master

Jan 16, 2017

Contributor

sulabhjain1991 commented Dec 7, 2016

jna library used for s-id of owner and group and dacl
MetaDataAccessImpl for saving data on bp
MetadataReceivedListenerImpl.java for restoring data on db
MetaDataUtil.java utility class for metadata


          build.gradle contains jna library

42a0941

MetaDataAccessImpl for saving data on bp
MetadataReceivedListenerImpl.java for restoring data on db
MetaDataUtil.java utility class for metadata

Contributor

rpmoore commented Dec 7, 2016

There was a test failure. It looks like the metadata change requires a class in the browser code?


          updated code of file rentention properties

55c97fd

Contributor Author

sulabhjain1991 commented Dec 8, 2016 •

edited

Loading

We have removed the undesired references and also made some code changes. Kindly review.

sulabhjain1991 closed this

Contributor Author

sulabhjain1991 commented Dec 8, 2016

closed by mistake. Reopened again

sulabhjain1991 reopened this

amit-dubey and others added 4 commits

December 9, 2016 15:06


          updated code of file rentention properties

92aeec4


          updated code of file rentention properties


          changes MetadataKeyConstants : for metadata key constants

1511dfb


          issue resolved with creation date in MAC

89c788a

Contributor

rpmoore commented Jan 5, 2017

I don't see any tests for these features. Please add some tests that can be ran on both windows and linux.

GraciesPadre suggested changes

View reviewed changes

Contributor

GraciesPadre left a comment

What you are merging really ought to be on a branch other than master.

Many of the files in this pull request lack the copyright block at the top of the file.

ds3-sdk/src/main/java/com/spectralogic/ds3client/metadata/MetaDataAccessImpl.java Outdated

+                      try {
+                          final MetaDataUtil metadataUtil = new MetaDataUtil(metadata);
+                          final Set<String> sets = metadataUtil.getSupportedFileAttributes(file);
+                          final String os = metadataUtil.getOS();

Contributor

GraciesPadre Jan 7, 2017

The name sets is very generic, making it difficult to know what the set contains.

ds3-sdk/src/main/java/com/spectralogic/ds3client/metadata/MetaDataAccessImpl.java Outdated

+                          final Set<String> sets = metadataUtil.getSupportedFileAttributes(file);
+                          final String os = metadataUtil.getOS();
+                          metadataUtil.saveOSMetaData();

Contributor

GraciesPadre Jan 7, 2017

This should be refactored into an interface with an abstract base for things common to all OS's and subclasses for OS's that need special handling. There needs to be a creational pattern that returns the correct version of subclass based on OS, or whatever criteria is appropriate.

ds3-sdk/src/main/java/com/spectralogic/ds3client/metadata/MetaDataAccessImpl.java Outdated

+                          for (final String set : sets) {
+                              switch (set) {
+                                  case "basic":
+                                      final BasicFileAttributes attr = Files.readAttributes(file, BasicFileAttributes.class);

Contributor

GraciesPadre Jan 7, 2017

It is difficult to know what the string constants mean based only on the variable name set

ds3-sdk/src/main/java/com/spectralogic/ds3client/metadata/MetaDataAccessImpl.java Outdated

+                   * @param metadata
+                   */
+                  private void saveWindowsfilePermissions(final Path file, final ImmutableMap.Builder<String, String> metadata) {
+                      try {

Contributor

GraciesPadre Jan 7, 2017

This should be a specialization of an interface with a concrete implementation
for windows.

ds3-sdk/src/main/java/com/spectralogic/ds3client/metadata/MetaDataAccessImpl.java Outdated

+                          final List<AclEntry> aclEntries = view.getAcl();
+                          Set<AclEntryPermission> aclEntryPermissions;
+                          String userType = "";
+                          String userDisplay = "";

Contributor

GraciesPadre Jan 7, 2017

The initializations are unnecessary.

ds3-sdk/src/main/java/com/spectralogic/ds3client/metadata/MetaDataAccessImpl.java Outdated

+                              }
+                          }
+                      } catch (final IOException ioe) {
+                          LOG.error("unable to get metadata", ioe);

Contributor

GraciesPadre Jan 7, 2017

Failure event callback

ds3-sdk/src/main/java/com/spectralogic/ds3client/metadata/MetadataReceivedListenerImpl.java Outdated

+                      try {
+                          aclAttributeView.setAcl(aclEntryBuilder.build());
+                      } catch (final IOException e) {
+                          LOG.error("Unable to restore dacl view", e);

Contributor

GraciesPadre Jan 7, 2017

Failure event or let the exception bubble back to the user's code.

ds3-sdk/src/main/java/com/spectralogic/ds3client/metadata/MetadataReceivedListenerImpl.java Outdated

+                          final String[] ordinalArr = permission.split("-");
+                          for (final String ordinal : ordinalArr) {
+                              switch (ordinal) {
+                                  case "0":

Contributor

GraciesPadre Jan 7, 2017

It is difficult to know what the numbers mean in terms of permissions. In addition the switch has many case statements. This can be refactored to be more readable and concise by using a map to associate some sort of value other than a number to the permission being set.

ds3-sdk/src/main/java/com/spectralogic/ds3client/metadata/jna/Advapi32.java Outdated


		Advapi32 INSTANCE = (Advapi32) Native.loadLibrary("advapi32", Advapi32.class, W32APIOptions.UNICODE_OPTIONS);

		int GetNamedSecurityInfo(String var1, int var2, int var3, PointerByReference var4, PointerByReference var5, PointerByReference var6, PointerByReference var7, PointerByReference var8);

Contributor

GraciesPadre Jan 7, 2017

var1, var2? What do these variables refer to? They should be named according to the win api entries, e.g. pObjectName, objectType, etc.

ds3-sdk/src/main/java/com/spectralogic/ds3client/utils/MetadataKeyConstants.java

		@@ -0,0 +1,37 @@
		package com.spectralogic.ds3client.utils;

Contributor

GraciesPadre Jan 7, 2017

All the public status final modifiers are redundant in an interface.


          Redesign the classes for sdk and test cases

026d702

GraciesPadre suggested changes

View reviewed changes

Contributor

GraciesPadre left a comment

When I ran the unit tests with coverage, only 3 classes had any coverage. Of those that had coverage, the most method coverage was 20%.

ds3-sdk/src/main/java/com/spectralogic/ds3client/metadata/MACMetaDataRestore.java Outdated

+               * ****************************************************************************
+               */
+              // This code is auto-generated, do not modify

Contributor

GraciesPadre Jan 10, 2017

Was this really auto-generated? This line appears, as far as I have looked, in all the copyright blocks in added files. If the file is not auto-generated, this line needs removing, because we interpret that comment to mean that we should not edit the containing file.

ds3-sdk/src/main/java/com/spectralogic/ds3client/metadata/interfaces/MetaDataRestore.java Outdated



		public interface MetaDataRestore {
		Logger LOG = LoggerFactory.getLogger(MetadataReceivedListenerImpl.class);

Contributor

GraciesPadre Jan 10, 2017

MetadataReceivedListenerImpl does not implement the MetaDataRestore interface. Anything that does will have the class name MetadataReceivedListenerImpl, regardless of what class it actually is, if it relies on this interface to log. Meaning that it will be difficult to correlate a log message to the class that actually logged the message.

ds3-sdk/src/main/java/com/spectralogic/ds3client/metadata/interfaces/MetaDataStoreListner.java

		package com.spectralogic.ds3client.metadata.interfaces;

		public interface MetaDataStoreListner {

Contributor

GraciesPadre Jan 10, 2017

Spelling. public interface MetaDataStoreListener

...dk/src/main/java/com/spectralogic/ds3client/metadata/interfaces/AbstractMetadataRestore.java Outdated

+                      }
+                      catch (final Exception e)
+                      {
+                          e.printStackTrace();

Contributor

GraciesPadre Jan 10, 2017

Customers won't know that there has been an exception thrown, unless they are looking at where ever the print stream writer is pointed. This should be propagated back to the client, either by letting the exception bubble back or by emitting a failure event.

...dk/src/main/java/com/spectralogic/ds3client/metadata/interfaces/AbstractMetadataRestore.java Outdated

+              import com.spectralogic.ds3client.networking.Metadata;
+              import java.io.File;
+              import java.io.IOException;

Contributor

GraciesPadre Jan 10, 2017

Unused imports

ds3-sdk/src/main/java/com/spectralogic/ds3client/metadata/WindowsMetaDataStore.java Outdated

+                   * @param file local file of the path
+                   * @return flag of file in String
+                   */
+                  public String saveFlagMetaData(final Path file) {

Contributor

GraciesPadre Jan 10, 2017

Should this really be public?

ds3-sdk/src/main/java/com/spectralogic/ds3client/metadata/WindowsMetaDataStore.java Outdated

+                   *
+                   * @param file local file path
+                   */
+                  public void saveWindowsfilePermissions(final Path file) {

Contributor

GraciesPadre Jan 10, 2017

Should this really be public?

ds3-sdk/src/main/java/com/spectralogic/ds3client/utils/MetaDataUtil.java

+                   * @param filename      : name of the file with logical folder
+                   * @return actualFilePath
+                   */
+                  public static String getRealFilePath(final String localFilePath, final String filename) {

Contributor

GraciesPadre Jan 11, 2017

Probably ought to take symbolic links into account. See StrategyUtils.resolveForSymbolic()

ds3-sdk/src/test/java/com/spectralogic/ds3client/metadata/MetaDataAccessImplModelTest.java Outdated

+                  }
+                  @Test
+                  public void saveFileGroupId() {

Contributor

GraciesPadre Jan 11, 2017

This test failed on my mac.

ds3-sdk/src/test/java/com/spectralogic/ds3client/metadata/MetaDataAccessImplModelTest.java Outdated

		@@ -0,0 +1,70 @@
		/*

Contributor

GraciesPadre Jan 11, 2017

When I ran the unit tests with coverage, only 3 classes had any coverage. Of those that had coverage, the most method coverage was 20%

Contributor

rpmoore commented Jan 12, 2017

This branch is now out of date. Please merge the latest master code into your pull request and then push the merge to this branch.

sulabhjain1991 added 2 commits

January 13, 2017 19:00


          Redesign the classes for sdk and test cases

f1e09f7


          Merge pull request #1 from SpectraLogic/master

67747f1

update sdk

GraciesPadre suggested changes

View reviewed changes

ds3-sdk/src/test/java/com/spectralogic/ds3client/metadata/MACMetadataRestore_Test.java

+              @RunWith(JUnit4.class)
+              public class MACMetadataRestore_Test {
+                  private final File file = new File(getClass().getClassLoader().getResource("LoremIpsum.txt").getFile());

Contributor

GraciesPadre Jan 13, 2017

This test failed on my mac

org.junit.ComparisonFailure:
Expected :1484248020000
Actual :1484248028000

GraciesPadre suggested changes

View reviewed changes

ds3-sdk/src/test/java/com/spectralogic/ds3client/metadata/PosixMetadataRestore_Test.java

		}


		@Test

Contributor

GraciesPadre Jan 13, 2017

This test fails on windows

java.lang.UnsupportedOperationException: View 'unix' not available

at sun.nio.fs.AbstractFileSystemProvider.readAttributes(AbstractFileSystemProvider.java:91)
at java.nio.file.Files.readAttributes(Files.java:1964)
at java.nio.file.Files.getAttribute(Files.java:1869)
at com.spectralogic.ds3client.metadata.PosixMetadataRestore_Test.restoreUserAndOwner(PosixMetadataRestore_Test.java:65)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at org.junit.runners.Suite.runChild(Suite.java:128)
at org.junit.runners.Suite.runChild(Suite.java:27)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:51)
at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:237)
at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at com.intellij.rt.execution.application.AppMain.main(AppMain.java:147)

ds3-sdk/src/test/java/com/spectralogic/ds3client/metadata/PosixMetadataRestore_Test.java



		@Test
		public void restorePermissions() throws Exception{

Contributor

GraciesPadre Jan 13, 2017

This test fails on windows

java.lang.UnsupportedOperationException
at sun.nio.fs.WindowsFileSystemProvider.readAttributes(WindowsFileSystemProvider.java:192)
at java.nio.file.Files.readAttributes(Files.java:1737)
at com.spectralogic.ds3client.metadata.PosixMetadataRestore_Test.restorePermissions(PosixMetadataRestore_Test.java:80)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at org.junit.runners.Suite.runChild(Suite.java:128)
at org.junit.runners.Suite.runChild(Suite.java:27)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:51)
at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:237)
at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at com.intellij.rt.execution.application.AppMain.main(AppMain.java:147)

sulabhjain1991 added 2 commits

January 16, 2017 14:49


          Mac test case issue resolved

1072beb


          Merge remote-tracking branch 'origin/master'

3f53d8c

GraciesPadre approved these changes

View reviewed changes

GraciesPadre merged commit 321c62a into SpectraLogic:master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet