----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69305/#review210482 -----------------------------------------------------------
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerDelegate.java Lines 41 (patched) <https://reviews.apache.org/r/69305/#comment295183> Instead of defining DELETE_TYPE enum here, consider adding in EntityREST.java, so that the possible values will be obvious to REST API users. Also rename DELETE_TYPE as DeleteType - as shown below: public enum DeleteType { DEFAULT, SOFT, HARD } repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerDelegate.java Lines 64 (patched) <https://reviews.apache.org/r/69305/#comment295194> Given only 3 entries, consider avoding use of map; instead use 3 'final' members: final SoftDeleteHandlerV1 softDeleteHandler; final HardDeleteHandlerV1 hardDeleteHandler; final DeleteHandlerV1 defaultDeleteHandler; and initialize them in the constructor; and use 'switch/case' to pick appropriate handler to use. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerDelegate.java Lines 83 (patched) <https://reviews.apache.org/r/69305/#comment295193> replace isSoftDeleteHandler(SoftDeleteHandlerV1 softDeleteHandler) with: DeleteHandlerV1 getDefaultDeleteHandler() { try { return ApplicationProperties.getClass(config, DELETE_HANDLER_V1_IMPLEMENTATION_PROPERTY, SoftDeleteHandlerV1.class.getName(), DeleteHandlerV1.class); } catch (AtlasException e) { throw new RuntimeException(e); } } repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java Lines 936 (patched) <https://reviews.apache.org/r/69305/#comment295189> is method by() needed anymore? If not, please remove. repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2.java Lines 436 (patched) <https://reviews.apache.org/r/69305/#comment295190> replace null with DEFAULT - just as in other such places (#358, #441). webapp/src/main/java/org/apache/atlas/web/resources/EntityResource.java Line 584 (original), 584 (patched) <https://reviews.apache.org/r/69305/#comment295191> replace null with DeleteMode.DEFAULT webapp/src/main/java/org/apache/atlas/web/rest/EntityREST.java Lines 246 (patched) <https://reviews.apache.org/r/69305/#comment295192> Consider using DeleteType enum as the type, instead of String. Also, consider specifying @DefaultValue("DEFAULT") here (reference - LineageREST.java). Review & update other methods here for above. - Madhan Neethiraj On Nov. 9, 2018, 7:50 p.m., Ashutosh Mestry wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69305/ > ----------------------------------------------------------- > > (Updated Nov. 9, 2018, 7:50 p.m.) > > > Review request for atlas, Kapildeo Nayak, Madhan Neethiraj, Nikhil Bonte, > Nixon Rodrigues, and Sarath Subramanian. > > > Bugs: ATLAS-2774 > https://issues.apache.org/jira/browse/ATLAS-2774 > > > Repository: atlas > > > Description > ------- > > **Approach** > - New: _DeleteHandlerDelegate_. This is a decorator over _DeleteHandlerV1_. > In its implementation it encapsulates: > - _SoftDeleteHandler_: Existing implementation. > - _HardDeleteHandler_: Existing implementation. > - _DefaultHandler_ which is initialized via _atlas-application.properties_. > > - Modified: _EntityREST_ to accept additional parameter to specify if the > delete is hard or soft. > - Modified: _AtlasEntityStoreV2_ to accept additional parameter. Existing > APIs delegate their function to the new more generic method. > - Modified: _ConditionalOnAtlasProperty_ to be applied on field instead of > class. This initializes the handler from _atlas-application.properties_. > > **CURL** > > Hard > ``` > curl -X DELETE > 'http://localhost:21000/api/atlas/v2/entity/guid/abc8c380-2bfe-47aa-b405-5b79ea9a2a0d?hard=true' > ``` > > Soft > ``` > curl -X DELETE > 'http://localhost:21000/api/atlas/v2/entity/guid/abc8c380-2bfe-47aa-b405-5b79ea9a2a0d?hard=false' > ``` > > Default: As initialized by application based on _atlas-application.properties_ > ``` > curl -X DELETE > 'http://localhost:21000/api/atlas/v2/entity/guid/abc8c380-2bfe-47aa-b405-5b79ea9a2a0d' > ``` > > > Diffs > ----- > > > common/src/main/java/org/apache/atlas/annotation/ConditionalOnAtlasProperty.java > 427e3a8f8 > > repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasEntityStore.java > 750fa1775 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerDelegate.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java > c57e30ab5 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/HardDeleteHandlerV1.java > edf1eed4c > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/SoftDeleteHandlerV1.java > 0bcf0d6cb > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2.java > 733369692 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasRelationshipStoreV2.java > 21617dcc3 > > repository/src/main/java/org/apache/atlas/util/AtlasRepositoryConfiguration.java > bf16145a3 > repository/src/test/java/org/apache/atlas/TestModules.java d3d30d5d9 > webapp/src/main/java/org/apache/atlas/web/resources/EntityResource.java > 00b29e6c8 > webapp/src/main/java/org/apache/atlas/web/rest/EntityREST.java 68c132c37 > webapp/src/test/java/org/apache/atlas/web/adapters/TestEntityREST.java > 6739364ac > > webapp/src/test/java/org/apache/atlas/web/adapters/TestEntityRESTDelete.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/69305/diff/2/ > > > Testing > ------- > > **Unit tests** > Additional unit tests. > > **Functional tests** > - Via REST APIs. (See above.) > - Tested with > atlas.DeleteHandlerV1.impl=org.apache.atlas.repository.store.graph.v1.SoftDeleteHandlerV1 > - Tested with > atlas.DeleteHandlerV1.impl=org.apache.atlas.repository.store.graph.v1.HardDeleteHandlerV1 > > > Thanks, > > Ashutosh Mestry > >
