Skip to content

Commit

Permalink
Sonar Fixes P1 (open-metadata#11507)
Browse files Browse the repository at this point in the history 
* sonar lint fixes

* PARENT_FIELD

* review comments

* Fix failing glossaryTest
  • Loading branch information
@mohityadav766
mohityadav766 authored May 10, 2023
1 parent e0a4361 commit 647ff49
Show file tree
Hide file tree
Showing 68 changed files with 505 additions and 613 deletions.
63 changes: 34 additions & 29 deletions openmetadata-service/src/main/java/org/openmetadata/csv/CsvUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ public static String formatCsv(CsvFile csvFile) throws IOException {
List<String> headers = getHeaders(csvFile.getHeaders());
CSVFormat csvFormat = Builder.create(CSVFormat.DEFAULT).setHeader(headers.toArray(new String[0])).build();
try (CSVPrinter printer = new CSVPrinter(writer, csvFormat)) {
for (List<String> record : listOrEmpty(csvFile.getRecords())) {
printer.printRecord(record);
for (List<String> csvRecord : listOrEmpty(csvFile.getRecords())) {
printer.printRecord(csvRecord);
}
}
return writer.toString();
Expand All @@ -65,14 +65,14 @@ public static List<String> getHeaders(List<CsvHeader> csvHeaders) {
return headers;
}

public static String recordToString(CSVRecord record) {
return recordToString(record.toList());
public static String recordToString(CSVRecord csvRecord) {
return recordToString(csvRecord.toList());
}

public static String recordToString(List<String> fields) {
return nullOrEmpty(fields)
? ""
: fields.stream().map(str -> str.contains(SEPARATOR) ? quote(str) : str).collect(Collectors.joining(SEPARATOR));
: fields.stream().map(CsvUtil::quoteCsvField).collect(Collectors.joining(SEPARATOR));
}

public static String recordToString(String[] fields) {
Expand All @@ -92,47 +92,52 @@ public static String quote(String field) {
public static String quoteField(List<String> field) {
return nullOrEmpty(field)
? ""
: field.stream()
.map(str -> str.contains(SEPARATOR) || str.contains(FIELD_SEPARATOR) ? quote(str) : str)
.collect(Collectors.joining(FIELD_SEPARATOR));
: field.stream().map(CsvUtil::quoteCsvField).collect(Collectors.joining(FIELD_SEPARATOR));
}

public static List<String> addField(List<String> record, Boolean field) {
record.add(field == null ? "" : field.toString());
return record;
public static List<String> addField(List<String> csvRecord, Boolean field) {
csvRecord.add(field == null ? "" : field.toString());
return csvRecord;
}

public static List<String> addField(List<String> record, String field) {
record.add(field);
return record;
public static List<String> addField(List<String> csvRecord, String field) {
csvRecord.add(field);
return csvRecord;
}

public static List<String> addFieldList(List<String> record, List<String> field) {
record.add(quoteField(field));
return record;
public static List<String> addFieldList(List<String> csvRecord, List<String> field) {
csvRecord.add(quoteField(field));
return csvRecord;
}

public static List<String> addEntityReferences(List<String> record, List<EntityReference> refs) {
record.add(
public static List<String> addEntityReferences(List<String> csvRecord, List<EntityReference> refs) {
csvRecord.add(
nullOrEmpty(refs)
? null
: refs.stream().map(EntityReference::getFullyQualifiedName).collect(Collectors.joining(FIELD_SEPARATOR)));
return record;
return csvRecord;
}

public static List<String> addEntityReference(List<String> record, EntityReference ref) {
record.add(nullOrEmpty(ref) ? null : ref.getFullyQualifiedName());
return record;
public static List<String> addEntityReference(List<String> csvRecord, EntityReference ref) {
csvRecord.add(nullOrEmpty(ref) ? null : ref.getFullyQualifiedName());
return csvRecord;
}

public static List<String> addTagLabels(List<String> record, List<TagLabel> tags) {
record.add(
public static List<String> addTagLabels(List<String> csvRecord, List<TagLabel> tags) {
csvRecord.add(
nullOrEmpty(tags) ? null : tags.stream().map(TagLabel::getTagFQN).collect(Collectors.joining(FIELD_SEPARATOR)));
return record;
return csvRecord;
}

public static List<String> addOwner(List<String> record, EntityReference owner) {
record.add(nullOrEmpty(owner) ? null : owner.getType() + FIELD_SEPARATOR + owner.getName());
return record;
public static List<String> addOwner(List<String> csvRecord, EntityReference owner) {
csvRecord.add(nullOrEmpty(owner) ? null : owner.getType() + FIELD_SEPARATOR + owner.getName());
return csvRecord;
}

private static String quoteCsvField(String str) {
if (str.contains(SEPARATOR) || str.contains(FIELD_SEPARATOR)) {
return quote(str);
}
return str;
}
}
103 changes: 53 additions & 50 deletions openmetadata-service/src/main/java/org/openmetadata/csv/EntityCsv.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
*/
@Slf4j
public abstract class EntityCsv<T extends EntityInterface> {
public static final String FIELD_ERROR_MSG = "#%s: Field %d error - %s";
public static final String IMPORT_STATUS_HEADER = "status";
public static final String IMPORT_STATUS_DETAILS = "details";
public static final String IMPORT_STATUS_SUCCESS = "success";
Expand Down Expand Up @@ -103,8 +104,8 @@ public final CsvImportResult importCsv(String csv, boolean dryRun) throws IOExce

// Validate and load each record
while (records.hasNext()) {
CSVRecord record = records.next();
processRecord(resultsPrinter, expectedHeaders, record);
CSVRecord csvRecord = records.next();
processRecord(resultsPrinter, expectedHeaders, csvRecord);
}

// Finally, create the entities parsed from the record
Expand All @@ -114,7 +115,7 @@ public final CsvImportResult importCsv(String csv, boolean dryRun) throws IOExce
}

/** Implement this method to a CSV record and turn it into an entity */
protected abstract T toEntity(CSVPrinter resultsPrinter, CSVRecord record) throws IOException;
protected abstract T toEntity(CSVPrinter resultsPrinter, CSVRecord csvRecord) throws IOException;

public final String exportCsv(List<T> entities) throws IOException {
CsvFile csvFile = new CsvFile().withHeaders(csvHeaders);
Expand Down Expand Up @@ -143,21 +144,21 @@ public static CsvDocumentation getCsvDocumentation(String entityType) {
protected abstract List<String> toRecord(T entity);

/** Owner field is in entityType;entityName format */
public EntityReference getOwner(CSVPrinter printer, CSVRecord record, int fieldNumber) throws IOException {
String owner = record.get(fieldNumber);
public EntityReference getOwner(CSVPrinter printer, CSVRecord csvRecord, int fieldNumber) throws IOException {
String owner = csvRecord.get(fieldNumber);
if (nullOrEmpty(owner)) {
return null;
}

List<String> list = CsvUtil.fieldToStrings(owner);
if (list.size() != 2) {
importFailure(printer, invalidOwner(fieldNumber), record);
importFailure(printer, invalidOwner(fieldNumber), csvRecord);
}
return getEntityReference(printer, record, fieldNumber, list.get(0), list.get(1));
return getEntityReference(printer, csvRecord, fieldNumber, list.get(0), list.get(1));
}

protected final Boolean getBoolean(CSVPrinter printer, CSVRecord record, int fieldNumber) throws IOException {
String field = record.get(fieldNumber);
protected final Boolean getBoolean(CSVPrinter printer, CSVRecord csvRecord, int fieldNumber) throws IOException {
String field = csvRecord.get(fieldNumber);
if (nullOrEmpty(field)) {
return null;
}
Expand All @@ -167,15 +168,15 @@ protected final Boolean getBoolean(CSVPrinter printer, CSVRecord record, int fie
if (field.equals(Boolean.FALSE.toString())) {
return false;
}
importFailure(printer, invalidBoolean(fieldNumber, field), record);
importFailure(printer, invalidBoolean(fieldNumber, field), csvRecord);
processRecord = false;
return false;
}

protected final EntityReference getEntityReference(
CSVPrinter printer, CSVRecord record, int fieldNumber, String entityType) throws IOException {
String fqn = record.get(fieldNumber);
return getEntityReference(printer, record, fieldNumber, entityType, fqn);
CSVPrinter printer, CSVRecord csvRecord, int fieldNumber, String entityType) throws IOException {
String fqn = csvRecord.get(fieldNumber);
return getEntityReference(printer, csvRecord, fieldNumber, entityType, fqn);
}

protected EntityInterface getEntityByName(String entityType, String fqn) {
Expand All @@ -188,29 +189,29 @@ protected EntityInterface getEntityByName(String entityType, String fqn) {
}

protected final EntityReference getEntityReference(
CSVPrinter printer, CSVRecord record, int fieldNumber, String entityType, String fqn) throws IOException {
CSVPrinter printer, CSVRecord csvRecord, int fieldNumber, String entityType, String fqn) throws IOException {
if (nullOrEmpty(fqn)) {
return null;
}
EntityInterface entity = getEntityByName(entityType, fqn);
if (entity == null) {
importFailure(printer, entityNotFound(fieldNumber, fqn), record);
importFailure(printer, entityNotFound(fieldNumber, fqn), csvRecord);
processRecord = false;
return null;
}
return entity.getEntityReference();
}

protected final List<EntityReference> getEntityReferences(
CSVPrinter printer, CSVRecord record, int fieldNumber, String entityType) throws IOException {
String fqns = record.get(fieldNumber);
CSVPrinter printer, CSVRecord csvRecord, int fieldNumber, String entityType) throws IOException {
String fqns = csvRecord.get(fieldNumber);
if (nullOrEmpty(fqns)) {
return null;
}
List<String> fqnList = listOrEmpty(CsvUtil.fieldToStrings(fqns));
List<EntityReference> refs = new ArrayList<>();
for (String fqn : fqnList) {
EntityReference ref = getEntityReference(printer, record, fieldNumber, entityType, fqn);
EntityReference ref = getEntityReference(printer, csvRecord, fieldNumber, entityType, fqn);
if (!processRecord) {
return null;
}
Expand All @@ -221,9 +222,9 @@ protected final List<EntityReference> getEntityReferences(
return refs.isEmpty() ? null : refs;
}

protected final List<TagLabel> getTagLabels(CSVPrinter printer, CSVRecord record, int fieldNumber)
protected final List<TagLabel> getTagLabels(CSVPrinter printer, CSVRecord csvRecord, int fieldNumber)
throws IOException {
List<EntityReference> refs = getEntityReferences(printer, record, fieldNumber, Entity.TAG);
List<EntityReference> refs = getEntityReferences(printer, csvRecord, fieldNumber, Entity.TAG);
if (!processRecord || nullOrEmpty(refs)) {
return null;
}
Expand Down Expand Up @@ -261,61 +262,61 @@ private Iterator<CSVRecord> parse(String csv) {
return null;
}

private boolean validateHeaders(List<String> expectedHeaders, CSVRecord record) {
importResult.withNumberOfRowsProcessed((int) record.getRecordNumber());
if (expectedHeaders.equals(record.toList())) {
private boolean validateHeaders(List<String> expectedHeaders, CSVRecord csvRecord) {
importResult.withNumberOfRowsProcessed((int) csvRecord.getRecordNumber());
if (expectedHeaders.equals(csvRecord.toList())) {
return true;
}
importResult.withNumberOfRowsFailed(1);
documentFailure(invalidHeader(recordToString(expectedHeaders), recordToString(record)));
documentFailure(invalidHeader(recordToString(expectedHeaders), recordToString(csvRecord)));
return false;
}

private void processRecord(CSVPrinter resultsPrinter, List<String> expectedHeader, CSVRecord record)
private void processRecord(CSVPrinter resultsPrinter, List<String> expectedHeader, CSVRecord csvRecord)
throws IOException {
processRecord = true;
// Every row must have total fields corresponding to the number of headers
if (csvHeaders.size() != record.size()) {
importFailure(resultsPrinter, invalidFieldCount(expectedHeader.size(), record.size()), record);
if (csvHeaders.size() != csvRecord.size()) {
importFailure(resultsPrinter, invalidFieldCount(expectedHeader.size(), csvRecord.size()), csvRecord);
return;
}

// Check if required values are present
List<String> errors = new ArrayList<>();
for (int i = 0; i < csvHeaders.size(); i++) {
String field = record.get(i);
String field = csvRecord.get(i);
boolean fieldRequired = Boolean.TRUE.equals(csvHeaders.get(i).getRequired());
if (fieldRequired && nullOrEmpty(field)) {
errors.add(fieldRequired(i));
}
}

if (!errors.isEmpty()) {
importFailure(resultsPrinter, String.join(FIELD_SEPARATOR, errors), record);
importFailure(resultsPrinter, String.join(FIELD_SEPARATOR, errors), csvRecord);
return;
}

// Finally, convert record into entity for importing
T entity = toEntity(resultsPrinter, record);
T entity = toEntity(resultsPrinter, csvRecord);
if (entity != null) {
// Finally, create entities
createEntity(resultsPrinter, record, entity);
createEntity(resultsPrinter, csvRecord, entity);
}
}

private void createEntity(CSVPrinter resultsPrinter, CSVRecord record, T entity) throws IOException {
private void createEntity(CSVPrinter resultsPrinter, CSVRecord csvRecord, T entity) throws IOException {
entity.setId(UUID.randomUUID());
entity.setUpdatedBy(importedBy);
entity.setUpdatedAt(System.currentTimeMillis());
EntityRepository<T> repository = (EntityRepository<T>) Entity.getEntityRepository(entityType);
Response.Status responseStatus;
if (!importResult.getDryRun()) {
if (Boolean.FALSE.equals(importResult.getDryRun())) {
try {
repository.prepareInternal(entity);
PutResponse<T> response = repository.createOrUpdate(null, entity);
responseStatus = response.getStatus();
} catch (Exception ex) {
importFailure(resultsPrinter, ex.getMessage(), record);
importFailure(resultsPrinter, ex.getMessage(), csvRecord);
return;
}
} else {
Expand All @@ -329,9 +330,9 @@ private void createEntity(CSVPrinter resultsPrinter, CSVRecord record, T entity)
}

if (Response.Status.CREATED.equals(responseStatus)) {
importSuccess(resultsPrinter, record, ENTITY_CREATED);
importSuccess(resultsPrinter, csvRecord, ENTITY_CREATED);
} else {
importSuccess(resultsPrinter, record, ENTITY_UPDATED);
importSuccess(resultsPrinter, csvRecord, ENTITY_UPDATED);
}
}

Expand All @@ -354,22 +355,22 @@ public static String fieldRequired(int field) {
}

public static String invalidField(int field, String error) {
return String.format("#%s: Field %d error - %s", CsvErrorType.INVALID_FIELD, field + 1, error);
return String.format(FIELD_ERROR_MSG, CsvErrorType.INVALID_FIELD, field + 1, error);
}

public static String entityNotFound(int field, String fqn) {
String error = String.format("Entity %s not found", fqn);
return String.format("#%s: Field %d error - %s", CsvErrorType.INVALID_FIELD, field + 1, error);
return String.format(FIELD_ERROR_MSG, CsvErrorType.INVALID_FIELD, field + 1, error);
}

public static String invalidOwner(int field) {
String error = "Owner should be of format user;userName or team;teamName";
return String.format("#%s: Field %d error - %s", CsvErrorType.INVALID_FIELD, field + 1, error);
return String.format(FIELD_ERROR_MSG, CsvErrorType.INVALID_FIELD, field + 1, error);
}

public static String invalidBoolean(int field, String fieldValue) {
String error = String.format("Field %s should be either 'true' of 'false'", fieldValue);
return String.format("#%s: Field %d error - %s", CsvErrorType.INVALID_FIELD, field + 1, error);
return String.format(FIELD_ERROR_MSG, CsvErrorType.INVALID_FIELD, field + 1, error);
}

private void documentFailure(String error) {
Expand All @@ -378,27 +379,29 @@ private void documentFailure(String error) {
}

private void importSuccess(CSVPrinter printer, CSVRecord inputRecord, String successDetails) throws IOException {
List<String> record = listOf(IMPORT_STATUS_SUCCESS, successDetails);
record.addAll(inputRecord.toList());
printer.printRecord(record);
List<String> recordList = listOf(IMPORT_STATUS_SUCCESS, successDetails);
recordList.addAll(inputRecord.toList());
printer.printRecord(recordList);
importResult.withNumberOfRowsProcessed((int) inputRecord.getRecordNumber());
importResult.withNumberOfRowsPassed(importResult.getNumberOfRowsPassed() + 1);
}

protected void importFailure(CSVPrinter printer, String failedReason, CSVRecord inputRecord) throws IOException {
List<String> record = listOf(IMPORT_STATUS_FAILED, failedReason);
record.addAll(inputRecord.toList());
printer.printRecord(record);
List<String> recordList = listOf(IMPORT_STATUS_FAILED, failedReason);
recordList.addAll(inputRecord.toList());
printer.printRecord(recordList);
importResult.withNumberOfRowsProcessed((int) inputRecord.getRecordNumber());
importResult.withNumberOfRowsFailed(importResult.getNumberOfRowsFailed() + 1);
processRecord = false;
}

private void setFinalStatus() {
Status status =
importResult.getNumberOfRowsPassed().equals(importResult.getNumberOfRowsProcessed())
? Status.SUCCESS
: importResult.getNumberOfRowsPassed() > 1 ? Status.PARTIAL_SUCCESS : Status.FAILURE;
Status status = Status.FAILURE;
if (importResult.getNumberOfRowsPassed().equals(importResult.getNumberOfRowsProcessed())) {
status = Status.SUCCESS;
} else if (importResult.getNumberOfRowsPassed() > 1) {
status = Status.PARTIAL_SUCCESS;
}
importResult.setStatus(status);
}
}
Loading

0 comments on commit 647ff49

Please sign in to comment.