yunchipang commented on code in PR #7476: URL: https://github.com/apache/gravitino/pull/7476#discussion_r2205986659
########## core/src/main/java/org/apache/gravitino/listener/api/event/ListMetadataObjectsForTagsEvent.java: ########## @@ -0,0 +1,63 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.listener.api.event; + +import org.apache.gravitino.annotation.DeveloperApi; +import org.apache.gravitino.utils.NameIdentifierUtil; + +/** + * Represents an event that is triggered upon successfully listing metadata objects for multiple + * tags. + */ +@DeveloperApi +public final class ListMetadataObjectsForTagsEvent extends TagEvent { + private final String[] tagNames; + + /** + * Constructs an instance of {@code ListMetadataObjectsForTagsEvent}. + * + * @param user The username of the individual who initiated the metadata objects listing. + * @param metalake The metalake from which metadata objects were listed. + * @param tagNames The names of the tags for which metadata objects were listed. + */ + public ListMetadataObjectsForTagsEvent(String user, String metalake, String[] tagNames) { + super(user, NameIdentifierUtil.ofTag(metalake, tagNames[0])); // Use first tag for identifier Review Comment: @FANNG1 @mchades since `TagEvent` was designed to handle single tag identifier, the approach that requires the least code to be changed I can see now is to build a multi-tag identifier, for example: ```suggestion super(user, createTagListIdentifier(metalake, tagNames)); ``` and in some common utils dir, ``` private static NameIdentifier createTagListIdentifier(String metalake, String[] tagNames) { String[] sortedTags = Arrays.stream(tagNames).sorted().toArray(String[]::new); String compositeTag = String.join(",", sortedTags); return NameIdentifierUtil.ofTag(metalake, compositeTag); } ``` Pros: represents all tags involved, no breaking change to `TagEvent` Cons: long tags list may exceed identifier limits? If this does not look good for now, how about I go ahead and open an issue for future improvements on this? ########## core/src/main/java/org/apache/gravitino/listener/api/event/ListMetadataObjectsForTagsEvent.java: ########## @@ -0,0 +1,63 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.listener.api.event; + +import org.apache.gravitino.annotation.DeveloperApi; +import org.apache.gravitino.utils.NameIdentifierUtil; + +/** + * Represents an event that is triggered upon successfully listing metadata objects for multiple + * tags. + */ +@DeveloperApi +public final class ListMetadataObjectsForTagsEvent extends TagEvent { + private final String[] tagNames; + + /** + * Constructs an instance of {@code ListMetadataObjectsForTagsEvent}. + * + * @param user The username of the individual who initiated the metadata objects listing. + * @param metalake The metalake from which metadata objects were listed. + * @param tagNames The names of the tags for which metadata objects were listed. + */ + public ListMetadataObjectsForTagsEvent(String user, String metalake, String[] tagNames) { + super(user, NameIdentifierUtil.ofTag(metalake, tagNames[0])); // Use first tag for identifier Review Comment: @FANNG1 @mchades since `TagEvent` was designed to handle single tag identifier, the approach that requires the least code to be changed I can see now is to build a multi-tag identifier, for example: ```suggestion super(user, createTagListIdentifier(metalake, tagNames)); ``` and in some common utils dir, ``` private static NameIdentifier createTagListIdentifier(String metalake, String[] tagNames) { String[] sortedTags = Arrays.stream(tagNames).sorted().toArray(String[]::new); String compositeTag = String.join(",", sortedTags); return NameIdentifierUtil.ofTag(metalake, compositeTag); } ``` Pros: represents all tags involved, no breaking change to `TagEvent` Cons: long tags list may exceed identifier limits? If this does not look good for now, how about I go ahead and open an issue for future improvements on this? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
