[
https://issues.apache.org/jira/browse/ACCUMULO-2817?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14333683#comment-14333683
]
Josh Elser commented on ACCUMULO-2817:
--------------------------------------
{code}
diff --git
a/core/src/main/java/org/apache/accumulo/core/replication/proto/Replication.java
b/core/src/main/java/org/apache/accumulo/core/replication/proto/Replication.java
index 030f4ec..72f706a 100644
---
a/core/src/main/java/org/apache/accumulo/core/replication/proto/Replication.java
+++
b/core/src/main/java/org/apache/accumulo/core/replication/proto/Replication.java
@@ -449,6 +449,17 @@ package org.apache.accumulo.core.replication.proto;
throws com.google.protobuf.InvalidProtocolBufferException {
return PARSER.parseFrom(data, extensionRegistry);
}
+ public static
org.apache.accumulo.core.replication.proto.Replication.Status parseFrom(
+ byte[] data, int offset, int len)
+ throws com.google.protobuf.InvalidProtocolBufferException {
+ return PARSER.parseFrom(data, offset, len);
+ }
+ public static
org.apache.accumulo.core.replication.proto.Replication.Status parseFrom(
+ byte[] data, int offset, int len,
+ com.google.protobuf.ExtensionRegistryLite extensionRegistry)
+ throws com.google.protobuf.InvalidProtocolBufferException {
+ return PARSER.parseFrom(data, offset, len, extensionRegistry);
+ }
public static
org.apache.accumulo.core.replication.proto.Replication.Status
parseFrom(java.io.InputStream input)
throws java.io.IOException {
return PARSER.parseFrom(input);
{code}
This is generated code. You don't need to be changing it. {{StatusCombiner}}
will just have to be changed to call the necessary method on {{Status}} if no
methods are exposed already.
{code}
try {
Preconditions.checkNotNull(b, "cannot decode null byte array");
} catch (NullPointerException e) {
throw new ValueFormatException(e);
}
try {
Preconditions.checkArgument(offset >= 0, "offset %s cannot be negative",
offset);
} catch (IllegalArgumentException e) {
throw new ValueFormatException(e);
}
try {
Preconditions.checkArgument(offset + len < b.length, "offset + length %s
exceeds byte array length %s",
(offset + len), b.length);
} catch (IllegalArgumentException e) {
throw new ValueFormatException(e);
}
{code}
You don't need to catch the Exceptions thrown by Preconditions. They're
RuntimeExceptions -- just let them filter up.
bq. Do I add "@since" to any public methods, or only ones implementing an
Interface, or all new methods?
Adding it to AbstractEncoder as you did is good. I think AbstractLexicoder is
better off being in {{org.apache.accumulo.core.client.lexicoder.impl}} which
would not need to have the annotation. Generally speaking, if you're
introducing a new class into the [public
API|https://github.com/apache/accumulo/blob/master/README.md#api], you can just
add it at the class level and you don't need the method-level annotations. When
new methods are added into that class later, {{since}} would be added on those
methods.
{code}
/**
* Calls {@link #decodeUnchecked(byte[], int, int)} as {@code
decodeUnchecked(b, 0, b.length);}.
* Does not check if b is null.
*/
@Override
public T decode(byte[] b) {
return decodeUnchecked(b, 0, b.length);
}
{code}
Why not check that {{b}} is non-null?
bq. I looked through semver, and I don't think "any backwards incompatible
changes are introduced to the public API," but feel free to correct me.
The concern is that a user who implemented their own Lexicoder against 1.6.x
would suddenly stop working with 1.7.0. The way you currently did it looks fine
to me (Lexicoder hasn't ultimately changed only the implementations in such a
way that only adds a new method).
> Add offset and limit arguments to byte array Encoder.decode method
> ------------------------------------------------------------------
>
> Key: ACCUMULO-2817
> URL: https://issues.apache.org/jira/browse/ACCUMULO-2817
> Project: Accumulo
> Issue Type: Improvement
> Components: client
> Reporter: Josh Elser
> Assignee: Matt Dailey
> Labels: newbie
> Fix For: 1.7.0
>
> Attachments: ACCUMULO-2817.patch
>
>
> Similar to ACCUMULO-2445, but presently the encoder only works on complete
> byte arrays. This forces an extra copy of the data when it is located in an
> array that contains other information (e.g. a composite key).
> It would be nice to be able to provide offset and length arguments to
> {{Encoder.decode}} so that users can avoid the additional arraycopy.
> Changing to a ByteBuffer instead of byte array argument would also be
> acceptable, but more churn on the API that, unless it's happening globally, I
> would rather avoid. It would also incur the penalty for that extra Object,
> which while minimal alone, could be significant if decoding every value in a
> table, for example.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
