Skip to content

improve: naming on ResourceID API #3151

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
csviri wants to merge 1 commit into from
Closed

improve: naming on ResourceID API #3151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 3 additions & 11 deletions ...framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ResourceID.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,9 @@ public boolean equals(Object o) {
return Objects.equals(name, that.name) && Objects.equals(namespace, that.namespace);
}

public boolean isSameResource(HasMetadata hasMetadata) {
Copy link
Collaborator

@metacosm metacosm Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would technically be an API break.

Copy link
Collaborator Author

@csviri csviri Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is true, so not sure to how extent we consider this as an internal API, but since users use this with customt SecondaryToPrimaryMapper it might be an issue.

if (hasMetadata == null) {
return false;
}
final var metadata = hasMetadata.getMetadata();
return isSameResource(metadata.getName(), metadata.getNamespace());
}

/**
* Whether this ResourceID points to the same resource as the one identified by the specified name
* and namespace.
* Whether this ResourceID points to the same resource as the one identified only by the specified
* name and namespace.
*
* <p>Note that this doesn't take API version or Kind into account so this should only be used
* when checking resources that are reasonably expected to be of the same type.
Expand All @@ -83,7 +75,7 @@ public boolean isSameResource(HasMetadata hasMetadata) {
* specified name and namespace, {@code false} otherwise
* @since 5.3.0
*/
public boolean isSameResource(String name, String namespace) {
public boolean equals(String name, String namespace) {
Copy link
Collaborator

@metacosm metacosm Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that equals is a better name (because equals has a special meaning in Java and it doesn't quite match here), though I agree that isSameResource is not great either… maybe equilaventTo? This way, it would be somewhat discoverable if someone is looking for equality-like methods?

Copy link
Collaborator Author

@csviri csviri Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I though overloaded would be too much an issue, but you might be right that naming wise this could raise some eyebrows.

return Objects.equals(this.name, name) && Objects.equals(this.namespace, namespace);
}

Expand Down
6 changes: 3 additions & 3 deletions ...ork-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContextTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,23 +95,23 @@ void latestDistinctKeepsOnlyLatestResourceVersion() {
// Find pod1 in default namespace - should have version 200
final var pod1InDefault =
result.stream()
.filter(r -> ResourceID.fromResource(r).isSameResource("pod1", "default"))
.filter(r -> ResourceID.fromResource(r).equals(new ResourceID("pod1", "default")))
.findFirst()
.orElseThrow();
assertThat(pod1InDefault.getMetadata().getResourceVersion()).isEqualTo("200");

// Find pod2 in default namespace - should exist
HasMetadata pod2InDefault =
result.stream()
.filter(r -> ResourceID.fromResource(r).isSameResource("pod2", "default"))
.filter(r -> ResourceID.fromResource(r).equals("pod2", "default"))
.findFirst()
.orElseThrow();
assertThat(pod2InDefault.getMetadata().getResourceVersion()).isEqualTo("100");

// Find pod1 in other namespace - should exist
HasMetadata pod1InOther =
result.stream()
.filter(r -> ResourceID.fromResource(r).isSameResource("pod1", "other"))
.filter(r -> ResourceID.fromResource(r).equals("pod1", "other"))
.findFirst()
.orElseThrow();
assertThat(pod1InOther.getMetadata().getResourceVersion()).isEqualTo("50");
Expand Down