----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54430/#review158352 -----------------------------------------------------------
catalog/src/main/java/org/apache/atlas/catalog/AtlasTypeSystem.java (line 115) <https://reviews.apache.org/r/54430/#comment229113> I think the methods referring to any graph element shouldn't be here as this is an abstraction for the typesystem. catalog/src/main/java/org/apache/atlas/catalog/DefaultTypeSystem.java (line 61) <https://reviews.apache.org/r/54430/#comment229114> Looks like your system is using tabs, can you revert to spaces for better code alignment. catalog/src/main/java/org/apache/atlas/catalog/DefaultTypeSystem.java (line 99) <https://reviews.apache.org/r/54430/#comment229115> No concatenations in the info string, please use template/variable placeholder "{}" catalog/src/main/java/org/apache/atlas/catalog/DefaultTypeSystem.java (line 114) <https://reviews.apache.org/r/54430/#comment229116> Same here. catalog/src/main/java/org/apache/atlas/catalog/GlossaryCategoryResourceProvider.java (line 58) <https://reviews.apache.org/r/54430/#comment229118> Template instead of string concatenation. - Apoorv Naik On Dec. 7, 2016, 2:42 p.m., David Radley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54430/ > ----------------------------------------------------------- > > (Updated Dec. 7, 2016, 2:42 p.m.) > > > Review request for atlas, DaveEL DaveEL, Shwetha GS, and Hemanth Yamijala. > > > Repository: atlas > > > Description > ------- > > I am looking to make this a small minimum viable contribution evolutionary > change rather than a revolutionary change and have raised follow on > enhancements in Jiras listed below. I do not see the need for a new version > of API with these changes as the APIs are either new or introduce optional > parameters to existing requests and add extra fields in responses. All of > this should be tolerated by existing apps. For example the Atlas UI still > works as before . > > 1) I have adding a glossary category to a taxonomy. It creates an edge in the > graph between the taxonomy (an entity) and the glossary category (an entity) > 2) I can do basic updates and gets for the category (change the description > and rename). > 3) I can add child categories to categories. The children properties are > derived from the edges in the graph, via projections and relationships > 4) You may wonder why the category is an entity not a type or a trait or a > trait instance. It seems that only entities have unique uuids called guids. I > have raised Jira 1245 to get more of these important objects having a guid - > but in the meantime categories are entities > 5) I can add terms (and subterms) to categories - terms are traits and trait > instances. So to go with the code I have amended addtrait to create a new > edge if a parent category has been specified. > 6) I can update the parent category to another category (as long it is in the > same taxonomy, not replacing the top category or trying to add to my children > or to myself). > 7) I was thinking of adds and updates not allowed if the name clashes with > one of the categories parents children.(I will upper case the strings for the > compare). there is a case to not have this check and allow duplicate named > child categories - which could have a different description or different > categories & terms attached. It will work without this restriction; which I > have not coded - maybe this could be on optional constraint held in Ranger. > 8) Part of this change exposes the guid for the taxonomy on the API. > 9) For deletes, I will delete a category. The delete will fail if the > category has children categories or terms. The delete is a soft delete for > the vertex. > 10) I could expose soft deletes, either using the existing isDeleted flag or > introduce a non editable end date which I would need to check. This would > leave an audit trail in the graph. But we would need a hard delete as well . > I am not sure the performance implications of a proliferation of old vertices > in the graph (I would hard delete edges). So for the moment I am leaning > towards adding hard deletes. > 11) At the moment I have just added to the existing V1 taxonomy API, as I > have only added optional fields to existing objects and returned more > information, so the APIs are not being changed in an incompatible way. > 12) I notice that transactions are committed operation by operation - there > is no top level transaction commit and rollback. So if an error occurs - we > could be left with updates occurring to some resources and the graph becoming > inconsistent. I am not sure how we need to work with Titan's eventually > consistent characteristic. I have raised this as a separate issue in Jira > 1252. > 13) Delete taxonomy gets rid of the taxonomies glossary categories > 14) Delete term gets rid of the edge to any parent category and any subterms > (and any subterm edges to categories) > 15) Delete subterm gets rid of the edge to any parent category > 16) Update terms to change the parent category > 17) Allow Rest call to unknit a term from a category DELETE > <<CATEGORY-ID>/terms/<<termname>> > 18) Throw exception when glossary category being called from the entity API > > > An example of what a category get call looks like now in the middle of the > hierarchy with a term is : > > curl --user admin:admin > http://127.0.0.1:21000/api/atlas/v1/taxonomies/Catalog/glossarycategories/8e716b12-ed4c-440e-a7aa-50a6d795526e > [ > { > "href": > "http://127.0.0.1:21000/api/atlas/v1/taxonomies/Catalog/glossarycategories/8e716b12-ed4c-440e-a7aa-50a6d795526e";, > "name": "FooChild2", > "id": "8e716b12-ed4c-440e-a7aa-50a6d795526e", > "description": "xyz", > "creation_time": "2016-10-28:14:21:15", > "parentcategoryid": "2d47baf4-947f-451f-be90-a02c756f94ad", > "taxonomyname": "Catalog", > "children": [ > { > "href": > "http://127.0.0.1:21000/api/atlas/v1/taxonomies/Catalog/glossarycategories/b0fd4ddb-7b30-45fe-9506-355e7c61182f";, > "name": "FooGrandChild" > } > ], > "parent": { > "href": > "http://127.0.0.1:21000/api/atlas/v1/taxonomies/Catalog/glossarycategories/2d47baf4-947f-451f-be90-a02c756f94ad";, > "name": "FooTop" > }, > "terms": [ > { > "href": > "http://127.0.0.1:21000/api/atlas/v1/taxonomies/Catalog/terms/FooTerm1";, > "name": "Catalog.FooTerm1" > } > ] > } > ] > > Some other example calls : > > create top category > curl --user admin:admin -H "Content-Type: application/json" -X POST -d > '{"description":"xyz","parentcategoryid":""}' > http://127.0.0.1:21000/api/atlas/v1/taxonomies/Catalog/glossarycategories/FooTop > > > Create a child cateogry > curl --user admin:admin -H "Content-Type: application/json" -X POST -d > '{"description":"xyz","parentcategoryid":"f8918dbb-4378-4700-bf01-72ecde323344"}' > > http://127.0.0.1:21000/api/atlas/v1/taxonomies/Catalog/glossarycategories/FooChild1 > > > get a category > curl --user admin:admin > http://127.0.0.1:21000/api/atlas/v1/taxonomies/Catalog/glossarycategories/1ac1cb93-1696-4f4d-8a32-f84d38905ddb > > create a term under a category > curl --user admin:admin -H "Content-Type: application/json" -X POST -d > '{"description":"xyz","parentcategoryid":"f73b44c7-9a05-4651-9ad5-44247b49bcaa"}' > http://127.0.0.1:21000/api/atlas/v1/taxonomies/Catalog/terms/FooTerm11 > > > update category parent > curl --user admin:admin -H "Content-Type: application/json" -X PUT -d > '{"parentcategoryid":"4115527f-da0e-4c73-834c-878e2b39022d"}' > http://127.0.0.1:21000/api/atlas/v1/taxonomies/Catalog/glossarycategories/bf41784a-8a7e-4b41-bf77-d75026444a65 > > > update/add terms category parent > curl --user admin:admin -H "Content-Type: application/json" -X PUT -d > '{"description":"xyz","parentcategoryid":"d0287505-96dd-46eb-9dac-bf744b0ed3de"}' > http://127.0.0.1:21000/api/atlas/v1/taxonomies/Catalog/terms/FooTerm11 > > Disconnect a term Term11 from it's parent category > curl --user admin:admin -H "Content-Type: application/json" -X DELETE > http://127.0.0.1:21000/api/atlas/v1/taxonomies/Catalog/glossarycategories/df2205d1-251e-4e75-9b2d-d181a5f1edae/terms/Term11 > > delete category > curl --user admin:admin -H "Content-Type: application/json" -X DELETE > http://127.0.0.1:21000/api/atlas/v1/taxonomies/Catalog/Catalog/glossarycategories/31f726c3-07a5-4e90-9cdd-2900d20a3b2e > > > Notes on the structure and code: > I use 'terms' because taxonomy does already > I have used Glossary Category so that we have the flexibility to add other > types of category later. > I have implemented Glossary Category in the style of taxonomy, I think we > should aspire to bring the glossary objects more fully into the type system. > I decided not to do this in this Jira as I think we need to enhance terms > first (so they can have relationships , have a relationships and guids). I > have raised Jira ATLAS-1344 to track this. > I have decided that children categories will show when the parent is queried > (rather than having a single href which you would then query to see the > children - I think this will be more useful for the usual use cases). > I use parent in terms to point to categories and categories to point to > categories or taxonomy. I can then use the same logic to traverse this tree. > I expose parentcategoryid which ends up on the vertex so can be indexed > I expose the name and href for parents and children as the href is required > for the rest calls and the name is mostly what we will be interested in > first. > The children and parents and terms are represented by edges in the graph and > are calculated from the graph > I have introduced some constants - as I kept misspelling strings. I realise > this is not consistent with the existing code -but I found it so helpful that > I left them in the code. > Currently the edges between categories are hard deleted - I have raised a > Jira to soft delete them. > > Raised Jiras > ATLAS-1326 for recursive delete (all deletion of categories with children) > ATLAS-1327 for fully qualified name (need to work out how to run Gremlin > incantation to produce this). > ATLAS-1245 for terms to implemented as entity (or at least trait with an > guid) > ATLAS-1328 to add related categories and terms > ATLAS-1329 to soft delete edges > ATLAS-1251 UI > aised Jira to address Taxonomy has hard baked V1 in the ResourceDefinition - > which is picked up by the REST calls in hrefs > ATLAS-1330 to add AtlasClient Glossary support. There is currently no support > for terms or taxonomy so I have not added glossary category > ATLAS-1331 investigate query expression enhancements for Glossary > ATLAS-1332 to document the new Glossary Category API > ATLAS-1344 allow types to be created that support parent child hierarchies. > > > Diffs > ----- > > catalog/src/main/java/org/apache/atlas/catalog/AtlasTypeSystem.java > 8f9cd1d34633b1219d01f74f7e825a3716d46bbd > catalog/src/main/java/org/apache/atlas/catalog/DefaultTypeSystem.java > f111eb61156786bb1a4f93b7753cb14f51b85f72 > catalog/src/main/java/org/apache/atlas/catalog/EntityResourceProvider.java > cee102af91af173f2110fb0d355bbcc4fef96cbf > > catalog/src/main/java/org/apache/atlas/catalog/EntityTagResourceProvider.java > c2a843b5b41110b0b309829ab255ca6f93b65721 > > catalog/src/main/java/org/apache/atlas/catalog/GlossaryCategoryResourceProvider.java > PRE-CREATION > > catalog/src/main/java/org/apache/atlas/catalog/GlossaryCategoryVertexWrapper.java > PRE-CREATION > catalog/src/main/java/org/apache/atlas/catalog/ResourceProvider.java > a63309ef305f826a6aae313c655c01d654023d3d > > catalog/src/main/java/org/apache/atlas/catalog/TaxonomyResourceProvider.java > b59dcae0c40fdbfcdb3a87efc7dcc222cbc095f7 > catalog/src/main/java/org/apache/atlas/catalog/TermResourceProvider.java > 3202d040d539a29e3b4acce59f0d108ef6cd4e05 > > catalog/src/main/java/org/apache/atlas/catalog/definition/GlossaryCategoryResourceDefinition.java > PRE-CREATION > > catalog/src/main/java/org/apache/atlas/catalog/definition/GlossaryResourceDefinition.java > PRE-CREATION > > catalog/src/main/java/org/apache/atlas/catalog/definition/TaxonomyResourceDefinition.java > 47d182cc6dd1fb50a39dbadcb023774c2271b808 > > catalog/src/main/java/org/apache/atlas/catalog/definition/TermResourceDefinition.java > 51ef65a5145112127cf6a9ae2d7c6b5a9b59dfe2 > > catalog/src/main/java/org/apache/atlas/catalog/projection/CategoryChildrenRelation.java > PRE-CREATION > > catalog/src/main/java/org/apache/atlas/catalog/projection/CategoryParentRelation.java > PRE-CREATION > > catalog/src/main/java/org/apache/atlas/catalog/projection/TermCategoryParentRelation.java > PRE-CREATION > > catalog/src/main/java/org/apache/atlas/catalog/query/AtlasGlossaryCategoryQuery.java > PRE-CREATION > catalog/src/main/java/org/apache/atlas/catalog/query/QueryFactory.java > a301912e2e1a0fbf3f5c5bfacc03d65f2390bbd7 > > catalog/src/test/java/org/apache/atlas/catalog/GlossaryCategoryResourceProviderTest.java > PRE-CREATION > > catalog/src/test/java/org/apache/atlas/catalog/TaxonomyResourceProviderTest.java > 8dfce5e952c6cfc627fd53bf3bf34e115a460f16 > > catalog/src/test/java/org/apache/atlas/catalog/TermResourceProviderTest.java > 7d61579dc2a1aa542ea2c9c98ed55362b8e80cd8 > > catalog/src/test/java/org/apache/atlas/catalog/definition/GlossaryCategoryResourceDefintionTest.java > PRE-CREATION > > catalog/src/test/java/org/apache/atlas/catalog/definition/TaxonomyResourceDefinitionTest.java > 1af8d14982ef404ff7650fb59b9206a1d273d869 > > catalog/src/test/java/org/apache/atlas/catalog/definition/TermResourceDefinitionTest.java > b7b23da2b329c65682444ccad3c8680a92a18d20 > > repository/src/main/java/org/apache/atlas/repository/MetadataRepository.java > a2a9ab6dafcefaa29110051c35d3b6762017a28f > > repository/src/main/java/org/apache/atlas/repository/graph/DeleteHandler.java > ae1ec45a521660648028add0d63f0f6cadb4a495 > > repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepository.java > 1a3faf778f01582e476792780a9dbaa8a4010d29 > repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java > cdeb117d1f0e9310902c72a2b5217985def9967b > > repository/src/main/java/org/apache/atlas/repository/graph/TypedInstanceToGraphMapper.java > 78e276e5972f6bba68198b3ae5a3d88676cf7585 > > repository/src/main/java/org/apache/atlas/services/DefaultMetadataService.java > d2793d2ce2792ee65ea84ee49afd4b3709942378 > server-api/src/main/java/org/apache/atlas/services/MetadataService.java > e6531848ea5a52d1184e1afd144d089d57edbda8 > typesystem/src/main/java/org/apache/atlas/typesystem/types/TypeSystem.java > d73a7b32add5d9e4eb24afefed042cf894964992 > webapp/src/main/java/org/apache/atlas/web/resources/BaseService.java > dfd29b144d6176bb58ef4a433fa49aa1919946bb > webapp/src/main/java/org/apache/atlas/web/resources/TaxonomyService.java > cc98207caf7618d05d2a13103071ce96b1d9fbb4 > > webapp/src/test/java/org/apache/atlas/web/resources/TaxonomyServiceTest.java > 3f20453f2d2844a39b10ac5a31d883e671907d64 > > Diff: https://reviews.apache.org/r/54430/diff/ > > > Testing > ------- > > Test plan using curl commands > 1) create top category (parentcategoryid = ""). Try to create it again and > get an error. > 2) create 3 children categories under top category, named child1, child2, > child3 > 3) get taxonomy - see the top glossary category > 4) use the returned href to query the top glossary category. Check the > children and parents look as expected > 5) create a term term0 with a description > 6) create a term term1 under child1 with a description > 7) create subterm term1.term2 under child3 > 8) get term0 > 9) get term1 > 10) get child1 > 11) Update glossary category name and description in child1.Then rename it > back to child1. > 12) Update child1 glossary category parentcategoryid to point to child2 > 13) Update child2 glossary category parentcategoryid to point to child1 - > this should fail > 14) Update term0 to put it under child1. Do a get on the term and glossary > category > 15) update subterm term1.term2 to move it under child1 > 16) Unknit term1 from child1. Do a get on the term and glossary category > 17) Delete category child2. This should fail as it has child1 as a child. > 18) Delete category child3. > 19) Delete term0 > 20) Delete the taxonomy. > 21) I have also tested invalid cases like adding/updating categories from > other taxonomies and adding/updating terms from other taxonomies. > > > Thanks, > > David Radley > >
