> On Jan. 17, 2018, 9:39 a.m., Yao Li wrote:
> > om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/AdditionalProperties.java
> > Lines 56 (patched)
> > <https://reviews.apache.org/r/63503/diff/3/?file=1939412#file1939412line56>
> >
> > here you checked whether additionalProperties is null, but in most
> > constructors you did not check, i am not sure whether it is necessary
The code should check for null in the copy/clone constructor and not in the
typical/default constructor. This is because in the typical/default
constructor, the parameter objects are created for the use of the new object.
The copy/clone constructor does need to test for null because the object passed
in is not for its use - it is just a template to use. The copy/clone should
only create a new object if the provided object is not null.
In this case there is a problem because the code should test for null but also
do the clone properly - it is not so it is wrong. The code should be:
if (additionalProperties != null)
{
this.additionalProperties = new HashMap<>(additionalProperties);
}
- Mandy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63503/#review195546
-----------------------------------------------------------
On Jan. 12, 2018, 1:13 p.m., Mandy Chessell wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63503/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2018, 1:13 p.m.)
>
>
> Review request for atlas and Madhan Neethiraj.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> This patch contains the open connector framework code. This code is located
> in the om-fwk-ocf component and is described in JIRA
> https://issues.apache.org/jira/browse/ATLAS-1095
>
> I have added a new patch to the Jira with fixes from Yao's comments.
> Upgraded Maven to 352 and rebuilt/rerun tests.
>
>
> Diffs
> -----
>
> om-fwk-ocf/README.md PRE-CREATION
> om-fwk-ocf/pom.xml PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/Connector.java PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ConnectorBase.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ConnectorBroker.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ConnectorProvider.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ConnectorProviderBase.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ffdc/ConnectionCheckedException.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ffdc/ConnectorCheckedException.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ffdc/OCFCheckedExceptionBase.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ffdc/OCFErrorCode.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ffdc/OCFRuntimeException.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ffdc/PropertyServerException.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ffdc/README.md PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/AdditionalProperties.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Annotation.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/AnnotationStatus.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Annotations.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/AssetDescriptor.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/AssetDetail.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/AssetPropertyBase.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/AssetPropertyIteratorBase.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/AssetSummary.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/AssetUniverse.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Certification.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Certifications.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Classification.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Classifications.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Comment.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/CommentType.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Comments.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/ConnectedAssetProperties.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Connection.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Connections.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/ConnectorType.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/DerivedSchemaElement.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/ElementHeader.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/ElementOrigin.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/ElementType.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/EmbeddedConnection.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/EmbeddedConnections.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Endpoint.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/ExternalIdentifier.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/ExternalIdentifiers.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/ExternalReference.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/ExternalReferences.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Feedback.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/InformalTag.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/InformalTags.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/KeyPattern.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/License.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Licenses.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Like.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Likes.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Lineage.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Location.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Locations.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/MapSchemaElement.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Meaning.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Meanings.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Note.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/NoteLog.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/NoteLogs.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Notes.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/PagingIterator.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/PrimitiveSchemaElement.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/PropertyBase.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Rating.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Ratings.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Referenceable.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/RelatedAsset.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/RelatedAssetProperties.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/RelatedAssets.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/RelatedMediaReference.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/RelatedMediaReferences.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/RelatedMediaType.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/RelatedMediaUsage.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Schema.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/SchemaAttribute.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/SchemaAttributes.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/SchemaElement.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/SchemaImplementationQueries.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/SchemaImplementationQuery.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/SchemaLink.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/SchemaLinks.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/SchemaType.java
> PRE-CREATION
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/StarRating.java
> PRE-CREATION
>
> om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/VirtualConnection.java
> PRE-CREATION
> pom.xml c64b26e
>
>
> Diff: https://reviews.apache.org/r/63503/diff/3/
>
>
> Testing
> -------
>
> Simple sniff test to create connectors.
>
>
> File Attachments
> ----------------
>
> 0001-ATLAS-1095-initial-code-drop-for-OCF-with-fixes-from.patch
>
> https://reviews.apache.org/media/uploaded/files/2017/11/10/ad9eae3c-bb68-4bdc-b6b9-62ba12f651a0__0001-ATLAS-1095-initial-code-drop-for-OCF-with-fixes-from.patch
>
>
> Thanks,
>
> Mandy Chessell
>
>