Skip to content

Commit

Permalink
Update equals() for combining filters
Browse files Browse the repository at this point in the history
Resolved an issue where combining filters were not evaluating
equivalency correctly since the size of the lists were not compared.

JiraIssue: DS-49092
  • Loading branch information
kqarryzada committed Aug 9, 2024
1 parent 1b4c1dd commit 60bc1e3
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 12 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ All notable changes to this project will be documented in this file.
This project adheres to [Semantic Versioning](https://semver.org/).

## v3.1.1 - TBD
Fixed an issue where `AndFilter.equals()` and `OrFilter.equals()` could incorrectly evaluate to
true.

## v3.1.0 - 2024-Jun-25
Updated all classes within the UnboundID SCIM 2 SDK to utilize `@Nullable` and `@NotNull`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,19 +101,18 @@ public boolean equals(@Nullable final Object o)
{
return true;
}
if (o == null || getClass() != o.getClass())
if (!(o instanceof AndFilter))
{
return false;
}

CombiningFilter that = (CombiningFilter) o;

if (!getCombinedFilters().containsAll(that.getCombinedFilters()))
AndFilter that = (AndFilter) o;
if (getCombinedFilters().size() != that.getCombinedFilters().size())
{
return false;
}

return true;
return getCombinedFilters().containsAll(that.getCombinedFilters());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
package com.unboundid.scim2.common.filters;

import com.unboundid.scim2.common.annotations.NotNull;
import com.unboundid.scim2.common.annotations.Nullable;

import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -66,7 +65,7 @@ public boolean isCombiningFilter()
* {@inheritDoc}
*/
@Override
@Nullable
@NotNull
public List<Filter> getCombinedFilters()
{
return filterComponents;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,19 +100,18 @@ public boolean equals(@Nullable final Object o)
{
return true;
}
if (o == null || getClass() != o.getClass())
if (!(o instanceof OrFilter))
{
return false;
}

CombiningFilter that = (CombiningFilter) o;

if (!getCombinedFilters().containsAll(that.getCombinedFilters()))
OrFilter that = (OrFilter) o;
if (getCombinedFilters().size() != that.getCombinedFilters().size())
{
return false;
}

return true;
return getCombinedFilters().containsAll(that.getCombinedFilters());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.util.Date;
import java.util.TimeZone;

import static org.assertj.core.api.Assertions.assertThat;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertTrue;
Expand Down Expand Up @@ -337,6 +338,70 @@ public void testGreaterOrEqualFilter() throws Exception
}


/**
* Validates the {@link com.unboundid.scim2.common.filters.AndFilter#equals}
* and {@link com.unboundid.scim2.common.filters.OrFilter#equals} methods.
*/
@Test
@SuppressWarnings({"EqualsWithItself", "ConstantValue"})
public void testCombiningFilterEquals() throws Exception
{
Filter andFilter = Filter.and(
Filter.eq("userName", "Ganon"),
Filter.eq("nickName", "Ganon")
);
Filter orFilter = Filter.or(
Filter.eq("userName", "Ganon"),
Filter.eq("nickName", "Ganon")
);

// A filter instance should always be equivalent to itself.
assertThat(andFilter.equals(andFilter)).isTrue();
assertThat(orFilter.equals(orFilter)).isTrue();

// An initialized AND/OR filter should never be equivalent to 'null'.
assertThat(andFilter.equals(null)).isFalse();
assertThat(orFilter.equals(null)).isFalse();

// An AND filter should not be equivalent to an OR filter, even if it has
// the same subordinate filters.
assertThat(andFilter.equals(orFilter)).isFalse();
assertThat(orFilter.equals(andFilter)).isFalse();

// When the one filter is a superset of the other filter, the filters should
// not be considered equivalent.
Filter andFilterSuperset = Filter.and(
Filter.eq("userName", "Ganon"),
Filter.eq("nickName", "Ganon"),
Filter.sw("title", "Mr")
);
assertThat(andFilter.equals(andFilterSuperset)).isFalse();
assertThat(andFilterSuperset.equals(andFilter)).isFalse();

Filter orFilterSuperset = Filter.or(
Filter.eq("userName", "Ganon"),
Filter.eq("nickName", "Ganon"),
Filter.sw("title", "Mr")
);
assertThat(orFilter.equals(orFilterSuperset)).isFalse();
assertThat(orFilterSuperset.equals(orFilter)).isFalse();

// The order of the subordinate filters should not affect equivalency.
Filter andFilterDifferentOrder = Filter.and(
Filter.eq("nickName", "Ganon"),
Filter.eq("userName", "Ganon")
);
assertThat(andFilter.equals(andFilterDifferentOrder)).isTrue();
assertThat(andFilterDifferentOrder.equals(andFilter)).isTrue();
Filter orFilterDifferentOrder = Filter.or(
Filter.eq("nickName", "Ganon"),
Filter.eq("userName", "Ganon")
);
assertThat(orFilter.equals(orFilterDifferentOrder)).isTrue();
assertThat(orFilterDifferentOrder.equals(orFilter)).isTrue();
}



/**
* Test that filters matching.
Expand Down

0 comments on commit 60bc1e3

Please sign in to comment.