> On Jan. 17, 2018, 3:16 p.m., Yao Li wrote: > > om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/PagingIterator.java > > Lines 88 (patched) > > <https://reviews.apache.org/r/63503/diff/3/?file=1939462#file1939462line88> > > > > in AssetPropertyIteratorBase, you have the constructor > > > > protected AssetPropertyIteratorBase(AssetDescriptor > > parentAsset, > > int > > totalElementCount, > > int > > maxCacheSize) > > { > > /* > > * Initialize superclass. > > */ > > super(parentAsset); > > > > pagingIterator = new PagingIterator(parentAsset, this, > > totalElementCount, maxCacheSize); > > } > > > > I am confused why does "this" have to be checked here whether it is > > null
It tests for null and throws an exception if null is present because the class does not work will a null iterator. Basically null is an invalid value in this case. > On Jan. 17, 2018, 3:16 p.m., Yao Li wrote: > > om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/PagingIterator.java > > Lines 142 (patched) > > <https://reviews.apache.org/r/63503/diff/3/?file=1939462#file1939462line142> > > > > in AssetPropertyIteratorBase, you have the constructor > > > > protected AssetPropertyIteratorBase(AssetDescriptor parentAsset, > > AssetPropertyIteratorBase template) > > { > > /* > > * Initialize superclass. > > */ > > super(parentAsset, template); > > > > if (template != null) > > { > > pagingIterator = new PagingIterator(parentAsset, this, > > template.pagingIterator); > > } > > } > > > > I am confused why does "this" have to be checked here whether it is > > null, here the "this" is always the class AssetPropertyIteratorBase which > > is initialized by Spefice class such as Locations. The code may change over time. We test for values that will cause errors if the class is called incorrectly. Even if we are sure all of the existing callers are working correctly. This is because a future change may create a caller that is behaving incorrectly. This is particularly important in the OCF because there are many callers using the classes. - Mandy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63503/#review195564 ----------------------------------------------------------- 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 > >
