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 fr...@apache.org on 2015/10/23 11:45:31 UTC
svn commit: r1710162 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak:
api/PropertyState.java api/Tree.java api/Type.java api/package-info.java
plugins/memory/ModifiedNodeState.java plugins/memory/package-info.java
Author: frm
Date: Fri Oct 23 09:45:31 2015
New Revision: 1710162
URL: http://svn.apache.org/viewvc?rev=1710162&view=rev
Log:
OAK-3544 - Remove dependencies on Guava from the o.a.j.o.api package
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/PropertyState.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Tree.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Type.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/package-info.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/ModifiedNodeState.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/package-info.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/PropertyState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/PropertyState.java?rev=1710162&r1=1710161&r2=1710162&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/PropertyState.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/PropertyState.java Fri Oct 23 09:45:31 2015
@@ -17,9 +17,6 @@
package org.apache.jackrabbit.oak.api;
import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
-
-import com.google.common.base.Function;
/**
* Immutable property state. A property consists of a name and a value.
@@ -119,19 +116,4 @@ public interface PropertyState {
*/
int count();
- /**
- * Mapping from a PropertyState instance to its name.
- */
- Function<PropertyState, String> GET_NAME =
- new Function<PropertyState, String>() {
- @Override @Nullable
- public String apply(@Nullable PropertyState input) {
- if (input != null) {
- return input.getName();
- } else {
- return null;
- }
- }
- };
-
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Tree.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Tree.java?rev=1710162&r1=1710161&r2=1710162&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Tree.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Tree.java Fri Oct 23 09:45:31 2015
@@ -22,8 +22,6 @@ import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
-import com.google.common.base.Function;
-
/**
* A tree instance represents a snapshot of the {@link ContentRepository}
* tree at the time the instance was acquired from a {@link ContentSession}.
@@ -349,19 +347,4 @@ public interface Tree {
*/
Tree[] EMPTY_ARRAY = new Tree[0];
- /**
- * Mapping from a Tree instance to its name.
- */
- Function<Tree, String> GET_NAME =
- new Function<Tree, String>() {
- @Override @Nullable
- public String apply(@Nullable Tree input) {
- if (input != null) {
- return input.getName();
- } else {
- return null;
- }
- }
- };
-
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Type.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Type.java?rev=1710162&r1=1710161&r2=1710162&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Type.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Type.java Fri Oct 23 09:45:31 2015
@@ -19,17 +19,12 @@
package org.apache.jackrabbit.oak.api;
import java.math.BigDecimal;
+import java.util.HashMap;
import java.util.Map;
import javax.annotation.Nonnull;
import javax.jcr.PropertyType;
-import com.google.common.base.Objects;
-import com.google.common.collect.ComparisonChain;
-
-import static com.google.common.base.Preconditions.checkState;
-import static com.google.common.collect.Maps.newHashMap;
-
/**
* Instances of this class map Java types to {@link PropertyType property types}.
* Passing an instance of this class to {@link PropertyState#getValue(Type)} determines
@@ -38,7 +33,7 @@ import static com.google.common.collect.
*/
public final class Type<T> implements Comparable<Type<?>> {
- private static final Map<String, Type<?>> TYPES = newHashMap();
+ private static final Map<String, Type<?>> TYPES = new HashMap<String, Type<?>>();
private static <T> Type<T> create(int tag, boolean array, String string) {
Type<T> type = new Type<T>(tag, array, string);
@@ -242,10 +237,23 @@ public final class Type<T> implements Co
@Override
public int compareTo(@Nonnull Type<?> that) {
- return ComparisonChain.start()
- .compare(tag, that.tag)
- .compareFalseFirst(array, that.array)
- .result();
+ if (tag < that.tag) {
+ return -1;
+ }
+
+ if (tag > that.tag) {
+ return 1;
+ }
+
+ if (!array && that.array) {
+ return -1;
+ }
+
+ if (array && !that.array) {
+ return 1;
+ }
+
+ return 0;
}
//------------------------------------------------------------< Object >--
@@ -257,7 +265,22 @@ public final class Type<T> implements Co
@Override
public int hashCode() {
- return Objects.hashCode(tag, array);
+ int result = 1;
+
+ result = result + 31 * tag;
+ result = result + 31 * hashCode(array);
+
+ return result;
+ }
+
+ private int hashCode(boolean value) {
+ return value ? 1231 : 1237;
+ }
+
+ private void checkState(boolean condition, String message) {
+ if (!condition) {
+ throw new IllegalStateException(message);
+ }
}
@Override
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/package-info.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/package-info.java?rev=1710162&r1=1710161&r2=1710162&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/package-info.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/package-info.java Fri Oct 23 09:45:31 2015
@@ -18,7 +18,7 @@
/**
* Oak repository API
*/
-@Version("2.1")
+@Version("3.0.0")
@Export(optional = "provide:=true")
package org.apache.jackrabbit.oak.api;
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/ModifiedNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/ModifiedNodeState.java?rev=1710162&r1=1710161&r2=1710162&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/ModifiedNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/ModifiedNodeState.java Fri Oct 23 09:45:31 2015
@@ -32,7 +32,9 @@ import java.util.Map;
import java.util.Map.Entry;
import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import com.google.common.base.Function;
import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.spi.state.AbstractNodeState;
import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry;
@@ -49,6 +51,21 @@ import com.google.common.base.Predicates
public class ModifiedNodeState extends AbstractNodeState {
/**
+ * Mapping from a PropertyState instance to its name.
+ */
+ private static final Function<PropertyState, String> GET_NAME =
+ new Function<PropertyState, String>() {
+ @Override @Nullable
+ public String apply(@Nullable PropertyState input) {
+ if (input != null) {
+ return input.getName();
+ } else {
+ return null;
+ }
+ }
+ };
+
+ /**
* Unwraps the given {@code NodeState} instance into the given internals
* of a {@link MutableNodeState} instance that is being created or reset.
* <p>
@@ -164,7 +181,7 @@ public class ModifiedNodeState extends A
properties = newHashMap(properties);
}
Predicate<PropertyState> predicate = Predicates.compose(
- not(in(properties.keySet())), PropertyState.GET_NAME);
+ not(in(properties.keySet())), GET_NAME);
return concat(
filter(base.getProperties(), predicate),
filter(properties.values(), notNull()));
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/package-info.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/package-info.java?rev=1710162&r1=1710161&r2=1710162&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/package-info.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/package-info.java Fri Oct 23 09:45:31 2015
@@ -14,7 +14,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-@Version("1.0")
+@Version("2.0.0")
@Export(optional = "provide:=true")
package org.apache.jackrabbit.oak.plugins.memory;
Re: svn commit: r1710162 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak:
api/PropertyState.java api/Tree.java api/Type.java api/package-info.java
plugins/memory/ModifiedNodeState.java plugins/memory/package-info.java
Posted by Angela Schreiber <an...@adobe.com>.
hi francesco
i disagree with your statement below. having tests even for trivial
changes just limits the risk of regressions now and for any later
modifications... hoping for others having covered your code with
some test, is IMO not good enough :-)
so, please add some tests or verify that it's actually covered
by some others (intelliJ actually allows you to visualise that
pretty easily when running tests with colllecting test-coverage).
kind regards
angela
On 23/10/15 12:13, "Francesco Mari" <ma...@gmail.com> wrote:
>I don't think that in this case a test is necessary, given the
>triviality of the change. Type is used by almost the whole stack, and
>the fixes are about the comparison between types. If I introduced a
>regression, it should have been caught by at least one test
>(hopefully).
>
>2015-10-23 12:08 GMT+02:00 Chetan Mehrotra <ch...@gmail.com>:
>> On Fri, Oct 23, 2015 at 3:15 PM, <fr...@apache.org> wrote:
>>> public final class Type<T> implements Comparable<Type<?>> {
>>>
>>> - private static final Map<String, Type<?>> TYPES = newHashMap();
>>> + private static final Map<String, Type<?>> TYPES = new
>>>HashMap<String, Type<?>>();
>>>
>>> private static <T> Type<T> create(int tag, boolean array, String
>>>string) {
>>> Type<T> type = new Type<T>(tag, array, string);
>>> @@ -242,10 +237,23 @@ public final class Type<T> implements Co
>>>
>>> @Override
>>> public int compareTo(@Nonnull Type<?> that) {
>>> - return ComparisonChain.start()
>>> - .compare(tag, that.tag)
>>> - .compareFalseFirst(array, that.array)
>>> - .result();
>>> + if (tag < that.tag) {
>>> + return -1;
>>> + }
>>> +
>>> + if (tag > that.tag) {
>>> + return 1;
>>> + }
>>> +
>>> + if (!array && that.array) {
>>> + return -1;
>>> + }
>>> +
>>> + if (array && !that.array) {
>>> + return 1;
>>> + }
>>> +
>>> + return 0;
>>> }
>>
>> I am fine with removing dependency on Guava from API but not sure if
>> we should remove it from implementation side
>>
>> Also it would be good to have a testcase to validate the above logic
>> if we are removing dependency from a tested Guava utility method
>>
>> Chetan Mehrotra
Re: svn commit: r1710162 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak:
api/PropertyState.java api/Tree.java api/Type.java api/package-info.java
plugins/memory/ModifiedNodeState.java plugins/memory/package-info.java
Posted by Francesco Mari <ma...@gmail.com>.
I don't think that in this case a test is necessary, given the
triviality of the change. Type is used by almost the whole stack, and
the fixes are about the comparison between types. If I introduced a
regression, it should have been caught by at least one test
(hopefully).
2015-10-23 12:08 GMT+02:00 Chetan Mehrotra <ch...@gmail.com>:
> On Fri, Oct 23, 2015 at 3:15 PM, <fr...@apache.org> wrote:
>> public final class Type<T> implements Comparable<Type<?>> {
>>
>> - private static final Map<String, Type<?>> TYPES = newHashMap();
>> + private static final Map<String, Type<?>> TYPES = new HashMap<String, Type<?>>();
>>
>> private static <T> Type<T> create(int tag, boolean array, String string) {
>> Type<T> type = new Type<T>(tag, array, string);
>> @@ -242,10 +237,23 @@ public final class Type<T> implements Co
>>
>> @Override
>> public int compareTo(@Nonnull Type<?> that) {
>> - return ComparisonChain.start()
>> - .compare(tag, that.tag)
>> - .compareFalseFirst(array, that.array)
>> - .result();
>> + if (tag < that.tag) {
>> + return -1;
>> + }
>> +
>> + if (tag > that.tag) {
>> + return 1;
>> + }
>> +
>> + if (!array && that.array) {
>> + return -1;
>> + }
>> +
>> + if (array && !that.array) {
>> + return 1;
>> + }
>> +
>> + return 0;
>> }
>
> I am fine with removing dependency on Guava from API but not sure if
> we should remove it from implementation side
>
> Also it would be good to have a testcase to validate the above logic
> if we are removing dependency from a tested Guava utility method
>
> Chetan Mehrotra
Re: svn commit: r1710162 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak:
api/PropertyState.java api/Tree.java api/Type.java api/package-info.java
plugins/memory/ModifiedNodeState.java plugins/memory/package-info.java
Posted by Chetan Mehrotra <ch...@gmail.com>.
On Fri, Oct 23, 2015 at 3:15 PM, <fr...@apache.org> wrote:
> public final class Type<T> implements Comparable<Type<?>> {
>
> - private static final Map<String, Type<?>> TYPES = newHashMap();
> + private static final Map<String, Type<?>> TYPES = new HashMap<String, Type<?>>();
>
> private static <T> Type<T> create(int tag, boolean array, String string) {
> Type<T> type = new Type<T>(tag, array, string);
> @@ -242,10 +237,23 @@ public final class Type<T> implements Co
>
> @Override
> public int compareTo(@Nonnull Type<?> that) {
> - return ComparisonChain.start()
> - .compare(tag, that.tag)
> - .compareFalseFirst(array, that.array)
> - .result();
> + if (tag < that.tag) {
> + return -1;
> + }
> +
> + if (tag > that.tag) {
> + return 1;
> + }
> +
> + if (!array && that.array) {
> + return -1;
> + }
> +
> + if (array && !that.array) {
> + return 1;
> + }
> +
> + return 0;
> }
I am fine with removing dependency on Guava from API but not sure if
we should remove it from implementation side
Also it would be good to have a testcase to validate the above logic
if we are removing dependency from a tested Guava utility method
Chetan Mehrotra