apache / apache/gravitino

[Improvement] How about modifying the interface of `HasIdentifier`?

Open
#151 9 comments 0 reactions 1 assignee Claimed by @jerryshao View on GitHub
improvement
Dominant language
Java
Stars
3.2k
Forks
935
Avg merge
1d 15h
Merged PRs (30d)
315

Description

### What would you like to be improved?

Currently, the definition of `Entity` has a mixin interface `HasIdentifier` which provides the name identifier of the entity.

```java
public interface HasIdentifier {

/** Return the name of the entity. */
String name();

/** Returns the namespace of the entity. */
default Namespace namespace() {
return Namespace.empty();
}

/** Returns the name identifier of the entity. */
default NameIdentifier nameIdentifier() {
return NameIdentifier.of(namespace(), name());
}
}
```

The problem is that `namespace()` could be null when fetching from the underlying storage, developers use `namespace()` and `nameIdentifier()` will get an ambiguous value.

The reason why the `namespace` could be null is that we don't store `namespace`. And why we don't store `namespace` is that `namespace` could be modified, if we store the namespace, then we need to update all the related entities if the namespace is changed.

### How should we improve?

So to avoid ambiguity, we'd better modify this `HasIdentifier`, one possible way is to remove `namespace()` and `nameIdentifier()` interface, since we can get this from code context.

```java
public interface HasIdentifier {

/** Return the name of the entity. */
String name();
}
```

Another way is to change the interface to set `namespace` implicitly to make sure it's not empty.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.