JyotinderSingh commented on code in PR #3483:
URL: https://github.com/apache/ozone/pull/3483#discussion_r895454919
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/TestCleanupTableInfoResponses.java:
##########
@@ -0,0 +1,331 @@
+package org.apache.hadoop.ozone.om.response;
Review Comment:
Please add the Apache License comment to the top of any new files you create
in the project.
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/TestCleanupTableInfoResponses.java:
##########
@@ -0,0 +1,331 @@
+package org.apache.hadoop.ozone.om.response;
+
+import com.google.common.collect.Maps;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.fs.SaveSpaceUsageToFile;
+import org.apache.hadoop.hdds.scm.storage.RatisBlockOutputStream;
+import org.apache.hadoop.hdds.server.ServerUtils;
+import org.apache.hadoop.hdds.utils.db.BatchOperation;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.response.s3.security.S3GetSecretResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.mockito.Mockito;
+import org.mockito.junit.MockitoJUnitRunner;
+import org.mockito.stubbing.Answer;
+import org.reflections.Reflections;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.IOException;
+import java.lang.reflect.Array;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static
org.apache.hadoop.ozone.om.response.TestCleanupTableInfo.OM_RESPONSE_PACKAGE;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.spy;
+
+@RunWith(MockitoJUnitRunner.class)
+public class TestCleanupTableInfoResponses{
+
+ private static final Logger LOG = LoggerFactory.getLogger(
+ RatisBlockOutputStream.class);
+
+ @Rule
+ public TemporaryFolder folder = new TemporaryFolder();
+
+ /**
+ * Creates a spy object over an instantiated OMMetadataManager, giving the
+ * possibility to redefine behaviour. In the current implementation
+ * there isn't any behaviour which is redefined.
+ *
+ * @return the OMMetadataManager spy instance created.
+ * @throws IOException if I/O error occurs in setting up data store for the
+ * metadata manager.
+ */
+ private OMMetadataManager createOMMetadataManagerSpy(File folder) throws
IOException {
+ OzoneConfiguration conf = new OzoneConfiguration();
+ File newFolder = folder;
+ if (!newFolder.exists()) {
+ Assert.assertTrue(newFolder.mkdirs());
+ }
+ ServerUtils.setOzoneMetaDirPath(conf, newFolder.toString());
+ return spy(new OmMetadataManagerImpl(conf));
+ }
+
+ private void mockTables(OMMetadataManager omMetadataManager,
+ Set<String> updatedTables,
+ boolean isFSO)
+ throws IOException, IllegalAccessException {
+ Map<String, Table> tableMap = omMetadataManager.listTables();
+ Map<Table, Table> mockedTableMap = new HashMap<>();
+ List<String> tables = new ArrayList<>(tableMap.keySet());
+ for (String table:tables) {
+ Table mockedTable = Mockito.spy(tableMap.get(table));
+ Answer answer = invocationOnMock -> {
+ String t = table;
+ if (isFSO) {
+ if (table.equals(OmMetadataManagerImpl.OPEN_KEY_TABLE)) {
+ t = OmMetadataManagerImpl.OPEN_FILE_TABLE;
+ }
+ if (table.equals(OmMetadataManagerImpl.KEY_TABLE)) {
+ t = OmMetadataManagerImpl.FILE_TABLE;
+ }
+ } else {
+ if (table.equals(OmMetadataManagerImpl.OPEN_FILE_TABLE)) {
+ t = OmMetadataManagerImpl.OPEN_KEY_TABLE;
+ }
+ if (table.equals(OmMetadataManagerImpl.FILE_TABLE)) {
+ t = OmMetadataManagerImpl.KEY_TABLE;
+ }
+ }
+ updatedTables.add(t);
+ return null;
+ };
+ Mockito.lenient().doAnswer(answer).when(mockedTable).put(any(), any());
+ Mockito.lenient().doAnswer(answer).when(mockedTable)
+ .putWithBatch(any(), any(), any());
+ Mockito.lenient().doAnswer(answer).when(mockedTable).delete(any());
+
Mockito.lenient().doAnswer(answer).when(mockedTable).deleteWithBatch(any(),
any());
+ mockedTableMap.put(tableMap.get(table), mockedTable);
+ tableMap.put(table, mockedTable);
+ }
+ for (Field f:omMetadataManager.getClass().getDeclaredFields()) {
+ boolean isAccessible = f.isAccessible();
+ f.setAccessible(true);
+ try {
+ if (Table.class.isAssignableFrom(f.getType())) {
+ f.set(omMetadataManager,
+ mockedTableMap.getOrDefault(f.get(omMetadataManager),
+ (Table)f.get(omMetadataManager)));
+ }
+ } finally {
+ f.setAccessible(isAccessible);
+ }
+ }
+ }
+ private <T> T getMockObject(Class<T> type,
+ Map<Class, Object> instanceMap,
+ OzoneManagerProtocolProtos.Status status,
+ boolean isFSO) {
+ T instance = Mockito.mock(type, invocationOnMock -> {
+ try {
+ return createInstance(invocationOnMock.getMethod().getReturnType(),
+ instanceMap, status, isFSO);
+ } catch (Throwable e) {
+ throw new RuntimeException(e);
+ }
+ });
+ instanceMap.put(type, instance);
+ return instance;
+ }
+ private List<Field> getFields(Class<?> type) {
+ List<Field> fields = new ArrayList<>();
+ while (type != Object.class) {
+ fields.addAll(Arrays.asList(type.getDeclaredFields()));
+ type = type.getSuperclass();
+ }
+ return fields;
+ }
+ private Object createInstance(Class<?> type,
+ Map<Class, Object> instanceMap,
+ OzoneManagerProtocolProtos.Status status,
boolean isFSO) {
+ if (instanceMap.containsKey(type)) {
+ return instanceMap.get(type);
+ }
+ Object instance = null;
+ if (type.isArray()) {
+ return Array.newInstance(type.getComponentType(), 0);
+ } else if (type.equals(Void.TYPE)) {
+ return null;
+ } else if (type.isPrimitive()) {
+ if (Boolean.TYPE.equals(type)) {
+ return true;
+ } else if (Character.TYPE.equals(type)) {
+ return 'a';
+ } else if (Byte.TYPE.equals(type)) {
+ return 0xFF;
+ } else if (Short.TYPE.equals(type)) {
+ return (short) 0;
+ } else if (Integer.TYPE.equals(type)) {
+ return 0;
+ } else if (Long.TYPE.equals(type)) {
+ return (long) 0;
+ } else if (Float.TYPE.equals(type)) {
+ return (float) 0.0;
+ } else if (Double.TYPE.equals(type)) {
+ return 0.0;
+ } else if (Void.TYPE.equals(type)) {
+ return null;
+ }
Review Comment:
A `switch` statement might be more readable here.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMAllocateBlockResponseWithFSO.java:
##########
@@ -54,7 +54,7 @@ public OMAllocateBlockResponseWithFSO(@Nonnull OMResponse
omResponse,
}
@Override
- public void addToDBBatch(OMMetadataManager omMetadataManager,
+ public void addToDBBatch(OMMetadataManager omMetadataManager,
Review Comment:
nit: remove indentation change.
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/TestCleanupTableInfoResponses.java:
##########
@@ -0,0 +1,331 @@
+package org.apache.hadoop.ozone.om.response;
+
+import com.google.common.collect.Maps;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.fs.SaveSpaceUsageToFile;
+import org.apache.hadoop.hdds.scm.storage.RatisBlockOutputStream;
+import org.apache.hadoop.hdds.server.ServerUtils;
+import org.apache.hadoop.hdds.utils.db.BatchOperation;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.response.s3.security.S3GetSecretResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.mockito.Mockito;
+import org.mockito.junit.MockitoJUnitRunner;
+import org.mockito.stubbing.Answer;
+import org.reflections.Reflections;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.IOException;
+import java.lang.reflect.Array;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static
org.apache.hadoop.ozone.om.response.TestCleanupTableInfo.OM_RESPONSE_PACKAGE;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.spy;
+
+@RunWith(MockitoJUnitRunner.class)
+public class TestCleanupTableInfoResponses{
+
+ private static final Logger LOG = LoggerFactory.getLogger(
+ RatisBlockOutputStream.class);
+
+ @Rule
+ public TemporaryFolder folder = new TemporaryFolder();
+
+ /**
+ * Creates a spy object over an instantiated OMMetadataManager, giving the
+ * possibility to redefine behaviour. In the current implementation
+ * there isn't any behaviour which is redefined.
+ *
+ * @return the OMMetadataManager spy instance created.
+ * @throws IOException if I/O error occurs in setting up data store for the
+ * metadata manager.
+ */
+ private OMMetadataManager createOMMetadataManagerSpy(File folder) throws
IOException {
+ OzoneConfiguration conf = new OzoneConfiguration();
+ File newFolder = folder;
+ if (!newFolder.exists()) {
+ Assert.assertTrue(newFolder.mkdirs());
+ }
+ ServerUtils.setOzoneMetaDirPath(conf, newFolder.toString());
+ return spy(new OmMetadataManagerImpl(conf));
+ }
+
+ private void mockTables(OMMetadataManager omMetadataManager,
+ Set<String> updatedTables,
+ boolean isFSO)
+ throws IOException, IllegalAccessException {
+ Map<String, Table> tableMap = omMetadataManager.listTables();
+ Map<Table, Table> mockedTableMap = new HashMap<>();
+ List<String> tables = new ArrayList<>(tableMap.keySet());
+ for (String table:tables) {
+ Table mockedTable = Mockito.spy(tableMap.get(table));
+ Answer answer = invocationOnMock -> {
+ String t = table;
+ if (isFSO) {
+ if (table.equals(OmMetadataManagerImpl.OPEN_KEY_TABLE)) {
+ t = OmMetadataManagerImpl.OPEN_FILE_TABLE;
+ }
+ if (table.equals(OmMetadataManagerImpl.KEY_TABLE)) {
+ t = OmMetadataManagerImpl.FILE_TABLE;
+ }
+ } else {
+ if (table.equals(OmMetadataManagerImpl.OPEN_FILE_TABLE)) {
+ t = OmMetadataManagerImpl.OPEN_KEY_TABLE;
+ }
+ if (table.equals(OmMetadataManagerImpl.FILE_TABLE)) {
+ t = OmMetadataManagerImpl.KEY_TABLE;
+ }
+ }
+ updatedTables.add(t);
+ return null;
+ };
+ Mockito.lenient().doAnswer(answer).when(mockedTable).put(any(), any());
+ Mockito.lenient().doAnswer(answer).when(mockedTable)
+ .putWithBatch(any(), any(), any());
+ Mockito.lenient().doAnswer(answer).when(mockedTable).delete(any());
+
Mockito.lenient().doAnswer(answer).when(mockedTable).deleteWithBatch(any(),
any());
+ mockedTableMap.put(tableMap.get(table), mockedTable);
+ tableMap.put(table, mockedTable);
+ }
+ for (Field f:omMetadataManager.getClass().getDeclaredFields()) {
+ boolean isAccessible = f.isAccessible();
+ f.setAccessible(true);
+ try {
+ if (Table.class.isAssignableFrom(f.getType())) {
+ f.set(omMetadataManager,
+ mockedTableMap.getOrDefault(f.get(omMetadataManager),
+ (Table)f.get(omMetadataManager)));
+ }
+ } finally {
+ f.setAccessible(isAccessible);
+ }
+ }
+ }
+ private <T> T getMockObject(Class<T> type,
+ Map<Class, Object> instanceMap,
+ OzoneManagerProtocolProtos.Status status,
+ boolean isFSO) {
+ T instance = Mockito.mock(type, invocationOnMock -> {
+ try {
+ return createInstance(invocationOnMock.getMethod().getReturnType(),
+ instanceMap, status, isFSO);
+ } catch (Throwable e) {
+ throw new RuntimeException(e);
+ }
+ });
+ instanceMap.put(type, instance);
+ return instance;
+ }
+ private List<Field> getFields(Class<?> type) {
+ List<Field> fields = new ArrayList<>();
+ while (type != Object.class) {
+ fields.addAll(Arrays.asList(type.getDeclaredFields()));
+ type = type.getSuperclass();
+ }
+ return fields;
+ }
+ private Object createInstance(Class<?> type,
+ Map<Class, Object> instanceMap,
+ OzoneManagerProtocolProtos.Status status,
boolean isFSO) {
+ if (instanceMap.containsKey(type)) {
+ return instanceMap.get(type);
+ }
+ Object instance = null;
+ if (type.isArray()) {
+ return Array.newInstance(type.getComponentType(), 0);
+ } else if (type.equals(Void.TYPE)) {
+ return null;
+ } else if (type.isPrimitive()) {
+ if (Boolean.TYPE.equals(type)) {
+ return true;
+ } else if (Character.TYPE.equals(type)) {
+ return 'a';
+ } else if (Byte.TYPE.equals(type)) {
+ return 0xFF;
+ } else if (Short.TYPE.equals(type)) {
+ return (short) 0;
+ } else if (Integer.TYPE.equals(type)) {
+ return 0;
+ } else if (Long.TYPE.equals(type)) {
+ return (long) 0;
+ } else if (Float.TYPE.equals(type)) {
+ return (float) 0.0;
+ } else if (Double.TYPE.equals(type)) {
+ return 0.0;
+ } else if (Void.TYPE.equals(type)) {
+ return null;
+ }
+ } else if (type.equals(OzoneManagerProtocolProtos.OMResponse.class)) {
+ return OzoneManagerProtocolProtos.OMResponse.newBuilder()
+ .setStatus(status).buildPartial();
+ } else if (type.equals(BucketLayout.class)) {
+ return isFSO ? BucketLayout.FILE_SYSTEM_OPTIMIZED
+ : BucketLayout.DEFAULT;
Review Comment:
It would be better to explicitly define the bucket layout here. For instance
`LEGACY` or `OBJECT_STORE` instead of `DEFAULT`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
