You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by an...@apache.org on 2015/10/14 12:27:13 UTC
svn commit: r1708579 - in /jackrabbit/oak/trunk/oak-auth-external/src:
main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/
test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/
Author: angela
Date: Wed Oct 14 10:27:13 2015
New Revision: 1708579
URL: http://svn.apache.org/viewvc?rev=1708579&view=rev
Log:
OAK-3510 : Troublesome ExternalIdentityRef.equals(Object) implementation
Added:
jackrabbit/oak/trunk/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/ExternalIdentityRefTest.java
Modified:
jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/ExternalIdentityRef.java
Modified: jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/ExternalIdentityRef.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/ExternalIdentityRef.java?rev=1708579&r1=1708578&r2=1708579&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/ExternalIdentityRef.java (original)
+++ jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/ExternalIdentityRef.java Wed Oct 14 10:27:13 2015
@@ -39,13 +39,13 @@ public class ExternalIdentityRef {
*/
public ExternalIdentityRef(@Nonnull String id, @CheckForNull String providerName) {
this.id = id;
- this.providerName = providerName;
+ this.providerName = (providerName == null || providerName.isEmpty()) ? null : providerName;
StringBuilder b = new StringBuilder();
escape(b, id);
- if (providerName != null && providerName.length() > 0) {
+ if (this.providerName != null) {
b.append(';');
- escape(b, providerName);
+ escape(b, this.providerName);
}
string = b.toString();
}
@@ -82,6 +82,7 @@ public class ExternalIdentityRef {
* @param str the string
* @return the reference
*/
+ @Nonnull
public static ExternalIdentityRef fromString(@Nonnull String str) {
int idx = str.indexOf(';');
if (idx < 0) {
@@ -99,7 +100,7 @@ public class ExternalIdentityRef {
* @param builder the builder
* @param str the string
*/
- private void escape(StringBuilder builder, CharSequence str) {
+ private static void escape(@Nonnull StringBuilder builder, @Nonnull CharSequence str) {
final int len = str.length();
for (int i=0; i<len; i++) {
char c = str.charAt(i);
@@ -119,16 +120,20 @@ public class ExternalIdentityRef {
}
/**
- * Tests if the given object is an external identity reference and if it's getString() is equal to this.
+ * Tests if the given object is an external identity reference and if it's
+ * getString() is equal to this. Note, that there is no need to
+ * include {@code id} and {@code provider} fields in the comparison as
+ * the string representation already incorporates both.
*/
@Override
public boolean equals(Object o) {
- try {
- // assuming that we never compare other types of classes
- return this == o || string.equals(((ExternalIdentityRef) o).string);
- } catch (Exception e) {
- return false;
+ if (this == o) {
+ return true;
+ }
+ if (o instanceof ExternalIdentityRef) {
+ return string.equals(((ExternalIdentityRef) o).string);
}
+ return false;
}
/**
Added: jackrabbit/oak/trunk/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/ExternalIdentityRefTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/ExternalIdentityRefTest.java?rev=1708579&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/ExternalIdentityRefTest.java (added)
+++ jackrabbit/oak/trunk/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/ExternalIdentityRefTest.java Wed Oct 14 10:27:13 2015
@@ -0,0 +1,150 @@
+/*
+ * 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.jackrabbit.oak.spi.security.authentication.external;
+
+import java.util.Map;
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+public class ExternalIdentityRefTest {
+
+ private static final String USERID = "user%id";
+ private static final String PROVIDER_NAME = "provider;Name";
+
+ private ExternalIdentityRef refNullProvider = new ExternalIdentityRef(USERID, null);
+ private ExternalIdentityRef refEmptyProvider = new ExternalIdentityRef(USERID, "");
+ private ExternalIdentityRef ref = new ExternalIdentityRef(USERID, PROVIDER_NAME);
+
+ @Test
+ public void testGetId() {
+ assertEquals(USERID, refNullProvider.getId());
+ assertEquals(USERID, refEmptyProvider.getId());
+ assertEquals(USERID, ref.getId());
+ }
+
+ @Test
+ public void testGetProviderName() {
+ assertNull(refNullProvider.getProviderName());
+ assertNull(refEmptyProvider.getProviderName());
+ assertEquals(PROVIDER_NAME, ref.getProviderName());
+ }
+
+ @Test
+ public void testGetString() {
+ String s = refNullProvider.getString();
+ assertNotNull(s);
+ assertFalse(USERID.equals(s));
+ assertEquals("user%25id", s);
+
+ s = refEmptyProvider.getString();
+ assertNotNull(s);
+ assertFalse(USERID.equals(s));
+ assertEquals("user%25id", s);
+
+ s = ref.getString();
+ assertNotNull(s);
+ assertEquals("user%25id;provider%3bName", s);
+ }
+
+ @Test
+ public void testFromString() {
+ ExternalIdentityRef r = ExternalIdentityRef.fromString(refNullProvider.getString());
+ assertEquals(refNullProvider, r);
+ assertEquals(USERID, r.getId());
+ assertEquals(refNullProvider.getString(), r.getString());
+ assertNull(r.getProviderName());
+
+ r = ExternalIdentityRef.fromString(refEmptyProvider.getString());
+ assertEquals(refEmptyProvider, r);
+ assertEquals(USERID, r.getId());
+ assertEquals(refEmptyProvider.getString(), r.getString());
+ assertNull(r.getProviderName()); // empty provider string is converted to null
+
+ r = ExternalIdentityRef.fromString(ref.getString());
+ assertEquals(ref, r);
+ assertEquals(USERID, r.getId());
+ assertEquals(PROVIDER_NAME, r.getProviderName());
+ assertEquals(ref.getString(), r.getString());
+ }
+
+ @Test
+ public void testEquals() {
+ assertTrue(refNullProvider.equals(refNullProvider));
+ assertTrue(refNullProvider.equals(new ExternalIdentityRef(USERID, refNullProvider.getProviderName())));
+ assertTrue(refNullProvider.equals(new ExternalIdentityRef(USERID, refEmptyProvider.getProviderName())));
+
+ assertTrue(refNullProvider.equals(refEmptyProvider));
+ assertTrue(refEmptyProvider.equals(refNullProvider));
+
+ assertFalse(refNullProvider.equals(ref));
+ assertFalse(ref.equals(refNullProvider));
+
+ assertFalse(refNullProvider.equals(null));
+ assertFalse(refNullProvider.equals(new ExternalIdentityRef("anotherId", null)));
+
+ assertTrue(ref.equals(ref));
+ assertTrue(ref.equals(new ExternalIdentityRef(ref.getId(), ref.getProviderName())));
+ assertTrue(ref.equals(new ExternalIdentityRef(USERID, PROVIDER_NAME)));
+ assertFalse(ref.equals(new ExternalIdentity() {
+ @Nonnull
+ @Override
+ public ExternalIdentityRef getExternalId() {
+ return ref;
+ }
+
+ @Nonnull
+ @Override
+ public String getId() {
+ return ref.getId();
+ }
+
+ @Nonnull
+ @Override
+ public String getPrincipalName() {
+ return ref.getId();
+ }
+
+ @CheckForNull
+ @Override
+ public String getIntermediatePath() {
+ return null;
+ }
+
+ @Nonnull
+ @Override
+ public Iterable<ExternalIdentityRef> getDeclaredGroups() throws ExternalIdentityException {
+ return ImmutableSet.of();
+ }
+
+ @Nonnull
+ @Override
+ public Map<String, ?> getProperties() {
+ return ImmutableMap.of();
+ }
+ }));
+ }
+}