You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by melrief <gi...@git.apache.org> on 2016/09/01 00:00:53 UTC
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
GitHub user melrief opened a pull request:
https://github.com/apache/accumulo/pull/145
ACCUMULO-4376 add KeyBuilder
Adds the KeyBuilder as proposed in ACCUMULO-4376. The PR addes a `KeyBuilder` which can be used to add build `Key`s in a mutable way. For the design, I took inspiration from the Step Builder design pattern as described in [Vladimir Stankovic blog](http://www.svlada.com/step-builder-pattern/) to try to give some rules to the way `Key`s are built. I also added a new constructor to `Key` that accepts all the parameters, including `deleted` similar to to the `Key.init` method, that is used to build `Key`s from the fields.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/melrief/accumulo ACCUMULO-4376
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/accumulo/pull/145.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #145
----
commit 4ff5f0cfc1f7f890e68736a35de86925fccddb62
Author: Mario Pastorelli <ma...@teralytics.ch>
Date: 2016-08-31T23:32:40Z
ACCUMULO-4376 add KeyBuilder
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77246511
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,278 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build <code>Key</code>s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the <code>Key</code> are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build<T> {
+
+ /**
+ * Build a <code>Key</code> from this builder.
+ *
+ * @param copyBytes
+ * if the byte arrays should be copied or not. If not, byte arrays will be reused in the resultant <code>Key</code>
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build(boolean copyBytes);
+
+ /**
+ * Build a <code>Key</code> from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the <code>Key</code> created.
+ *
+ * @param timestamp
+ * the timestamp to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the <code>Key</code> to the parameter.
+ *
+ * @param deleted
+ * if the <code>Key</code> should be marked as deleted or not
+ * @return this builder
+ */
+ Build<T> deleted(boolean deleted);
+ }
+
+ public interface ColumnFamilyStep<T> extends ColumnVisibilityStep<T> {
--- End diff --
Maybe I'm wrong by I always had the impression that the columnQualifier needs a columnFamily else it makes no sense. It's true that you could in theory have no family but a qualifier but what's the real use for it? Why not using family not null and qualifier null also for that case?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on the issue:
https://github.com/apache/accumulo/pull/145
After reading @dhutchis comment about Guava I would suggest using the method name `builder()` rather than `newBuilder()`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by madrob <gi...@git.apache.org>.
Github user madrob commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77241833
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,278 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build <code>Key</code>s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the <code>Key</code> are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build<T> {
+
+ /**
+ * Build a <code>Key</code> from this builder.
+ *
+ * @param copyBytes
+ * if the byte arrays should be copied or not. If not, byte arrays will be reused in the resultant <code>Key</code>
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build(boolean copyBytes);
+
+ /**
+ * Build a <code>Key</code> from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the <code>Key</code> created.
+ *
+ * @param timestamp
+ * the timestamp to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the <code>Key</code> to the parameter.
+ *
+ * @param deleted
+ * if the <code>Key</code> should be marked as deleted or not
+ * @return this builder
+ */
+ Build<T> deleted(boolean deleted);
+ }
+
+ public interface ColumnFamilyStep<T> extends ColumnVisibilityStep<T> {
--- End diff --
Should this extend `ColumnQualifierStep`? I can imagine a null cf with a non-null cq.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77282647
--- Diff: core/src/test/java/org/apache/accumulo/core/data/KeyBuilderTest.java ---
@@ -0,0 +1,282 @@
+/*
+ * 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.accumulo.core.data;
+
+import org.apache.hadoop.io.Text;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class KeyBuilderTest {
--- End diff --
Nice test. Very thorough. I tried running code coverage and everything in key builder was covered.
I think it would be useful to test the following. This ensures that the behavior of calling timestamp multiple times is well defined going forward and does not change to take the first.
```java
k = Key.builder().row("r").timestamp(44).timestamp(99).build();
assertEquals(new Key("r","","",99l), k);
```
It would be nice to test that all builder methods fail fast when passed null. I think they all will at the moment. Want to avoid the case where null passed to earlier method causes failure in `build()` method and user does not know if the row, family, or qualifier was null.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77647568
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,420 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.accumulo.core.security.ColumnVisibility;
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build {@link Key}s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the {@link Key} are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ * <code>CharSequence</code>s must be UTF-8 encoded.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.9
+ */
+public class KeyBuilder {
+
+ /**
+ * @since 1.9
+ */
+ public interface Build {
+
+ /**
+ * Build a {@link Key} from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the {@link Key} built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the {@link Key} created.
+ *
+ * @param timestamp
+ * the timestamp to use for the {@link Key}
+ * @return this builder
+ */
+ Build timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the {@link Key} to the parameter.
+ *
+ * @param deleted
+ * if the {@link Key} should be marked as deleted or not
+ * @return this builder
+ */
+ Build deleted(boolean deleted);
+ }
+
+ /**
+ * @since 1.9
+ */
+ public interface RowStep extends Build {
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final Text row);
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final byte[] row);
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @param offset
+ * the offset within the array of the first byte to be read; must be non-negative and no larger than row.length
+ * @param length
+ * the number of bytes to be read from the given array; must be non-negative and no larger than row.length - offset
+ * @return this builder
+ */
+ ColumnFamilyStep row(final byte[] row, int offset, int length);
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key. The encoding must be UTF-8
+ * @return this builder
+ */
+ ColumnFamilyStep row(final CharSequence row);
+ }
+
+ /**
+ * @since 1.9
+ */
+ public interface ColumnFamilyStep extends ColumnVisibilityStep {
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnQualifierStep family(final byte[] columnFamily);
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}
+ * @param offset
+ * the offset within the array of the first byte to be read; must be non-negative and no larger than row.length
+ * @param length
+ * the number of bytes to be read from the given array; must be non-negative and no larger than row.length - offset
+ * @return this builder
+ */
+ ColumnQualifierStep family(final byte[] columnFamily, int offset, int length);
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnQualifierStep family(final Text columnFamily);
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}. The encoding must be UTF-8
+ * @return this builder
+ */
+ ColumnQualifierStep family(final CharSequence columnFamily);
+ }
+
+ /**
+ * @since 1.9
+ */
+ public interface ColumnQualifierStep extends ColumnVisibilityStep {
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final byte[] columnQualifier);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}
+ * @param offset
+ * the offset within the array of the first byte to be read; must be non-negative and no larger than row.length
+ * @param length
+ * the number of bytes to be read from the given array; must be non-negative and no larger than row.length - offset
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final byte[] columnQualifier, int offset, int length);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final Text columnQualifier);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}. The encoding must be UTF-8
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final CharSequence columnQualifier);
+ }
+
+ /**
+ * @since 1.9
+ */
+ public interface ColumnVisibilityStep extends Build {
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the {@link Key}
+ * @return this builder
+ */
+ Build visibility(final byte[] columnVisibility);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the {@link Key}
+ * @param offset
+ * the offset within the array of the first byte to be read; must be non-negative and no larger than row.length
+ * @param length
+ * the number of bytes to be read from the given array; must be non-negative and no larger than row.length - offset
+ * @return this builder
+ */
+ Build visibility(final byte[] columnVisibility, int offset, int length);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the {@link Key}
+ * @return this builder
+ */
+ Build visibility(final Text columnVisibility);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the {@link Key}. The encoding must be UTF-8
+ * @return this builder
+ */
+ Build visibility(final CharSequence columnVisibility);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the {@link Key}
+ * @return this builder
+ */
+ Build visibility(final ColumnVisibility columnVisibility);
+ }
+
+ /**
+ * @since 1.9
+ */
+ static class KeyBuilderImpl implements RowStep, ColumnFamilyStep, ColumnQualifierStep,
+ ColumnVisibilityStep {
+
+ protected static final byte EMPTY_BYTES[] = new byte[0];
+
+ private final boolean copyBytes;
+ private byte[] row = EMPTY_BYTES;
+ private int rowOffset = 0;
+ private int rowLength = 0;
+ private byte[] family = EMPTY_BYTES;
+ private int familyOffset = 0;
+ private int familyLength = 0;
+ private byte[] qualifier = EMPTY_BYTES;
+ private int qualifierOffset = 0;
+ private int qualifierLength = 0;
+ private byte[] visibility = EMPTY_BYTES;
+ private int visibilityOffset = 0;
+ private int visibilityLength = 0;
+ private long timestamp = Long.MAX_VALUE;
+ private boolean deleted = false;
+
+ KeyBuilderImpl(boolean copyBytes) {
+ this.copyBytes = copyBytes;
+ }
+
+ private byte[] copyBytesIfNeeded(final byte[] bytes, int offset, int length) {
+ return Key.copyIfNeeded(bytes, offset, length, this.copyBytes);
+ }
+
+ @Override
+ public ColumnFamilyStep row(final byte[] row, int offset, int length) {
+ this.row = copyBytesIfNeeded(row, offset, length);
+ this.rowOffset = this.copyBytes ? 0 : offset;
+ this.rowLength = this.copyBytes ? this.row.length : length;
+ return this;
+ }
+
+ @Override
+ public ColumnFamilyStep row(final byte[] row) {
+ return row(row, 0, row.length);
+ }
+
+ @Override
+ public ColumnFamilyStep row(final Text row) {
+ return row(row.getBytes(), 0, row.getLength());
+ }
+
+ @Override
+ public ColumnFamilyStep row(final CharSequence row) {
+ return row(new Text(row.toString()));
--- End diff --
Would be nice to circumvent the creation of the Text and go straight from `String` to `byte[]`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77287423
--- Diff: core/src/test/java/org/apache/accumulo/core/data/KeyBuilderTest.java ---
@@ -0,0 +1,282 @@
+/*
+ * 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.accumulo.core.data;
+
+import org.apache.hadoop.io.Text;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class KeyBuilderTest {
--- End diff --
> It would be nice to test that all builder methods fail fast when passed null.
This is probably better handled with static analysis tools and the use of `@Nonnull` annotations than unit test,, which is a out of scope for this change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on the issue:
https://github.com/apache/accumulo/pull/145
> Accumulo but not for the new developer and being used to see columnFamily or colFam everywhere would make this choice particularly difficult imho for no reasons.
@melrief I was reading back over your comments. Consistency with the existing methods names in key is a very good reason to use 'columnFamily()` intsead of 'family()`. Sigh, now I am torn also.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77424333
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,300 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build {@link Key}s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the {@link Key} are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build {
+
+ /**
+ * Build a {@link Key} from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the {@link Key} built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the {@link Key} created.
+ *
+ * @param timestamp
+ * the timestamp to use for the {@link Key}
+ * @return this builder
+ */
+ Build timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the {@link Key} to the parameter.
+ *
+ * @param deleted
+ * if the {@link Key} should be marked as deleted or not
+ * @return this builder
+ */
+ Build deleted(boolean deleted);
+ }
+
+ public interface RowStep extends Build {
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final Text row);
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final byte[] row);
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final CharSequence row);
+ }
+
+ public interface ColumnFamilyStep extends ColumnVisibilityStep {
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnQualifierStep family(final byte[] columnFamily);
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnQualifierStep family(final Text columnFamily);
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnQualifierStep family(final CharSequence columnFamily);
+ }
+
+ public interface ColumnQualifierStep extends ColumnVisibilityStep {
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final byte[] columnQualifier);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final Text columnQualifier);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final CharSequence columnQualifier);
+ }
+
+ public interface ColumnVisibilityStep extends Build {
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the {@link Key}
+ * @return this builder
+ */
+ Build visibility(final byte[] columnVisibility);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the {@link Key}
+ * @return this builder
+ */
+ Build visibility(final Text columnVisibility);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the {@link Key}
+ * @return this builder
+ */
+ Build visibility(final CharSequence columnVisibility);
+ }
+
+ static class AbstractKeyBuilder implements RowStep, ColumnFamilyStep, ColumnQualifierStep,
--- End diff --
Oh nice catch, I set it to `KeyBuilderImpl`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77238861
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,278 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build <code>Key</code>s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the <code>Key</code> are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build<T> {
+
+ /**
+ * Build a <code>Key</code> from this builder.
+ *
+ * @param copyBytes
+ * if the byte arrays should be copied or not. If not, byte arrays will be reused in the resultant <code>Key</code>
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build(boolean copyBytes);
+
+ /**
+ * Build a <code>Key</code> from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the <code>Key</code> created.
+ *
+ * @param timestamp
+ * the timestamp to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the <code>Key</code> to the parameter.
+ *
+ * @param deleted
+ * if the <code>Key</code> should be marked as deleted or not
+ * @return this builder
+ */
+ Build<T> deleted(boolean deleted);
+ }
+
+ public interface ColumnFamilyStep<T> extends ColumnVisibilityStep<T> {
+
+ /**
+ * Set the column family of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the <code>Key</code>
+ * @return this builder
+ */
+ ColumnQualifierStep<T> columnFamily(final T columnFamily);
+
+ /**
+ * Set the column family, qualifier and visibility of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the <code>Key</code>
+ * @param columnQualifier
+ * the column qualifier to use for the <code>Key</code>
+ * @param columnVisibility
+ * the column visibility to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> column(final T columnFamily, final T columnQualifier, final T columnVisibility);
+
+ /**
+ * Set the column family and the qualifier of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the <code>Key</code>
+ * @param columnQualifier
+ * the column qualifier to use for the <code>Key</code>
+ * @return this builder
+ */
+ ColumnVisibilityStep<T> column(final T columnFamily, final T columnQualifier);
+ }
+
+ public interface ColumnQualifierStep<T> extends ColumnVisibilityStep<T> {
+
+ /**
+ * Set the column qualifier of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the <code>Key</code>
+ * @return this builder
+ */
+ ColumnVisibilityStep<T> columnQualifier(final T columnQualifier);
+ }
+
+ public interface ColumnVisibilityStep<T> extends Build<T> {
+ /**
+ * Set the column qualifier of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> columnVisibility(final T columnVisibility);
+ }
+
+ private static abstract class AbstractKeyBuilder<T> implements ColumnFamilyStep<T>, ColumnQualifierStep<T>,
+ ColumnVisibilityStep<T> {
+
+ protected static final byte EMPTY_BYTES[] = new byte[0];
+
+ protected T row = null;
+ protected T columnFamily = null;
+ protected T columnQualifier = null;
+ protected T columnVisibility = null;
+ protected long timestamp = Long.MAX_VALUE;
+ protected boolean deleted = false;
+
+ final public ColumnFamilyStep<T> row(final T row) {
+ this.row = row;
+ return this;
+ }
+
+ @Override
+ final public ColumnQualifierStep<T> columnFamily(final T columnFamily) {
+ this.columnFamily = columnFamily;
+ return this;
+ }
+
+ @Override
+ final public ColumnVisibilityStep<T> columnQualifier(final T columnQualifier) {
+ this.columnQualifier = columnQualifier;
+ return this;
+ }
+
+ @Override
+ final public Build<T> columnVisibility(final T columnVisibility) {
+ this.columnVisibility = columnVisibility;
+ return this;
+ }
+
+ @Override
+ final public Build<T> timestamp(long timestamp) {
+ this.timestamp = timestamp;
+ return this;
+ }
+
+ @Override
+ public Build<T> deleted(boolean deleted) {
+ this.deleted = deleted;
+ return this;
+ }
+
+ @Override
+ public Key build() {
+ return this.build(true);
+ }
+
+ @Override
+ public Build<T> column(final T columnFamily, final T columnQualifier, final T columnVisibility) {
+ return this.columnFamily(columnFamily).columnQualifier(columnQualifier).columnVisibility(columnVisibility);
+ }
+
+ @Override
+ public ColumnVisibilityStep<T> column(final T columnFamily, final T columnQualifier) {
+ return this.columnFamily(columnFamily).columnQualifier(columnQualifier);
+ }
+ }
+
+ private static class TextKeyBuilder extends AbstractKeyBuilder<Text> {
+
+ private final Text EMPTY_TEXT = new Text();
+
+ @Override
+ public Key build(boolean copyBytes) {
+ Text columnFamily = (this.columnFamily == null) ? EMPTY_TEXT : this.columnFamily;
+ Text columnQualifier = (this.columnQualifier == null) ? EMPTY_TEXT : this.columnQualifier;
+ Text columnVisibility = (this.columnVisibility == null) ? EMPTY_TEXT : this.columnVisibility;
+ return new Key(row.getBytes(), 0, row.getLength(), columnFamily.getBytes(), 0, columnFamily.getLength(),
+ columnQualifier.getBytes(), 0, columnQualifier.getLength(), columnVisibility.getBytes(), 0,
+ columnVisibility.getLength(), timestamp, deleted, copyBytes);
+ }
+ }
+
+ private static class ByteArrayKeyBuilder extends AbstractKeyBuilder<byte[]> {
--- End diff --
You mean a single `ByteBuffer` as input and the offsets of the components or multiple `ByteBuffer`s, one for each component? In the second case it's easy, I can extract the underlying array from the `ByteBuffer` while in the first case I would need special methods that get the offset and length to be used for each components and I don't think it's really nice. Can you give me an example of how would you use a `ByteBuffer` as input to build a `Key`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77279651
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,300 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build {@link Key}s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the {@link Key} are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build {
+
+ /**
+ * Build a {@link Key} from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the {@link Key} built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the {@link Key} created.
+ *
+ * @param timestamp
+ * the timestamp to use for the {@link Key}
+ * @return this builder
+ */
+ Build timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the {@link Key} to the parameter.
+ *
+ * @param deleted
+ * if the {@link Key} should be marked as deleted or not
+ * @return this builder
+ */
+ Build deleted(boolean deleted);
+ }
+
+ public interface RowStep extends Build {
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final Text row);
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final byte[] row);
--- End diff --
Could have an additional method for byte[] type that takes an offset and length to copy.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77243116
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,278 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build <code>Key</code>s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the <code>Key</code> are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build<T> {
+
+ /**
+ * Build a <code>Key</code> from this builder.
+ *
+ * @param copyBytes
+ * if the byte arrays should be copied or not. If not, byte arrays will be reused in the resultant <code>Key</code>
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build(boolean copyBytes);
--- End diff --
I will add test cases after I refactor this such that each component can be of a different type.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on the issue:
https://github.com/apache/accumulo/pull/145
Good, done and also change since to 2.0.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on the issue:
https://github.com/apache/accumulo/pull/145
It makes sense to mimic what common libraries do and I understand that `Key.newBuilder()` would be easier to find for the user because it's directly in the `Key` class. Both point make sense and the `null` thing is internal and easy to workaround anyway. You convinced me, I'll use this pattern. I'll need sometime to complete the refactoring.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77424201
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,300 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build {@link Key}s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the {@link Key} are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build {
+
+ /**
+ * Build a {@link Key} from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the {@link Key} built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the {@link Key} created.
+ *
+ * @param timestamp
+ * the timestamp to use for the {@link Key}
+ * @return this builder
+ */
+ Build timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the {@link Key} to the parameter.
+ *
+ * @param deleted
+ * if the {@link Key} should be marked as deleted or not
+ * @return this builder
+ */
+ Build deleted(boolean deleted);
+ }
+
+ public interface RowStep extends Build {
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final Text row);
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final byte[] row);
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final CharSequence row);
+ }
+
+ public interface ColumnFamilyStep extends ColumnVisibilityStep {
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnQualifierStep family(final byte[] columnFamily);
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnQualifierStep family(final Text columnFamily);
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnQualifierStep family(final CharSequence columnFamily);
+ }
+
+ public interface ColumnQualifierStep extends ColumnVisibilityStep {
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final byte[] columnQualifier);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final Text columnQualifier);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final CharSequence columnQualifier);
+ }
+
+ public interface ColumnVisibilityStep extends Build {
--- End diff --
Done
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77646893
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,420 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.accumulo.core.security.ColumnVisibility;
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build {@link Key}s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the {@link Key} are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ * <code>CharSequence</code>s must be UTF-8 encoded.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.9
+ */
+public class KeyBuilder {
+
+ /**
+ * @since 1.9
+ */
+ public interface Build {
+
+ /**
+ * Build a {@link Key} from this builder. copyBytes defaults to true.
--- End diff --
"copyBytes defaults to true" isn't relevant here anymore.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by madrob <gi...@git.apache.org>.
Github user madrob commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77242419
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,278 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build <code>Key</code>s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the <code>Key</code> are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build<T> {
+
+ /**
+ * Build a <code>Key</code> from this builder.
+ *
+ * @param copyBytes
+ * if the byte arrays should be copied or not. If not, byte arrays will be reused in the resultant <code>Key</code>
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build(boolean copyBytes);
+
+ /**
+ * Build a <code>Key</code> from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the <code>Key</code> created.
+ *
+ * @param timestamp
+ * the timestamp to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the <code>Key</code> to the parameter.
+ *
+ * @param deleted
+ * if the <code>Key</code> should be marked as deleted or not
+ * @return this builder
+ */
+ Build<T> deleted(boolean deleted);
+ }
+
+ public interface ColumnFamilyStep<T> extends ColumnVisibilityStep<T> {
+
+ /**
+ * Set the column family of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the <code>Key</code>
+ * @return this builder
+ */
+ ColumnQualifierStep<T> columnFamily(final T columnFamily);
+
+ /**
+ * Set the column family, qualifier and visibility of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the <code>Key</code>
+ * @param columnQualifier
+ * the column qualifier to use for the <code>Key</code>
+ * @param columnVisibility
+ * the column visibility to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> column(final T columnFamily, final T columnQualifier, final T columnVisibility);
+
+ /**
+ * Set the column family and the qualifier of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the <code>Key</code>
+ * @param columnQualifier
+ * the column qualifier to use for the <code>Key</code>
+ * @return this builder
+ */
+ ColumnVisibilityStep<T> column(final T columnFamily, final T columnQualifier);
+ }
+
+ public interface ColumnQualifierStep<T> extends ColumnVisibilityStep<T> {
+
+ /**
+ * Set the column qualifier of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the <code>Key</code>
+ * @return this builder
+ */
+ ColumnVisibilityStep<T> columnQualifier(final T columnQualifier);
+ }
+
+ public interface ColumnVisibilityStep<T> extends Build<T> {
+ /**
+ * Set the column qualifier of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> columnVisibility(final T columnVisibility);
+ }
+
+ private static abstract class AbstractKeyBuilder<T> implements ColumnFamilyStep<T>, ColumnQualifierStep<T>,
+ ColumnVisibilityStep<T> {
+
+ protected static final byte EMPTY_BYTES[] = new byte[0];
+
+ protected T row = null;
+ protected T columnFamily = null;
+ protected T columnQualifier = null;
+ protected T columnVisibility = null;
+ protected long timestamp = Long.MAX_VALUE;
+ protected boolean deleted = false;
+
+ final public ColumnFamilyStep<T> row(final T row) {
+ this.row = row;
+ return this;
+ }
+
+ @Override
+ final public ColumnQualifierStep<T> columnFamily(final T columnFamily) {
+ this.columnFamily = columnFamily;
+ return this;
+ }
+
+ @Override
+ final public ColumnVisibilityStep<T> columnQualifier(final T columnQualifier) {
+ this.columnQualifier = columnQualifier;
+ return this;
+ }
+
+ @Override
+ final public Build<T> columnVisibility(final T columnVisibility) {
+ this.columnVisibility = columnVisibility;
+ return this;
+ }
+
+ @Override
+ final public Build<T> timestamp(long timestamp) {
+ this.timestamp = timestamp;
+ return this;
+ }
+
+ @Override
+ public Build<T> deleted(boolean deleted) {
+ this.deleted = deleted;
+ return this;
+ }
+
+ @Override
+ public Key build() {
+ return this.build(true);
+ }
+
+ @Override
+ public Build<T> column(final T columnFamily, final T columnQualifier, final T columnVisibility) {
+ return this.columnFamily(columnFamily).columnQualifier(columnQualifier).columnVisibility(columnVisibility);
+ }
+
+ @Override
+ public ColumnVisibilityStep<T> column(final T columnFamily, final T columnQualifier) {
+ return this.columnFamily(columnFamily).columnQualifier(columnQualifier);
+ }
+ }
+
+ private static class TextKeyBuilder extends AbstractKeyBuilder<Text> {
+
+ private final Text EMPTY_TEXT = new Text();
--- End diff --
Declare this `private static final` in `KeyBuilder`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77281375
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,300 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build {@link Key}s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the {@link Key} are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build {
+
+ /**
+ * Build a {@link Key} from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the {@link Key} built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the {@link Key} created.
+ *
+ * @param timestamp
+ * the timestamp to use for the {@link Key}
+ * @return this builder
+ */
+ Build timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the {@link Key} to the parameter.
+ *
+ * @param deleted
+ * if the {@link Key} should be marked as deleted or not
+ * @return this builder
+ */
+ Build deleted(boolean deleted);
+ }
+
+ public interface RowStep extends Build {
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final Text row);
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final byte[] row);
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final CharSequence row);
+ }
+
+ public interface ColumnFamilyStep extends ColumnVisibilityStep {
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnQualifierStep family(final byte[] columnFamily);
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnQualifierStep family(final Text columnFamily);
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnQualifierStep family(final CharSequence columnFamily);
+ }
+
+ public interface ColumnQualifierStep extends ColumnVisibilityStep {
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final byte[] columnQualifier);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final Text columnQualifier);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final CharSequence columnQualifier);
+ }
+
+ public interface ColumnVisibilityStep extends Build {
--- End diff --
Could add a visibility method that accepts a ColumnVisibility object.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77915565
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,420 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.accumulo.core.security.ColumnVisibility;
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build {@link Key}s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the {@link Key} are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ * <code>CharSequence</code>s must be UTF-8 encoded.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.9
+ */
+public class KeyBuilder {
+
+ /**
+ * @since 1.9
+ */
+ public interface Build {
+
+ /**
+ * Build a {@link Key} from this builder. copyBytes defaults to true.
--- End diff --
Removed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77280348
--- Diff: core/src/main/java/org/apache/accumulo/core/data/Key.java ---
@@ -51,6 +51,26 @@
protected long timestamp;
protected boolean deleted;
+ /**
+ * Create a {@link Key} builder.
+ *
+ * @param copyBytes
+ * if the bytes of the {@link Key} components should be copied
+ * @return the builder at the {@link KeyBuilder.RowStep}
+ */
+ public static KeyBuilder.RowStep builder(boolean copyBytes) {
--- End diff --
I like how you moved this from the end to the beginning.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77242251
--- Diff: core/src/main/java/org/apache/accumulo/core/data/Key.java ---
@@ -185,6 +185,44 @@ public Key(byte row[], int rOff, int rLen, byte cf[], int cfOff, int cfLen, byte
}
/**
+ * Creates a key.
+ *
+ * @param row
+ * bytes containing row ID
+ * @param rOff
+ * offset into row where key's row ID begins (inclusive)
+ * @param rLen
+ * length of row ID in row
+ * @param cf
+ * bytes containing column family
+ * @param cfOff
+ * offset into cf where key's column family begins (inclusive)
+ * @param cfLen
+ * length of column family in cf
+ * @param cq
+ * bytes containing column qualifier
+ * @param cqOff
+ * offset into cq where key's column qualifier begins (inclusive)
+ * @param cqLen
+ * length of column qualifier in cq
+ * @param cv
+ * bytes containing column visibility
+ * @param cvOff
+ * offset into cv where key's column visibility begins (inclusive)
+ * @param cvLen
+ * length of column visibility in cv
+ * @param ts
+ * timestamp
+ * @param deleted
+ * delete marker
+ * @param copy
+ * if true, forces copy of byte array values into key
+ */
+ public Key(byte row[], int rOff, int rLen, byte cf[], int cfOff, int cfLen, byte cq[], int cqOff, int cqLen, byte cv[], int cvOff, int cvLen, long ts, boolean deleted, boolean copy) {
--- End diff --
What makes this constructor different from the others and not worthy to be exposed to the client? It's a low level constructor that sets all the possible components of `Key` in the proper way. There is another public constructor that is basically this but with `deleted` defaulting to `false` and `copy` to `true` and I don't see why changing those two flags could have any undesired effects.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on the issue:
https://github.com/apache/accumulo/pull/145
I also changed this so that it will be merged in `master` and not in `1.8`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77424108
--- Diff: core/src/test/java/org/apache/accumulo/core/data/KeyBuilderTest.java ---
@@ -0,0 +1,239 @@
+package org.apache.accumulo.core.data;
+
+import org.apache.hadoop.io.Text;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class KeyBuilderTest {
+
+ private static final byte EMPTY_BYTES[] = new byte[0];
+ byte[] rowBytes = "row".getBytes();
--- End diff --
Done
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by madrob <gi...@git.apache.org>.
Github user madrob commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77262800
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,278 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build <code>Key</code>s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the <code>Key</code> are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build<T> {
+
+ /**
+ * Build a <code>Key</code> from this builder.
+ *
+ * @param copyBytes
+ * if the byte arrays should be copied or not. If not, byte arrays will be reused in the resultant <code>Key</code>
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build(boolean copyBytes);
+
+ /**
+ * Build a <code>Key</code> from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the <code>Key</code> created.
+ *
+ * @param timestamp
+ * the timestamp to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the <code>Key</code> to the parameter.
+ *
+ * @param deleted
+ * if the <code>Key</code> should be marked as deleted or not
+ * @return this builder
+ */
+ Build<T> deleted(boolean deleted);
+ }
+
+ public interface ColumnFamilyStep<T> extends ColumnVisibilityStep<T> {
+
+ /**
+ * Set the column family of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the <code>Key</code>
+ * @return this builder
+ */
+ ColumnQualifierStep<T> columnFamily(final T columnFamily);
+
+ /**
+ * Set the column family, qualifier and visibility of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the <code>Key</code>
+ * @param columnQualifier
+ * the column qualifier to use for the <code>Key</code>
+ * @param columnVisibility
+ * the column visibility to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> column(final T columnFamily, final T columnQualifier, final T columnVisibility);
+
+ /**
+ * Set the column family and the qualifier of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the <code>Key</code>
+ * @param columnQualifier
+ * the column qualifier to use for the <code>Key</code>
+ * @return this builder
+ */
+ ColumnVisibilityStep<T> column(final T columnFamily, final T columnQualifier);
+ }
+
+ public interface ColumnQualifierStep<T> extends ColumnVisibilityStep<T> {
+
+ /**
+ * Set the column qualifier of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the <code>Key</code>
+ * @return this builder
+ */
+ ColumnVisibilityStep<T> columnQualifier(final T columnQualifier);
+ }
+
+ public interface ColumnVisibilityStep<T> extends Build<T> {
+ /**
+ * Set the column qualifier of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> columnVisibility(final T columnVisibility);
+ }
+
+ private static abstract class AbstractKeyBuilder<T> implements ColumnFamilyStep<T>, ColumnQualifierStep<T>,
+ ColumnVisibilityStep<T> {
+
+ protected static final byte EMPTY_BYTES[] = new byte[0];
+
+ protected T row = null;
+ protected T columnFamily = null;
+ protected T columnQualifier = null;
+ protected T columnVisibility = null;
+ protected long timestamp = Long.MAX_VALUE;
+ protected boolean deleted = false;
+
+ final public ColumnFamilyStep<T> row(final T row) {
+ this.row = row;
+ return this;
+ }
+
+ @Override
+ final public ColumnQualifierStep<T> columnFamily(final T columnFamily) {
+ this.columnFamily = columnFamily;
+ return this;
+ }
+
+ @Override
+ final public ColumnVisibilityStep<T> columnQualifier(final T columnQualifier) {
+ this.columnQualifier = columnQualifier;
+ return this;
+ }
+
+ @Override
+ final public Build<T> columnVisibility(final T columnVisibility) {
+ this.columnVisibility = columnVisibility;
+ return this;
+ }
+
+ @Override
+ final public Build<T> timestamp(long timestamp) {
+ this.timestamp = timestamp;
+ return this;
+ }
+
+ @Override
+ public Build<T> deleted(boolean deleted) {
+ this.deleted = deleted;
+ return this;
+ }
+
+ @Override
+ public Key build() {
+ return this.build(true);
+ }
+
+ @Override
+ public Build<T> column(final T columnFamily, final T columnQualifier, final T columnVisibility) {
+ return this.columnFamily(columnFamily).columnQualifier(columnQualifier).columnVisibility(columnVisibility);
+ }
+
+ @Override
+ public ColumnVisibilityStep<T> column(final T columnFamily, final T columnQualifier) {
+ return this.columnFamily(columnFamily).columnQualifier(columnQualifier);
+ }
+ }
+
+ private static class TextKeyBuilder extends AbstractKeyBuilder<Text> {
+
+ private final Text EMPTY_TEXT = new Text();
+
+ @Override
+ public Key build(boolean copyBytes) {
+ Text columnFamily = (this.columnFamily == null) ? EMPTY_TEXT : this.columnFamily;
+ Text columnQualifier = (this.columnQualifier == null) ? EMPTY_TEXT : this.columnQualifier;
+ Text columnVisibility = (this.columnVisibility == null) ? EMPTY_TEXT : this.columnVisibility;
+ return new Key(row.getBytes(), 0, row.getLength(), columnFamily.getBytes(), 0, columnFamily.getLength(),
+ columnQualifier.getBytes(), 0, columnQualifier.getLength(), columnVisibility.getBytes(), 0,
+ columnVisibility.getLength(), timestamp, deleted, copyBytes);
+ }
+ }
+
+ private static class ByteArrayKeyBuilder extends AbstractKeyBuilder<byte[]> {
+
+ @Override
+ public Key build(boolean copyBytes) {
+ byte[] columnFamily = (this.columnFamily == null) ? EMPTY_BYTES : this.columnFamily;
+ byte[] columnQualifier = (this.columnQualifier == null) ? EMPTY_BYTES : this.columnQualifier;
+ byte[] columnVisibility = (this.columnVisibility == null) ? EMPTY_BYTES : this.columnVisibility;
+ return new Key(row, columnFamily, columnQualifier, columnVisibility, timestamp, deleted, copyBytes);
+ }
+ }
+
+ private static class CharSequenceKeyBuilder extends AbstractKeyBuilder<CharSequence> {
+
+ private final Text EMPTY_TEXT = new Text();
+
+ @Override
+ public Key build(boolean copyBytes) {
--- End diff --
I'm fine with this as long as you document the behavior clearly
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on the issue:
https://github.com/apache/accumulo/pull/145
I'll have to wait tomorrow but if you want to do it you are welcome :).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on the issue:
https://github.com/apache/accumulo/pull/145
> I have to disagree in removing the column from the column components. It may be easier for who knows Accumulo but not for the new developer and being used to see columnFamily or colFam everywhere would make this choice particularly difficult imho for no reasons.
@melrief those are valid points. There are definitely two classes of users to consider, new users and experienced users. I am thinking about making the code pleasant to read and write for more experienced users and you want to make the code easier to read/write for new users. They are both valid concerns. Do you think having shorter names and good javadocs on the methods would be good enough for both users?
> I don't understand why the newBuilder() method. What problem does it solve?
Not much. It offers the following advantages :
* One less class in public API. The interfaces need to be in public API, but with this static method the class implementing the interfaces can be package private.
* When a developer is looking at the Key class methods in an IDE, it makes it easy to discover that a builder exists. This could also be accomplished with good javadoc for Key that points to the Builder.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by madrob <gi...@git.apache.org>.
Github user madrob commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77241306
--- Diff: core/src/main/java/org/apache/accumulo/core/data/Key.java ---
@@ -185,6 +185,44 @@ public Key(byte row[], int rOff, int rLen, byte cf[], int cfOff, int cfLen, byte
}
/**
+ * Creates a key.
+ *
+ * @param row
+ * bytes containing row ID
+ * @param rOff
+ * offset into row where key's row ID begins (inclusive)
+ * @param rLen
+ * length of row ID in row
+ * @param cf
+ * bytes containing column family
+ * @param cfOff
+ * offset into cf where key's column family begins (inclusive)
+ * @param cfLen
+ * length of column family in cf
+ * @param cq
+ * bytes containing column qualifier
+ * @param cqOff
+ * offset into cq where key's column qualifier begins (inclusive)
+ * @param cqLen
+ * length of column qualifier in cq
+ * @param cv
+ * bytes containing column visibility
+ * @param cvOff
+ * offset into cv where key's column visibility begins (inclusive)
+ * @param cvLen
+ * length of column visibility in cv
+ * @param ts
+ * timestamp
+ * @param deleted
+ * delete marker
+ * @param copy
+ * if true, forces copy of byte array values into key
+ */
+ public Key(byte row[], int rOff, int rLen, byte cf[], int cfOff, int cfLen, byte cq[], int cqOff, int cqLen, byte cv[], int cvOff, int cvLen, long ts, boolean deleted, boolean copy) {
--- End diff --
This should be package-private.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77281256
--- Diff: core/src/test/java/org/apache/accumulo/core/data/KeyBuilderTest.java ---
@@ -0,0 +1,282 @@
+/*
+ * 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.accumulo.core.data;
+
+import org.apache.hadoop.io.Text;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class KeyBuilderTest {
+
+ private static final byte EMPTY_BYTES[] = new byte[0];
+ byte[] rowBytes = "row".getBytes();
+ byte[] familyBytes = "family".getBytes();
+ byte[] qualifierBytes = "qualifier".getBytes();
+ byte[] visibilityBytes = "visibility".getBytes();
+ Text rowText = new Text(rowBytes);
+ Text familyText = new Text(familyBytes);
+ Text qualifierText = new Text(qualifierBytes);
+ Text visibilityText = new Text(visibilityBytes);
+
+ @Test
+ public void testKeyBuildingFromRow() {
+ Key keyBuilt = Key.builder().row("foo").build();
+ Key keyExpected = new Key("foo");
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamily() {
+ Key keyBuilt = Key.builder().row("foo").family("bar").build();
+ Key keyExpected = new Key("foo", "bar");
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifier() {
+ Key keyBuilt = Key.builder().row("foo").family("bar").qualifier("baz").build();
+ Key keyExpected = new Key("foo", "bar", "baz");
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibility() {
+ Key keyBuilt = Key.builder().row("foo").family("bar").qualifier("baz").visibility("v").build();
+ Key keyExpected = new Key("foo", "bar", "baz", "v");
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityTimestamp() {
+ Key keyBuilt = Key.builder().row("foo").family("bar").qualifier("baz").visibility("v").timestamp(1L).build();
+ Key keyExpected = new Key("foo", "bar", "baz", "v", 1L);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityTimestampDeleted() {
+ Key keyBuilt =
+ Key.builder()
+ .row("foo")
+ .family("bar")
+ .qualifier("baz")
+ .visibility("v")
+ .timestamp(10L)
+ .deleted(true)
+ .build();
+ Key keyExpected = new Key("foo", "bar", "baz", "v", 10L);
+ keyExpected.setDeleted(true);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowVisibility() {
+ Key keyBuilt = Key.builder().row("foo").visibility("v").build();
+ Key keyExpected = new Key("foo", "", "", "v");
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyVisibility() {
+ Key keyBuilt = Key.builder().row("foo").family("bar").visibility("v").build();
+ Key keyExpected = new Key("foo", "bar", "", "v");
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void textKeyBuildingFromRowTimestamp() {
+ Key keyBuilt = Key.builder().row("foo").timestamp(3L).build();
+ Key keyExpected = new Key("foo");
+ keyExpected.setTimestamp(3L);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).build();
+ Key keyExpected = new Key(rowBytes, EMPTY_BYTES, EMPTY_BYTES, EMPTY_BYTES, Long.MAX_VALUE);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).family(familyBytes).build();
+ Key keyExpected = new Key(rowBytes, familyBytes, EMPTY_BYTES, EMPTY_BYTES, Long.MAX_VALUE);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).family(familyBytes).qualifier(qualifierBytes).build();
+ Key keyExpected = new Key(rowBytes, familyBytes, qualifierBytes, EMPTY_BYTES, Long.MAX_VALUE);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).family(familyBytes).qualifier(qualifierBytes).visibility(visibilityBytes).build();
+ Key keyExpected = new Key(rowBytes, familyBytes, qualifierBytes, visibilityBytes, Long.MAX_VALUE);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityTimestampBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).family(familyBytes).qualifier(qualifierBytes).visibility(visibilityBytes).timestamp(1L).build();
+ Key keyExpected = new Key(rowBytes, familyBytes, qualifierBytes, visibilityBytes, 1L);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityTimestampDeletedBytes() {
+ Key keyBuilt =
+ Key.builder()
+ .row(rowBytes)
+ .family(familyBytes)
+ .qualifier(qualifierBytes)
+ .visibility(visibilityBytes)
+ .timestamp(10L)
+ .deleted(true)
+ .build();
+ Key keyExpected = new Key(rowBytes, familyBytes, qualifierBytes, visibilityBytes, 10L);
+ keyExpected.setDeleted(true);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowVisibilityBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).visibility(visibilityBytes).build();
+ Key keyExpected = new Key(rowBytes, EMPTY_BYTES, EMPTY_BYTES, visibilityBytes, Long.MAX_VALUE);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyVisibilityBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).family(familyBytes).visibility(visibilityBytes).build();
+ Key keyExpected = new Key(rowBytes, familyBytes, EMPTY_BYTES, visibilityBytes, Long.MAX_VALUE);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void textKeyBuildingFromRowTimestampBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).timestamp(3L).build();
+ Key keyExpected = new Key(rowBytes, EMPTY_BYTES, EMPTY_BYTES, EMPTY_BYTES, Long.MAX_VALUE);
+ keyExpected.setTimestamp(3L);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowText() {
+ Key keyBuilt = Key.builder().row(rowText).build();
+ Key keyExpected = new Key(rowText);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyText() {
+ Key keyBuilt = Key.builder().row(rowText).family(familyText).build();
+ Key keyExpected = new Key(rowText, familyText);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierText() {
+ Key keyBuilt = Key.builder().row(rowText).family(familyText).qualifier(qualifierText).build();
+ Key keyExpected = new Key(rowText, familyText, qualifierText);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityText() {
+ Key keyBuilt = Key.builder().row(rowText).family(familyText).qualifier(qualifierText).visibility(visibilityText).build();
+ Key keyExpected = new Key(rowText, familyText, qualifierText, visibilityText);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityTimestampText() {
+ Key keyBuilt = Key.builder().row(rowText).family(familyText).qualifier(qualifierText).visibility(visibilityText).timestamp(1L).build();
+ Key keyExpected = new Key(rowText, familyText, qualifierText, visibilityText, 1L);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityTimestampDeletedText() {
+ Key keyBuilt =
+ Key.builder()
+ .row(rowText)
+ .family(familyText)
+ .qualifier(qualifierText)
+ .visibility(visibilityText)
+ .timestamp(10L)
+ .deleted(true)
+ .build();
+ Key keyExpected = new Key(rowText, familyText, qualifierText, visibilityText, 10L);
+ keyExpected.setDeleted(true);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowVisibilityText() {
+ Key keyBuilt = Key.builder().row(rowText).visibility(visibilityText).build();
+ Key keyExpected = new Key(rowText, new Text(), new Text(), visibilityText);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyVisibilityText() {
+ Key keyBuilt = Key.builder().row(rowText).family(familyText).visibility(visibilityText).build();
+ Key keyExpected = new Key(rowText, familyText, new Text(), visibilityText);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowTimestampText() {
+ Key keyBuilt = Key.builder().row(rowText).timestamp(3L).build();
+ Key keyExpected = new Key(rowText);
+ keyExpected.setTimestamp(3L);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingReusingBytes() {
+ byte[] reuse = new byte[] { 1, 2, 3 };
+ KeyBuilder.Build keyBuilder = Key.builder(false).row(reuse);
+ Key keyBuilt = keyBuilder.build();
+ assertEquals(reuse, keyBuilt.getRowBytes());
--- End diff --
Could use `assertSame()` here. Its not needed for correctness in this case, just makes intentions clear.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77247911
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,278 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build <code>Key</code>s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the <code>Key</code> are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build<T> {
+
+ /**
+ * Build a <code>Key</code> from this builder.
+ *
+ * @param copyBytes
+ * if the byte arrays should be copied or not. If not, byte arrays will be reused in the resultant <code>Key</code>
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build(boolean copyBytes);
+
+ /**
+ * Build a <code>Key</code> from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the <code>Key</code> created.
+ *
+ * @param timestamp
+ * the timestamp to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the <code>Key</code> to the parameter.
+ *
+ * @param deleted
+ * if the <code>Key</code> should be marked as deleted or not
+ * @return this builder
+ */
+ Build<T> deleted(boolean deleted);
+ }
+
+ public interface ColumnFamilyStep<T> extends ColumnVisibilityStep<T> {
+
+ /**
+ * Set the column family of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the <code>Key</code>
+ * @return this builder
+ */
+ ColumnQualifierStep<T> columnFamily(final T columnFamily);
+
+ /**
+ * Set the column family, qualifier and visibility of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the <code>Key</code>
+ * @param columnQualifier
+ * the column qualifier to use for the <code>Key</code>
+ * @param columnVisibility
+ * the column visibility to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> column(final T columnFamily, final T columnQualifier, final T columnVisibility);
+
+ /**
+ * Set the column family and the qualifier of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the <code>Key</code>
+ * @param columnQualifier
+ * the column qualifier to use for the <code>Key</code>
+ * @return this builder
+ */
+ ColumnVisibilityStep<T> column(final T columnFamily, final T columnQualifier);
+ }
+
+ public interface ColumnQualifierStep<T> extends ColumnVisibilityStep<T> {
+
+ /**
+ * Set the column qualifier of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the <code>Key</code>
+ * @return this builder
+ */
+ ColumnVisibilityStep<T> columnQualifier(final T columnQualifier);
+ }
+
+ public interface ColumnVisibilityStep<T> extends Build<T> {
+ /**
+ * Set the column qualifier of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> columnVisibility(final T columnVisibility);
+ }
+
+ private static abstract class AbstractKeyBuilder<T> implements ColumnFamilyStep<T>, ColumnQualifierStep<T>,
+ ColumnVisibilityStep<T> {
+
+ protected static final byte EMPTY_BYTES[] = new byte[0];
+
+ protected T row = null;
+ protected T columnFamily = null;
+ protected T columnQualifier = null;
+ protected T columnVisibility = null;
+ protected long timestamp = Long.MAX_VALUE;
+ protected boolean deleted = false;
+
+ final public ColumnFamilyStep<T> row(final T row) {
+ this.row = row;
+ return this;
+ }
+
+ @Override
+ final public ColumnQualifierStep<T> columnFamily(final T columnFamily) {
+ this.columnFamily = columnFamily;
+ return this;
+ }
+
+ @Override
+ final public ColumnVisibilityStep<T> columnQualifier(final T columnQualifier) {
+ this.columnQualifier = columnQualifier;
+ return this;
+ }
+
+ @Override
+ final public Build<T> columnVisibility(final T columnVisibility) {
+ this.columnVisibility = columnVisibility;
+ return this;
+ }
+
+ @Override
+ final public Build<T> timestamp(long timestamp) {
+ this.timestamp = timestamp;
+ return this;
+ }
+
+ @Override
+ public Build<T> deleted(boolean deleted) {
+ this.deleted = deleted;
+ return this;
+ }
+
+ @Override
+ public Key build() {
+ return this.build(true);
+ }
+
+ @Override
+ public Build<T> column(final T columnFamily, final T columnQualifier, final T columnVisibility) {
+ return this.columnFamily(columnFamily).columnQualifier(columnQualifier).columnVisibility(columnVisibility);
+ }
+
+ @Override
+ public ColumnVisibilityStep<T> column(final T columnFamily, final T columnQualifier) {
+ return this.columnFamily(columnFamily).columnQualifier(columnQualifier);
+ }
+ }
+
+ private static class TextKeyBuilder extends AbstractKeyBuilder<Text> {
+
+ private final Text EMPTY_TEXT = new Text();
+
+ @Override
+ public Key build(boolean copyBytes) {
+ Text columnFamily = (this.columnFamily == null) ? EMPTY_TEXT : this.columnFamily;
+ Text columnQualifier = (this.columnQualifier == null) ? EMPTY_TEXT : this.columnQualifier;
+ Text columnVisibility = (this.columnVisibility == null) ? EMPTY_TEXT : this.columnVisibility;
+ return new Key(row.getBytes(), 0, row.getLength(), columnFamily.getBytes(), 0, columnFamily.getLength(),
+ columnQualifier.getBytes(), 0, columnQualifier.getLength(), columnVisibility.getBytes(), 0,
+ columnVisibility.getLength(), timestamp, deleted, copyBytes);
+ }
+ }
+
+ private static class ByteArrayKeyBuilder extends AbstractKeyBuilder<byte[]> {
+
+ @Override
+ public Key build(boolean copyBytes) {
+ byte[] columnFamily = (this.columnFamily == null) ? EMPTY_BYTES : this.columnFamily;
+ byte[] columnQualifier = (this.columnQualifier == null) ? EMPTY_BYTES : this.columnQualifier;
+ byte[] columnVisibility = (this.columnVisibility == null) ? EMPTY_BYTES : this.columnVisibility;
+ return new Key(row, columnFamily, columnQualifier, columnVisibility, timestamp, deleted, copyBytes);
+ }
+ }
+
+ private static class CharSequenceKeyBuilder extends AbstractKeyBuilder<CharSequence> {
+
+ private final Text EMPTY_TEXT = new Text();
+
+ @Override
+ public Key build(boolean copyBytes) {
--- End diff --
No it doesn't for now because I'm still not sure if I can access the underlying array of a `CharSequence`. The issue is that not all types will allow to access their underlying bytes and with a common interface there is no way to set this flag only for the types that support this.
One solution would be to have another interface for types that support the use of the underlying bytes so that those types have the possibility to set the `copyBytes` flag and the others not but this complicates the things a little bit and it wouldn't play well with heterogeneous components. For instance, if I have
```java
byte r[];
String f;
Text q;
Key k = Key.newBuilder().row(r).family(f).qualifier(q).build();
```
Does build takes the flag `copyBytes` or not considering that `f` won't use the underlying array anyway? Shall we add the flag to each component method so that we can decide only for components that make sense? I think I rather prefer to have the flag regardless of the type with the idea what if the flag is set to `false` but the type doesn't support direct access to the underlying array, then the flag is ignored and bytes are copied.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by dhutchis <gi...@git.apache.org>.
Github user dhutchis commented on the issue:
https://github.com/apache/accumulo/pull/145
@melrief Here is a user scenario for building a Key from ByteBuffers. The main advantage is being able to create keys from the middle of existing arrays given their offsets and lengths.
Sadly I realized after writing the example that the `Key.copyIfNeeded()` method will copy all of these anyway, so the efficiency gained by the code may just be an illusion. If it is an illusion, it would be simpler for a user to use `ByteBufferUtil.toArray(bbRow)` and so forth. Changing the Key class to use segments of arrays is a big change.
```java
// These byte[]s are obtained from user code.
byte[] bigArray = new byte[500], bigArray2 = new byte[80], EMPTY_BYTES = new byte[0];
// Suppose that we want to create a key whose row is in the bigArray from bytes 105 to 115,
// whose column family is in the bigArray2 from bytes 200 to 207,
// and whose column qualifier is in the bigArray2 from bytes 60 to 69.
ByteBuffer bbRow = ByteBuffer.wrap(bigArray, 105, 10);
ByteBuffer bbFam = ByteBuffer.wrap(bigArray, 200, 7);
ByteBuffer bbQual = ByteBuffer.wrap(bigArray2, 60, 9);
// We expect hasArray() to be true for all of these.
// When hasArray() is true, we can avoid copying out the portion of the ByteBuffer to a new byte[].
// I include the case for ByteBuffers that do not have a backing array for completeness.
final byte[] bRow, bFam, bQual;
final int offRow, offFam, offQual, lenRow, lenFam, lenQual;
if (bbRow.hasArray()) {
bRow = bbRow.array(); offRow = bbRow.arrayOffset() + bbRow.position(); lenRow = bbRow.remaining();
} else {
// fallback to copy case
bRow = ByteBufferUtil.toBytes(bbRow);
// The ByteBufferUtil.toBytes method will have the following effect:
//bRow = new byte[bbRow.remaining()];
//bbRow.duplicate().get(bRow);
offRow = 0; lenRow = bRow.length;
}
// Repeat for family and qualifier
if (bbFam.hasArray()) {
bFam = bbFam.array(); offFam = bbFam.arrayOffset() + bbFam.position(); lenFam = bbFam.remaining();
} else {
// fallback to copy case
bFam = ByteBufferUtil.toBytes(bbFam);
offFam = 0; lenFam = bFam.length;
}
if (bbQual.hasArray()) {
bQual = bbQual.array(); offQual = bbQual.arrayOffset() + bbQual.position(); lenQual = bbQual.remaining();
} else {
// fallback to copy case
bQual = ByteBufferUtil.toBytes(bbQual);
offQual = 0; lenQual = bQual.length;
}
// Construct key from all byte[]s with offsets and lengths.
// Note that this old API method copies the data. It would be nice to not copy the data,
// but this would bigger changes to the Key class.
Key k = new Key(bRow, offRow, lenRow, bFam, offFam, lenFam, bQual, offQual, lenQual,
EMPTY_BYTES, 0, 0, Long.MAX_VALUE);
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on the issue:
https://github.com/apache/accumulo/pull/145
To expand on my previous comment about consistency and why I am less certain. The reason I was torn after re-reading @melrief comment is that the issue of consistency is not about new vs existing users. Having 20 different ways to say the same thing in the API is aesthetically displeasing. So even though the new API may be aesthetically pleasing when considered alone, it may not be when considering it as part of the entire API. This may be ok if a new standard is set thats followed from the point forward. Existing precedents must be considered when adding new APIs.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77424280
--- Diff: core/src/test/java/org/apache/accumulo/core/data/KeyBuilderTest.java ---
@@ -0,0 +1,282 @@
+/*
+ * 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.accumulo.core.data;
+
+import org.apache.hadoop.io.Text;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class KeyBuilderTest {
--- End diff --
Java 8 has a `@NonNull` annotation which should do some tests but I'm not sure what I have to import to use it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by dhutchis <gi...@git.apache.org>.
Github user dhutchis commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77288983
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,300 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build {@link Key}s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the {@link Key} are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build {
+
+ /**
+ * Build a {@link Key} from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the {@link Key} built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the {@link Key} created.
+ *
+ * @param timestamp
+ * the timestamp to use for the {@link Key}
+ * @return this builder
+ */
+ Build timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the {@link Key} to the parameter.
+ *
+ * @param deleted
+ * if the {@link Key} should be marked as deleted or not
+ * @return this builder
+ */
+ Build deleted(boolean deleted);
+ }
+
+ public interface RowStep extends Build {
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final Text row);
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final byte[] row);
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final CharSequence row);
+ }
+
+ public interface ColumnFamilyStep extends ColumnVisibilityStep {
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnQualifierStep family(final byte[] columnFamily);
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnQualifierStep family(final Text columnFamily);
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnQualifierStep family(final CharSequence columnFamily);
+ }
+
+ public interface ColumnQualifierStep extends ColumnVisibilityStep {
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final byte[] columnQualifier);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final Text columnQualifier);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final CharSequence columnQualifier);
+ }
+
+ public interface ColumnVisibilityStep extends Build {
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the {@link Key}
+ * @return this builder
+ */
+ Build visibility(final byte[] columnVisibility);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the {@link Key}
+ * @return this builder
+ */
+ Build visibility(final Text columnVisibility);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the {@link Key}
+ * @return this builder
+ */
+ Build visibility(final CharSequence columnVisibility);
+ }
+
+ static class AbstractKeyBuilder implements RowStep, ColumnFamilyStep, ColumnQualifierStep,
--- End diff --
Since this class is no longer abstract, rename to `KeyBuilder`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/accumulo/pull/145
> I'll have to wait tomorrow but if you want to do it you are welcome :).
Superb. I'm running the tests now, and will hopefully commit tonight.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by madrob <gi...@git.apache.org>.
Github user madrob commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77262460
--- Diff: core/src/main/java/org/apache/accumulo/core/data/Key.java ---
@@ -185,6 +185,44 @@ public Key(byte row[], int rOff, int rLen, byte cf[], int cfOff, int cfLen, byte
}
/**
+ * Creates a key.
+ *
+ * @param row
+ * bytes containing row ID
+ * @param rOff
+ * offset into row where key's row ID begins (inclusive)
+ * @param rLen
+ * length of row ID in row
+ * @param cf
+ * bytes containing column family
+ * @param cfOff
+ * offset into cf where key's column family begins (inclusive)
+ * @param cfLen
+ * length of column family in cf
+ * @param cq
+ * bytes containing column qualifier
+ * @param cqOff
+ * offset into cq where key's column qualifier begins (inclusive)
+ * @param cqLen
+ * length of column qualifier in cq
+ * @param cv
+ * bytes containing column visibility
+ * @param cvOff
+ * offset into cv where key's column visibility begins (inclusive)
+ * @param cvLen
+ * length of column visibility in cv
+ * @param ts
+ * timestamp
+ * @param deleted
+ * delete marker
+ * @param copy
+ * if true, forces copy of byte array values into key
+ */
+ public Key(byte row[], int rOff, int rLen, byte cf[], int cfOff, int cfLen, byte cq[], int cqOff, int cqLen, byte cv[], int cvOff, int cvLen, long ts, boolean deleted, boolean copy) {
--- End diff --
Because it's new and something that we'd have to support forever. It has 15 arguments which is a user error waiting to happen. If I could go back in time and drop those other constructors in favor of a builder pattern, I probably would. The whole point of what we're doing here is to avoid exposing the additional combinatorial explosion of parameters.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by madrob <gi...@git.apache.org>.
Github user madrob commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77242371
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,278 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build <code>Key</code>s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the <code>Key</code> are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build<T> {
+
+ /**
+ * Build a <code>Key</code> from this builder.
+ *
+ * @param copyBytes
+ * if the byte arrays should be copied or not. If not, byte arrays will be reused in the resultant <code>Key</code>
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build(boolean copyBytes);
+
+ /**
+ * Build a <code>Key</code> from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the <code>Key</code> created.
+ *
+ * @param timestamp
+ * the timestamp to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the <code>Key</code> to the parameter.
+ *
+ * @param deleted
+ * if the <code>Key</code> should be marked as deleted or not
+ * @return this builder
+ */
+ Build<T> deleted(boolean deleted);
+ }
+
+ public interface ColumnFamilyStep<T> extends ColumnVisibilityStep<T> {
+
+ /**
+ * Set the column family of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the <code>Key</code>
+ * @return this builder
+ */
+ ColumnQualifierStep<T> columnFamily(final T columnFamily);
+
+ /**
+ * Set the column family, qualifier and visibility of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the <code>Key</code>
+ * @param columnQualifier
+ * the column qualifier to use for the <code>Key</code>
+ * @param columnVisibility
+ * the column visibility to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> column(final T columnFamily, final T columnQualifier, final T columnVisibility);
+
+ /**
+ * Set the column family and the qualifier of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the <code>Key</code>
+ * @param columnQualifier
+ * the column qualifier to use for the <code>Key</code>
+ * @return this builder
+ */
+ ColumnVisibilityStep<T> column(final T columnFamily, final T columnQualifier);
+ }
+
+ public interface ColumnQualifierStep<T> extends ColumnVisibilityStep<T> {
+
+ /**
+ * Set the column qualifier of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the <code>Key</code>
+ * @return this builder
+ */
+ ColumnVisibilityStep<T> columnQualifier(final T columnQualifier);
+ }
+
+ public interface ColumnVisibilityStep<T> extends Build<T> {
+ /**
+ * Set the column qualifier of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> columnVisibility(final T columnVisibility);
+ }
+
+ private static abstract class AbstractKeyBuilder<T> implements ColumnFamilyStep<T>, ColumnQualifierStep<T>,
+ ColumnVisibilityStep<T> {
+
+ protected static final byte EMPTY_BYTES[] = new byte[0];
--- End diff --
Declare this `private static final` in KeyBuilder.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on the issue:
https://github.com/apache/accumulo/pull/145
Thanks a bunch @melrief . This will be very nice to have.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77098307
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,278 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build <code>Key</code>s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the <code>Key</code> are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build<T> {
+
+ /**
+ * Build a <code>Key</code> from this builder.
+ *
+ * @param copyBytes
+ * if the byte arrays should be copied or not. If not, byte arrays will be reused in the resultant <code>Key</code>
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build(boolean copyBytes);
--- End diff --
I don't see any test cases covering this `build(boolean)` method.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on the issue:
https://github.com/apache/accumulo/pull/145
I did the rebase and started with the changes but I have to force push it and probably we are going to miss some comments after the push. Are you ok with force pushing the rebase with the changes right now?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77654478
--- Diff: core/src/test/java/org/apache/accumulo/core/data/KeyBuilderTest.java ---
@@ -0,0 +1,282 @@
+/*
+ * 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.accumulo.core.data;
+
+import org.apache.hadoop.io.Text;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class KeyBuilderTest {
+
+ private static final byte EMPTY_BYTES[] = new byte[0];
+ byte[] rowBytes = "row".getBytes();
+ byte[] familyBytes = "family".getBytes();
+ byte[] qualifierBytes = "qualifier".getBytes();
+ byte[] visibilityBytes = "visibility".getBytes();
+ Text rowText = new Text(rowBytes);
+ Text familyText = new Text(familyBytes);
+ Text qualifierText = new Text(qualifierBytes);
+ Text visibilityText = new Text(visibilityBytes);
+
+ @Test
+ public void testKeyBuildingFromRow() {
+ Key keyBuilt = Key.builder().row("foo").build();
+ Key keyExpected = new Key("foo");
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamily() {
+ Key keyBuilt = Key.builder().row("foo").family("bar").build();
+ Key keyExpected = new Key("foo", "bar");
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifier() {
+ Key keyBuilt = Key.builder().row("foo").family("bar").qualifier("baz").build();
+ Key keyExpected = new Key("foo", "bar", "baz");
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibility() {
+ Key keyBuilt = Key.builder().row("foo").family("bar").qualifier("baz").visibility("v").build();
+ Key keyExpected = new Key("foo", "bar", "baz", "v");
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityTimestamp() {
+ Key keyBuilt = Key.builder().row("foo").family("bar").qualifier("baz").visibility("v").timestamp(1L).build();
+ Key keyExpected = new Key("foo", "bar", "baz", "v", 1L);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityTimestampDeleted() {
+ Key keyBuilt =
+ Key.builder()
+ .row("foo")
+ .family("bar")
+ .qualifier("baz")
+ .visibility("v")
+ .timestamp(10L)
+ .deleted(true)
+ .build();
+ Key keyExpected = new Key("foo", "bar", "baz", "v", 10L);
+ keyExpected.setDeleted(true);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowVisibility() {
+ Key keyBuilt = Key.builder().row("foo").visibility("v").build();
+ Key keyExpected = new Key("foo", "", "", "v");
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyVisibility() {
+ Key keyBuilt = Key.builder().row("foo").family("bar").visibility("v").build();
+ Key keyExpected = new Key("foo", "bar", "", "v");
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void textKeyBuildingFromRowTimestamp() {
+ Key keyBuilt = Key.builder().row("foo").timestamp(3L).build();
+ Key keyExpected = new Key("foo");
+ keyExpected.setTimestamp(3L);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).build();
+ Key keyExpected = new Key(rowBytes, EMPTY_BYTES, EMPTY_BYTES, EMPTY_BYTES, Long.MAX_VALUE);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).family(familyBytes).build();
+ Key keyExpected = new Key(rowBytes, familyBytes, EMPTY_BYTES, EMPTY_BYTES, Long.MAX_VALUE);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).family(familyBytes).qualifier(qualifierBytes).build();
+ Key keyExpected = new Key(rowBytes, familyBytes, qualifierBytes, EMPTY_BYTES, Long.MAX_VALUE);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).family(familyBytes).qualifier(qualifierBytes).visibility(visibilityBytes).build();
+ Key keyExpected = new Key(rowBytes, familyBytes, qualifierBytes, visibilityBytes, Long.MAX_VALUE);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityTimestampBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).family(familyBytes).qualifier(qualifierBytes).visibility(visibilityBytes).timestamp(1L).build();
+ Key keyExpected = new Key(rowBytes, familyBytes, qualifierBytes, visibilityBytes, 1L);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityTimestampDeletedBytes() {
+ Key keyBuilt =
+ Key.builder()
+ .row(rowBytes)
+ .family(familyBytes)
+ .qualifier(qualifierBytes)
+ .visibility(visibilityBytes)
+ .timestamp(10L)
+ .deleted(true)
+ .build();
+ Key keyExpected = new Key(rowBytes, familyBytes, qualifierBytes, visibilityBytes, 10L);
+ keyExpected.setDeleted(true);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowVisibilityBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).visibility(visibilityBytes).build();
+ Key keyExpected = new Key(rowBytes, EMPTY_BYTES, EMPTY_BYTES, visibilityBytes, Long.MAX_VALUE);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyVisibilityBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).family(familyBytes).visibility(visibilityBytes).build();
+ Key keyExpected = new Key(rowBytes, familyBytes, EMPTY_BYTES, visibilityBytes, Long.MAX_VALUE);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void textKeyBuildingFromRowTimestampBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).timestamp(3L).build();
+ Key keyExpected = new Key(rowBytes, EMPTY_BYTES, EMPTY_BYTES, EMPTY_BYTES, Long.MAX_VALUE);
+ keyExpected.setTimestamp(3L);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowText() {
+ Key keyBuilt = Key.builder().row(rowText).build();
+ Key keyExpected = new Key(rowText);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyText() {
+ Key keyBuilt = Key.builder().row(rowText).family(familyText).build();
+ Key keyExpected = new Key(rowText, familyText);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierText() {
+ Key keyBuilt = Key.builder().row(rowText).family(familyText).qualifier(qualifierText).build();
+ Key keyExpected = new Key(rowText, familyText, qualifierText);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityText() {
+ Key keyBuilt = Key.builder().row(rowText).family(familyText).qualifier(qualifierText).visibility(visibilityText).build();
+ Key keyExpected = new Key(rowText, familyText, qualifierText, visibilityText);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityTimestampText() {
+ Key keyBuilt = Key.builder().row(rowText).family(familyText).qualifier(qualifierText).visibility(visibilityText).timestamp(1L).build();
+ Key keyExpected = new Key(rowText, familyText, qualifierText, visibilityText, 1L);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityTimestampDeletedText() {
+ Key keyBuilt =
+ Key.builder()
+ .row(rowText)
+ .family(familyText)
+ .qualifier(qualifierText)
+ .visibility(visibilityText)
+ .timestamp(10L)
+ .deleted(true)
+ .build();
+ Key keyExpected = new Key(rowText, familyText, qualifierText, visibilityText, 10L);
+ keyExpected.setDeleted(true);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowVisibilityText() {
+ Key keyBuilt = Key.builder().row(rowText).visibility(visibilityText).build();
+ Key keyExpected = new Key(rowText, new Text(), new Text(), visibilityText);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyVisibilityText() {
+ Key keyBuilt = Key.builder().row(rowText).family(familyText).visibility(visibilityText).build();
+ Key keyExpected = new Key(rowText, familyText, new Text(), visibilityText);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowTimestampText() {
+ Key keyBuilt = Key.builder().row(rowText).timestamp(3L).build();
+ Key keyExpected = new Key(rowText);
+ keyExpected.setTimestamp(3L);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingReusingBytes() {
+ byte[] reuse = new byte[] { 1, 2, 3 };
+ KeyBuilder.Build keyBuilder = Key.builder(false).row(reuse);
+ Key keyBuilt = keyBuilder.build();
+ assertEquals(reuse, keyBuilt.getRowBytes());
--- End diff --
`assertSame()` appears to work with local testing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/accumulo/pull/145
Fine by me. I just had the two comments (javadoc cleanup and the unnecessary string->text->bytes conversion). It's all recorded on JIRA for posterity.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77424294
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,300 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build {@link Key}s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the {@link Key} are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build {
--- End diff --
Added `@since`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77279515
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,300 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build {@link Key}s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the {@link Key} are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build {
+
+ /**
+ * Build a {@link Key} from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the {@link Key} built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the {@link Key} created.
+ *
+ * @param timestamp
+ * the timestamp to use for the {@link Key}
+ * @return this builder
+ */
+ Build timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the {@link Key} to the parameter.
+ *
+ * @param deleted
+ * if the {@link Key} should be marked as deleted or not
+ * @return this builder
+ */
+ Build deleted(boolean deleted);
+ }
+
+ public interface RowStep extends Build {
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final Text row);
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final byte[] row);
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
--- End diff --
For the methods that accept CharSequence, the javadoc should state it will be UTF-8 encoded.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on the issue:
https://github.com/apache/accumulo/pull/145
@keith-turner Ok I'll add the possibility to use different data types. I wasn't aware about this requirement and didn't design for it. I'll need some time.
I have to disagree in removing the `column` from the column components. It may be easier for who knows Accumulo but not for the new developer and being used to see `columnFamily` or `colFam` everywhere would make this choice particularly difficult imho for no reasons.
I don't understand why the `newBuilder()` method. What problem does it solve? The issue on doing this is that `row` cannot be final in the `KeyBuilder` anymore and must default to `null` when the `KeyBuilder` is built, which means that I have another `null` around for a value that cannot be `null` and must be set. That's why I prefer to combine passing the `row` and build the builder together, so I can use the constructor of the builder to set the `row` asap and skip a `null` that eventually must be not `null`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on the issue:
https://github.com/apache/accumulo/pull/145
Tests are failing on both Travis and my computer but for different reasons. On my computer the tests failing are
```java
Failed tests:
AccumuloReloadingVFSClassLoaderTest.testReloading:127
ContextManagerTest.differentContexts:121
```
but I'm quite sure I didn't change anything related. I usually would fix the problems before submitting the code but I'm clueless. Can somebody with better knowledge of the project what is wrong with this PR and why those tests fail? The error that is printed is: `org.apache.accumulo.start.classloader.vfs.ContextManagerTest.differentContexts(ContextManagerTest.java:121)`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/accumulo/pull/145
> Good, done and also change since to 2.0.
This looks good. One last thing is the `assertSame()` call. Want to fix that too? I am also ok fixing on that local commit.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77915578
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,420 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.accumulo.core.security.ColumnVisibility;
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build {@link Key}s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the {@link Key} are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ * <code>CharSequence</code>s must be UTF-8 encoded.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.9
+ */
+public class KeyBuilder {
+
+ /**
+ * @since 1.9
+ */
+ public interface Build {
+
+ /**
+ * Build a {@link Key} from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the {@link Key} built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the {@link Key} created.
+ *
+ * @param timestamp
+ * the timestamp to use for the {@link Key}
+ * @return this builder
+ */
+ Build timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the {@link Key} to the parameter.
+ *
+ * @param deleted
+ * if the {@link Key} should be marked as deleted or not
+ * @return this builder
+ */
+ Build deleted(boolean deleted);
+ }
+
+ /**
+ * @since 1.9
--- End diff --
Done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by dhutchis <gi...@git.apache.org>.
Github user dhutchis commented on the issue:
https://github.com/apache/accumulo/pull/145
The `Key.newBuilder()` also mirrors common practice in Guava and other libraries, that developers may be more familiar with. For example, [`ImmutableList.builder()`](https://google.github.io/guava/releases/snapshot/api/docs/com/google/common/collect/ImmutableList.html#builder()). Of course this comes down to style and the sequence you suggest could work too.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77247660
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,278 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build <code>Key</code>s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the <code>Key</code> are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build<T> {
+
+ /**
+ * Build a <code>Key</code> from this builder.
+ *
+ * @param copyBytes
+ * if the byte arrays should be copied or not. If not, byte arrays will be reused in the resultant <code>Key</code>
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build(boolean copyBytes);
+
+ /**
+ * Build a <code>Key</code> from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the <code>Key</code> created.
+ *
+ * @param timestamp
+ * the timestamp to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the <code>Key</code> to the parameter.
+ *
+ * @param deleted
+ * if the <code>Key</code> should be marked as deleted or not
+ * @return this builder
+ */
+ Build<T> deleted(boolean deleted);
+ }
+
+ public interface ColumnFamilyStep<T> extends ColumnVisibilityStep<T> {
--- End diff --
The column family has to be set to something. We could make it default to empty string. Personally I don't think we should provide a short cut in the API for setting the family to an empty string. For this fringe case the user can just pass an empty string.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on the issue:
https://github.com/apache/accumulo/pull/145
@keith-turner @dhutchis I pushed another commit with the design changes we discussed. Now it is possible to write
```java
Key.builder().row(new Text("row")).family(new byte[] { 1, 2, 3 }).qualifier("foo").build()
```
to build a `Key`. There is still to decide about column family, qualifier and visibility names and, while my point is that we should be consistent and keep the `column` before the component name, I don't have a strong opinion about that.
@joshelser I added two tests, one to check if the bytes are reused if `copyBytes` is `false` and one to be sure that they are not if `copyBytes` is `true`.
p.s. because a lot of people are commenting, I'm going to pile new commits and then squash them at the end, if and when you decide to merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on the issue:
https://github.com/apache/accumulo/pull/145
The new `KeyBuilderImpl` now keeps not only the byte arrays but also offsets and lengths of each byte and each method now takes this offset and length as optional parameters. In this way we are able to reuse byte arrays to create `Key`s:
```java
byte[] keyBytes = new byte[] { 0, 1, 2, 3, 4, 5 }
Key key = Key.builder().row(keyBytes, 0, 2).column(keyBytes, 2, 2).qualifier(keyBytes, 4, 2).build();
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by madrob <gi...@git.apache.org>.
Github user madrob commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77262611
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,278 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build <code>Key</code>s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the <code>Key</code> are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build<T> {
+
+ /**
+ * Build a <code>Key</code> from this builder.
+ *
+ * @param copyBytes
+ * if the byte arrays should be copied or not. If not, byte arrays will be reused in the resultant <code>Key</code>
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build(boolean copyBytes);
+
+ /**
+ * Build a <code>Key</code> from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the <code>Key</code> created.
+ *
+ * @param timestamp
+ * the timestamp to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the <code>Key</code> to the parameter.
+ *
+ * @param deleted
+ * if the <code>Key</code> should be marked as deleted or not
+ * @return this builder
+ */
+ Build<T> deleted(boolean deleted);
+ }
+
+ public interface ColumnFamilyStep<T> extends ColumnVisibilityStep<T> {
--- End diff --
:+1:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77647186
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,420 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.accumulo.core.security.ColumnVisibility;
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build {@link Key}s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the {@link Key} are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ * <code>CharSequence</code>s must be UTF-8 encoded.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.9
+ */
+public class KeyBuilder {
+
+ /**
+ * @since 1.9
+ */
+ public interface Build {
+
+ /**
+ * Build a {@link Key} from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the {@link Key} built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the {@link Key} created.
+ *
+ * @param timestamp
+ * the timestamp to use for the {@link Key}
+ * @return this builder
+ */
+ Build timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the {@link Key} to the parameter.
+ *
+ * @param deleted
+ * if the {@link Key} should be marked as deleted or not
+ * @return this builder
+ */
+ Build deleted(boolean deleted);
+ }
+
+ /**
+ * @since 1.9
--- End diff --
We should probably have some sort of javadoc description for these interfaces (RowStep, ColumnFamilyStep, etc) since they are in the public API. One sentence should be sufficient.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on the issue:
https://github.com/apache/accumulo/pull/145
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by madrob <gi...@git.apache.org>.
Github user madrob commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77242827
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,278 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build <code>Key</code>s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the <code>Key</code> are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build<T> {
+
+ /**
+ * Build a <code>Key</code> from this builder.
+ *
+ * @param copyBytes
+ * if the byte arrays should be copied or not. If not, byte arrays will be reused in the resultant <code>Key</code>
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build(boolean copyBytes);
+
+ /**
+ * Build a <code>Key</code> from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the <code>Key</code> created.
+ *
+ * @param timestamp
+ * the timestamp to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the <code>Key</code> to the parameter.
+ *
+ * @param deleted
+ * if the <code>Key</code> should be marked as deleted or not
+ * @return this builder
+ */
+ Build<T> deleted(boolean deleted);
+ }
+
+ public interface ColumnFamilyStep<T> extends ColumnVisibilityStep<T> {
+
+ /**
+ * Set the column family of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the <code>Key</code>
+ * @return this builder
+ */
+ ColumnQualifierStep<T> columnFamily(final T columnFamily);
+
+ /**
+ * Set the column family, qualifier and visibility of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the <code>Key</code>
+ * @param columnQualifier
+ * the column qualifier to use for the <code>Key</code>
+ * @param columnVisibility
+ * the column visibility to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> column(final T columnFamily, final T columnQualifier, final T columnVisibility);
+
+ /**
+ * Set the column family and the qualifier of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the <code>Key</code>
+ * @param columnQualifier
+ * the column qualifier to use for the <code>Key</code>
+ * @return this builder
+ */
+ ColumnVisibilityStep<T> column(final T columnFamily, final T columnQualifier);
+ }
+
+ public interface ColumnQualifierStep<T> extends ColumnVisibilityStep<T> {
+
+ /**
+ * Set the column qualifier of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the <code>Key</code>
+ * @return this builder
+ */
+ ColumnVisibilityStep<T> columnQualifier(final T columnQualifier);
+ }
+
+ public interface ColumnVisibilityStep<T> extends Build<T> {
+ /**
+ * Set the column qualifier of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> columnVisibility(final T columnVisibility);
+ }
+
+ private static abstract class AbstractKeyBuilder<T> implements ColumnFamilyStep<T>, ColumnQualifierStep<T>,
+ ColumnVisibilityStep<T> {
+
+ protected static final byte EMPTY_BYTES[] = new byte[0];
+
+ protected T row = null;
+ protected T columnFamily = null;
+ protected T columnQualifier = null;
+ protected T columnVisibility = null;
+ protected long timestamp = Long.MAX_VALUE;
+ protected boolean deleted = false;
+
+ final public ColumnFamilyStep<T> row(final T row) {
+ this.row = row;
+ return this;
+ }
+
+ @Override
+ final public ColumnQualifierStep<T> columnFamily(final T columnFamily) {
+ this.columnFamily = columnFamily;
+ return this;
+ }
+
+ @Override
+ final public ColumnVisibilityStep<T> columnQualifier(final T columnQualifier) {
+ this.columnQualifier = columnQualifier;
+ return this;
+ }
+
+ @Override
+ final public Build<T> columnVisibility(final T columnVisibility) {
+ this.columnVisibility = columnVisibility;
+ return this;
+ }
+
+ @Override
+ final public Build<T> timestamp(long timestamp) {
+ this.timestamp = timestamp;
+ return this;
+ }
+
+ @Override
+ public Build<T> deleted(boolean deleted) {
+ this.deleted = deleted;
+ return this;
+ }
+
+ @Override
+ public Key build() {
+ return this.build(true);
+ }
+
+ @Override
+ public Build<T> column(final T columnFamily, final T columnQualifier, final T columnVisibility) {
+ return this.columnFamily(columnFamily).columnQualifier(columnQualifier).columnVisibility(columnVisibility);
+ }
+
+ @Override
+ public ColumnVisibilityStep<T> column(final T columnFamily, final T columnQualifier) {
+ return this.columnFamily(columnFamily).columnQualifier(columnQualifier);
+ }
+ }
+
+ private static class TextKeyBuilder extends AbstractKeyBuilder<Text> {
+
+ private final Text EMPTY_TEXT = new Text();
+
+ @Override
+ public Key build(boolean copyBytes) {
+ Text columnFamily = (this.columnFamily == null) ? EMPTY_TEXT : this.columnFamily;
+ Text columnQualifier = (this.columnQualifier == null) ? EMPTY_TEXT : this.columnQualifier;
+ Text columnVisibility = (this.columnVisibility == null) ? EMPTY_TEXT : this.columnVisibility;
+ return new Key(row.getBytes(), 0, row.getLength(), columnFamily.getBytes(), 0, columnFamily.getLength(),
+ columnQualifier.getBytes(), 0, columnQualifier.getLength(), columnVisibility.getBytes(), 0,
+ columnVisibility.getLength(), timestamp, deleted, copyBytes);
+ }
+ }
+
+ private static class ByteArrayKeyBuilder extends AbstractKeyBuilder<byte[]> {
+
+ @Override
+ public Key build(boolean copyBytes) {
+ byte[] columnFamily = (this.columnFamily == null) ? EMPTY_BYTES : this.columnFamily;
+ byte[] columnQualifier = (this.columnQualifier == null) ? EMPTY_BYTES : this.columnQualifier;
+ byte[] columnVisibility = (this.columnVisibility == null) ? EMPTY_BYTES : this.columnVisibility;
+ return new Key(row, columnFamily, columnQualifier, columnVisibility, timestamp, deleted, copyBytes);
+ }
+ }
+
+ private static class CharSequenceKeyBuilder extends AbstractKeyBuilder<CharSequence> {
+
+ private final Text EMPTY_TEXT = new Text();
+
+ @Override
+ public Key build(boolean copyBytes) {
--- End diff --
Does `copyBytes` change the behaviour here? Looks like you're already copying when you build the Text objects the first time.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77915765
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,420 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.accumulo.core.security.ColumnVisibility;
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build {@link Key}s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the {@link Key} are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ * <code>CharSequence</code>s must be UTF-8 encoded.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.9
+ */
+public class KeyBuilder {
+
+ /**
+ * @since 1.9
+ */
+ public interface Build {
+
+ /**
+ * Build a {@link Key} from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the {@link Key} built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the {@link Key} created.
+ *
+ * @param timestamp
+ * the timestamp to use for the {@link Key}
+ * @return this builder
+ */
+ Build timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the {@link Key} to the parameter.
+ *
+ * @param deleted
+ * if the {@link Key} should be marked as deleted or not
+ * @return this builder
+ */
+ Build deleted(boolean deleted);
+ }
+
+ /**
+ * @since 1.9
+ */
+ public interface RowStep extends Build {
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final Text row);
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final byte[] row);
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @param offset
+ * the offset within the array of the first byte to be read; must be non-negative and no larger than row.length
+ * @param length
+ * the number of bytes to be read from the given array; must be non-negative and no larger than row.length - offset
+ * @return this builder
+ */
+ ColumnFamilyStep row(final byte[] row, int offset, int length);
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key. The encoding must be UTF-8
+ * @return this builder
+ */
+ ColumnFamilyStep row(final CharSequence row);
+ }
+
+ /**
+ * @since 1.9
+ */
+ public interface ColumnFamilyStep extends ColumnVisibilityStep {
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnQualifierStep family(final byte[] columnFamily);
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}
+ * @param offset
+ * the offset within the array of the first byte to be read; must be non-negative and no larger than row.length
+ * @param length
+ * the number of bytes to be read from the given array; must be non-negative and no larger than row.length - offset
+ * @return this builder
+ */
+ ColumnQualifierStep family(final byte[] columnFamily, int offset, int length);
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnQualifierStep family(final Text columnFamily);
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}. The encoding must be UTF-8
+ * @return this builder
+ */
+ ColumnQualifierStep family(final CharSequence columnFamily);
+ }
+
+ /**
+ * @since 1.9
+ */
+ public interface ColumnQualifierStep extends ColumnVisibilityStep {
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final byte[] columnQualifier);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}
+ * @param offset
+ * the offset within the array of the first byte to be read; must be non-negative and no larger than row.length
+ * @param length
+ * the number of bytes to be read from the given array; must be non-negative and no larger than row.length - offset
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final byte[] columnQualifier, int offset, int length);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final Text columnQualifier);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}. The encoding must be UTF-8
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final CharSequence columnQualifier);
+ }
+
+ /**
+ * @since 1.9
+ */
+ public interface ColumnVisibilityStep extends Build {
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the {@link Key}
+ * @return this builder
+ */
+ Build visibility(final byte[] columnVisibility);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the {@link Key}
+ * @param offset
+ * the offset within the array of the first byte to be read; must be non-negative and no larger than row.length
+ * @param length
+ * the number of bytes to be read from the given array; must be non-negative and no larger than row.length - offset
+ * @return this builder
+ */
+ Build visibility(final byte[] columnVisibility, int offset, int length);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the {@link Key}
+ * @return this builder
+ */
+ Build visibility(final Text columnVisibility);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the {@link Key}. The encoding must be UTF-8
+ * @return this builder
+ */
+ Build visibility(final CharSequence columnVisibility);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the {@link Key}
+ * @return this builder
+ */
+ Build visibility(final ColumnVisibility columnVisibility);
+ }
+
+ /**
+ * @since 1.9
+ */
+ static class KeyBuilderImpl implements RowStep, ColumnFamilyStep, ColumnQualifierStep,
+ ColumnVisibilityStep {
+
+ protected static final byte EMPTY_BYTES[] = new byte[0];
+
+ private final boolean copyBytes;
+ private byte[] row = EMPTY_BYTES;
+ private int rowOffset = 0;
+ private int rowLength = 0;
+ private byte[] family = EMPTY_BYTES;
+ private int familyOffset = 0;
+ private int familyLength = 0;
+ private byte[] qualifier = EMPTY_BYTES;
+ private int qualifierOffset = 0;
+ private int qualifierLength = 0;
+ private byte[] visibility = EMPTY_BYTES;
+ private int visibilityOffset = 0;
+ private int visibilityLength = 0;
+ private long timestamp = Long.MAX_VALUE;
+ private boolean deleted = false;
+
+ KeyBuilderImpl(boolean copyBytes) {
+ this.copyBytes = copyBytes;
+ }
+
+ private byte[] copyBytesIfNeeded(final byte[] bytes, int offset, int length) {
+ return Key.copyIfNeeded(bytes, offset, length, this.copyBytes);
+ }
+
+ @Override
+ public ColumnFamilyStep row(final byte[] row, int offset, int length) {
+ this.row = copyBytesIfNeeded(row, offset, length);
+ this.rowOffset = this.copyBytes ? 0 : offset;
+ this.rowLength = this.copyBytes ? this.row.length : length;
+ return this;
+ }
+
+ @Override
+ public ColumnFamilyStep row(final byte[] row) {
+ return row(row, 0, row.length);
+ }
+
+ @Override
+ public ColumnFamilyStep row(final Text row) {
+ return row(row.getBytes(), 0, row.getLength());
+ }
+
+ @Override
+ public ColumnFamilyStep row(final CharSequence row) {
+ return row(new Text(row.toString()));
--- End diff --
Done, now I'm using `CharBuffer` to encode any `CharSequence`. If the `CharSequence` is not UTF-8 coded, the `KeyBuilder` throws a `RuntimeException` containing a `CharacterCodingException`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by madrob <gi...@git.apache.org>.
Github user madrob commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77241539
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,278 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build <code>Key</code>s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the <code>Key</code> are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build<T> {
+
+ /**
+ * Build a <code>Key</code> from this builder.
+ *
+ * @param copyBytes
+ * if the byte arrays should be copied or not. If not, byte arrays will be reused in the resultant <code>Key</code>
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build(boolean copyBytes);
+
+ /**
+ * Build a <code>Key</code> from this builder. copyBytes defaults to true.
--- End diff --
Nit: Use `{@link Key}` instead of `<code>Key</code>`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by madrob <gi...@git.apache.org>.
Github user madrob commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77259102
--- Diff: core/src/test/java/org/apache/accumulo/core/data/KeyBuilderTest.java ---
@@ -0,0 +1,239 @@
+package org.apache.accumulo.core.data;
+
+import org.apache.hadoop.io.Text;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class KeyBuilderTest {
+
+ private static final byte EMPTY_BYTES[] = new byte[0];
+ byte[] rowBytes = "row".getBytes();
--- End diff --
Nit: Specify UTF-8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on the issue:
https://github.com/apache/accumulo/pull/145
Agreed, then we will have `Key.builder()`. @keith-turner about the `column` components, I'm a bit torn about names without `column` but yes, I think having good documentation together with the fact that the builder forces you into selecting one components at time could do the trick. Probably a simple example directly in the doc of `KeyBuilder` would be enough for new devs.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on the issue:
https://github.com/apache/accumulo/pull/145
> Presently, the only branch available to land this in is master because semver prevents us from adding new APIs to 1.8.
@melrief, currently the version in master in 2.0.0-SNAP. So would need to change the since version in javadocs to 2.0. I had commented on this earlier and was uncertain, sorry for the confusion. Not sure why I was uncertain, its really very simple to figure out.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77098365
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,278 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build <code>Key</code>s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the <code>Key</code> are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build<T> {
+
+ /**
+ * Build a <code>Key</code> from this builder.
+ *
+ * @param copyBytes
+ * if the byte arrays should be copied or not. If not, byte arrays will be reused in the resultant <code>Key</code>
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build(boolean copyBytes);
+
+ /**
+ * Build a <code>Key</code> from this builder. copyBytes defaults to true.
--- End diff --
Nit: "Build a <code>Key</code> from this builder, copying any provided byte arrays." or similar.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by dhutchis <gi...@git.apache.org>.
Github user dhutchis commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77121529
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,278 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build <code>Key</code>s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the <code>Key</code> are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build<T> {
+
+ /**
+ * Build a <code>Key</code> from this builder.
+ *
+ * @param copyBytes
+ * if the byte arrays should be copied or not. If not, byte arrays will be reused in the resultant <code>Key</code>
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build(boolean copyBytes);
+
+ /**
+ * Build a <code>Key</code> from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the <code>Key</code> built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the <code>Key</code> created.
+ *
+ * @param timestamp
+ * the timestamp to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the <code>Key</code> to the parameter.
+ *
+ * @param deleted
+ * if the <code>Key</code> should be marked as deleted or not
+ * @return this builder
+ */
+ Build<T> deleted(boolean deleted);
+ }
+
+ public interface ColumnFamilyStep<T> extends ColumnVisibilityStep<T> {
+
+ /**
+ * Set the column family of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the <code>Key</code>
+ * @return this builder
+ */
+ ColumnQualifierStep<T> columnFamily(final T columnFamily);
+
+ /**
+ * Set the column family, qualifier and visibility of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the <code>Key</code>
+ * @param columnQualifier
+ * the column qualifier to use for the <code>Key</code>
+ * @param columnVisibility
+ * the column visibility to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> column(final T columnFamily, final T columnQualifier, final T columnVisibility);
+
+ /**
+ * Set the column family and the qualifier of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the <code>Key</code>
+ * @param columnQualifier
+ * the column qualifier to use for the <code>Key</code>
+ * @return this builder
+ */
+ ColumnVisibilityStep<T> column(final T columnFamily, final T columnQualifier);
+ }
+
+ public interface ColumnQualifierStep<T> extends ColumnVisibilityStep<T> {
+
+ /**
+ * Set the column qualifier of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the <code>Key</code>
+ * @return this builder
+ */
+ ColumnVisibilityStep<T> columnQualifier(final T columnQualifier);
+ }
+
+ public interface ColumnVisibilityStep<T> extends Build<T> {
+ /**
+ * Set the column qualifier of the <code>Key</code> that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the <code>Key</code>
+ * @return this builder
+ */
+ Build<T> columnVisibility(final T columnVisibility);
+ }
+
+ private static abstract class AbstractKeyBuilder<T> implements ColumnFamilyStep<T>, ColumnQualifierStep<T>,
+ ColumnVisibilityStep<T> {
+
+ protected static final byte EMPTY_BYTES[] = new byte[0];
+
+ protected T row = null;
+ protected T columnFamily = null;
+ protected T columnQualifier = null;
+ protected T columnVisibility = null;
+ protected long timestamp = Long.MAX_VALUE;
+ protected boolean deleted = false;
+
+ final public ColumnFamilyStep<T> row(final T row) {
+ this.row = row;
+ return this;
+ }
+
+ @Override
+ final public ColumnQualifierStep<T> columnFamily(final T columnFamily) {
+ this.columnFamily = columnFamily;
+ return this;
+ }
+
+ @Override
+ final public ColumnVisibilityStep<T> columnQualifier(final T columnQualifier) {
+ this.columnQualifier = columnQualifier;
+ return this;
+ }
+
+ @Override
+ final public Build<T> columnVisibility(final T columnVisibility) {
+ this.columnVisibility = columnVisibility;
+ return this;
+ }
+
+ @Override
+ final public Build<T> timestamp(long timestamp) {
+ this.timestamp = timestamp;
+ return this;
+ }
+
+ @Override
+ public Build<T> deleted(boolean deleted) {
+ this.deleted = deleted;
+ return this;
+ }
+
+ @Override
+ public Key build() {
+ return this.build(true);
+ }
+
+ @Override
+ public Build<T> column(final T columnFamily, final T columnQualifier, final T columnVisibility) {
+ return this.columnFamily(columnFamily).columnQualifier(columnQualifier).columnVisibility(columnVisibility);
+ }
+
+ @Override
+ public ColumnVisibilityStep<T> column(final T columnFamily, final T columnQualifier) {
+ return this.columnFamily(columnFamily).columnQualifier(columnQualifier);
+ }
+ }
+
+ private static class TextKeyBuilder extends AbstractKeyBuilder<Text> {
+
+ private final Text EMPTY_TEXT = new Text();
+
+ @Override
+ public Key build(boolean copyBytes) {
+ Text columnFamily = (this.columnFamily == null) ? EMPTY_TEXT : this.columnFamily;
+ Text columnQualifier = (this.columnQualifier == null) ? EMPTY_TEXT : this.columnQualifier;
+ Text columnVisibility = (this.columnVisibility == null) ? EMPTY_TEXT : this.columnVisibility;
+ return new Key(row.getBytes(), 0, row.getLength(), columnFamily.getBytes(), 0, columnFamily.getLength(),
+ columnQualifier.getBytes(), 0, columnQualifier.getLength(), columnVisibility.getBytes(), 0,
+ columnVisibility.getLength(), timestamp, deleted, copyBytes);
+ }
+ }
+
+ private static class ByteArrayKeyBuilder extends AbstractKeyBuilder<byte[]> {
--- End diff --
A `ByteBuffer` version would be nice, for those users that build keys from contiguous segments of a large byte[].
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on the issue:
https://github.com/apache/accumulo/pull/145
I agree w/ @dhutchis and would really like the ability to mix types. For example a binary row and string column is a common usage pattern. The builder pattern supports this usage pattern really well.
I would suggest using `family` instead of `columnFamily`, `qualifier` instead `columnQualifier`, and `visibility` instead of `columnVisibility` for the methods names. IMO using these names will make the code users write shorter and less redundant w/o any loss in readability.
I would suggest making the builder class package private and adding a `newBuilder()` method to Key that returns the first interface.
Below is an example of code using all of these suggestions.
```java
byte r[];
String f;
Text q;
Key k = Key.newBuilder().row(r).family(f).qualifier(q).build();
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/accumulo/pull/145
> Tests are failing on both Travis and my computer but for different reasons. On my computer the tests failing are
I'd agree with you that these are unrelated to your changes. Just ignore them for now -- we'll have to figure out why they're busted (again).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77282867
--- Diff: core/src/main/java/org/apache/accumulo/core/data/Key.java ---
@@ -51,6 +51,26 @@
protected long timestamp;
protected boolean deleted;
+ /**
+ * Create a {@link Key} builder.
+ *
+ * @param copyBytes
+ * if the bytes of the {@link Key} components should be copied
+ * @return the builder at the {@link KeyBuilder.RowStep}
+ */
+ public static KeyBuilder.RowStep builder(boolean copyBytes) {
+ return new KeyBuilder.AbstractKeyBuilder(copyBytes);
+ }
+
+ /**
+ * Create a {@link Key} builder. Copy bytes defaults to true.
+ *
+ * @return the builder at the {@link KeyBuilder.RowStep}
+ */
+ public static KeyBuilder.RowStep builder() {
--- End diff --
Need to add `@since` tags to these two new methods in Key.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77424318
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,300 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build {@link Key}s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the {@link Key} are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
--- End diff --
Set to 1.9 but we can change it if needed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/accumulo/pull/145
I was able to easily (without conflict) rebase this branch onto master. Let me(us) know if you want to make the rest of the fixes @melrief, otherwise, I can just make them on commit (since they're all rather simple).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77424180
--- Diff: core/src/main/java/org/apache/accumulo/core/data/Key.java ---
@@ -51,6 +51,26 @@
protected long timestamp;
protected boolean deleted;
+ /**
+ * Create a {@link Key} builder.
+ *
+ * @param copyBytes
+ * if the bytes of the {@link Key} components should be copied
+ * @return the builder at the {@link KeyBuilder.RowStep}
+ */
+ public static KeyBuilder.RowStep builder(boolean copyBytes) {
--- End diff --
Well, this `KeyBuilder` is strict while the previous implementation was lazy which means that now I can't set the building option at the end. IMHO it's nice also in this way.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77651069
--- Diff: core/src/test/java/org/apache/accumulo/core/data/KeyBuilderTest.java ---
@@ -0,0 +1,282 @@
+/*
+ * 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.accumulo.core.data;
+
+import org.apache.hadoop.io.Text;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class KeyBuilderTest {
+
+ private static final byte EMPTY_BYTES[] = new byte[0];
+ byte[] rowBytes = "row".getBytes();
+ byte[] familyBytes = "family".getBytes();
+ byte[] qualifierBytes = "qualifier".getBytes();
+ byte[] visibilityBytes = "visibility".getBytes();
+ Text rowText = new Text(rowBytes);
+ Text familyText = new Text(familyBytes);
+ Text qualifierText = new Text(qualifierBytes);
+ Text visibilityText = new Text(visibilityBytes);
+
+ @Test
+ public void testKeyBuildingFromRow() {
+ Key keyBuilt = Key.builder().row("foo").build();
+ Key keyExpected = new Key("foo");
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamily() {
+ Key keyBuilt = Key.builder().row("foo").family("bar").build();
+ Key keyExpected = new Key("foo", "bar");
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifier() {
+ Key keyBuilt = Key.builder().row("foo").family("bar").qualifier("baz").build();
+ Key keyExpected = new Key("foo", "bar", "baz");
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibility() {
+ Key keyBuilt = Key.builder().row("foo").family("bar").qualifier("baz").visibility("v").build();
+ Key keyExpected = new Key("foo", "bar", "baz", "v");
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityTimestamp() {
+ Key keyBuilt = Key.builder().row("foo").family("bar").qualifier("baz").visibility("v").timestamp(1L).build();
+ Key keyExpected = new Key("foo", "bar", "baz", "v", 1L);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityTimestampDeleted() {
+ Key keyBuilt =
+ Key.builder()
+ .row("foo")
+ .family("bar")
+ .qualifier("baz")
+ .visibility("v")
+ .timestamp(10L)
+ .deleted(true)
+ .build();
+ Key keyExpected = new Key("foo", "bar", "baz", "v", 10L);
+ keyExpected.setDeleted(true);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowVisibility() {
+ Key keyBuilt = Key.builder().row("foo").visibility("v").build();
+ Key keyExpected = new Key("foo", "", "", "v");
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyVisibility() {
+ Key keyBuilt = Key.builder().row("foo").family("bar").visibility("v").build();
+ Key keyExpected = new Key("foo", "bar", "", "v");
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void textKeyBuildingFromRowTimestamp() {
+ Key keyBuilt = Key.builder().row("foo").timestamp(3L).build();
+ Key keyExpected = new Key("foo");
+ keyExpected.setTimestamp(3L);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).build();
+ Key keyExpected = new Key(rowBytes, EMPTY_BYTES, EMPTY_BYTES, EMPTY_BYTES, Long.MAX_VALUE);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).family(familyBytes).build();
+ Key keyExpected = new Key(rowBytes, familyBytes, EMPTY_BYTES, EMPTY_BYTES, Long.MAX_VALUE);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).family(familyBytes).qualifier(qualifierBytes).build();
+ Key keyExpected = new Key(rowBytes, familyBytes, qualifierBytes, EMPTY_BYTES, Long.MAX_VALUE);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).family(familyBytes).qualifier(qualifierBytes).visibility(visibilityBytes).build();
+ Key keyExpected = new Key(rowBytes, familyBytes, qualifierBytes, visibilityBytes, Long.MAX_VALUE);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityTimestampBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).family(familyBytes).qualifier(qualifierBytes).visibility(visibilityBytes).timestamp(1L).build();
+ Key keyExpected = new Key(rowBytes, familyBytes, qualifierBytes, visibilityBytes, 1L);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityTimestampDeletedBytes() {
+ Key keyBuilt =
+ Key.builder()
+ .row(rowBytes)
+ .family(familyBytes)
+ .qualifier(qualifierBytes)
+ .visibility(visibilityBytes)
+ .timestamp(10L)
+ .deleted(true)
+ .build();
+ Key keyExpected = new Key(rowBytes, familyBytes, qualifierBytes, visibilityBytes, 10L);
+ keyExpected.setDeleted(true);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowVisibilityBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).visibility(visibilityBytes).build();
+ Key keyExpected = new Key(rowBytes, EMPTY_BYTES, EMPTY_BYTES, visibilityBytes, Long.MAX_VALUE);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyVisibilityBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).family(familyBytes).visibility(visibilityBytes).build();
+ Key keyExpected = new Key(rowBytes, familyBytes, EMPTY_BYTES, visibilityBytes, Long.MAX_VALUE);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void textKeyBuildingFromRowTimestampBytes() {
+ Key keyBuilt = Key.builder().row(rowBytes).timestamp(3L).build();
+ Key keyExpected = new Key(rowBytes, EMPTY_BYTES, EMPTY_BYTES, EMPTY_BYTES, Long.MAX_VALUE);
+ keyExpected.setTimestamp(3L);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowText() {
+ Key keyBuilt = Key.builder().row(rowText).build();
+ Key keyExpected = new Key(rowText);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyText() {
+ Key keyBuilt = Key.builder().row(rowText).family(familyText).build();
+ Key keyExpected = new Key(rowText, familyText);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierText() {
+ Key keyBuilt = Key.builder().row(rowText).family(familyText).qualifier(qualifierText).build();
+ Key keyExpected = new Key(rowText, familyText, qualifierText);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityText() {
+ Key keyBuilt = Key.builder().row(rowText).family(familyText).qualifier(qualifierText).visibility(visibilityText).build();
+ Key keyExpected = new Key(rowText, familyText, qualifierText, visibilityText);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityTimestampText() {
+ Key keyBuilt = Key.builder().row(rowText).family(familyText).qualifier(qualifierText).visibility(visibilityText).timestamp(1L).build();
+ Key keyExpected = new Key(rowText, familyText, qualifierText, visibilityText, 1L);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyQualifierVisibilityTimestampDeletedText() {
+ Key keyBuilt =
+ Key.builder()
+ .row(rowText)
+ .family(familyText)
+ .qualifier(qualifierText)
+ .visibility(visibilityText)
+ .timestamp(10L)
+ .deleted(true)
+ .build();
+ Key keyExpected = new Key(rowText, familyText, qualifierText, visibilityText, 10L);
+ keyExpected.setDeleted(true);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowVisibilityText() {
+ Key keyBuilt = Key.builder().row(rowText).visibility(visibilityText).build();
+ Key keyExpected = new Key(rowText, new Text(), new Text(), visibilityText);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowFamilyVisibilityText() {
+ Key keyBuilt = Key.builder().row(rowText).family(familyText).visibility(visibilityText).build();
+ Key keyExpected = new Key(rowText, familyText, new Text(), visibilityText);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingFromRowTimestampText() {
+ Key keyBuilt = Key.builder().row(rowText).timestamp(3L).build();
+ Key keyExpected = new Key(rowText);
+ keyExpected.setTimestamp(3L);
+ assertEquals(keyExpected, keyBuilt);
+ }
+
+ @Test
+ public void testKeyBuildingReusingBytes() {
+ byte[] reuse = new byte[] { 1, 2, 3 };
+ KeyBuilder.Build keyBuilder = Key.builder(false).row(reuse);
+ Key keyBuilt = keyBuilder.build();
+ assertEquals(reuse, keyBuilt.getRowBytes());
--- End diff --
This actually triggers a findbugs warning too:
```
[INFO] --- findbugs-maven-plugin:3.0.3:check (run-findbugs) @ accumulo-core ---
[INFO] BugInstance size is 1
[INFO] Error size is 0
[INFO] Total bugs: 1
[INFO] Using .equals to compare two byte[]'s, (equivalent to ==) in org.apache.accumulo.core.data.KeyBuilderTest.testKeyBuildingReusingBytes() [org.apache.accumulo.core.data.KeyBuilderTest] At KeyBuilderTest.java:[line 248] EC_BAD_ARRAY_COMPARE
[INFO]
```
Hopefully `assertSame(...)` wouldn't trigger this. Should be able to verify that by running `mvn verify -DskipTests` locally.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77282812
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,300 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build {@link Key}s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the {@link Key} are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build {
--- End diff --
Not 100% sure about this, but I think the `@since` tag from the enclosing class will not show up in the javadoc for this class. So these inner interfaces may needs `@since` tags.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by dhutchis <gi...@git.apache.org>.
Github user dhutchis commented on the issue:
https://github.com/apache/accumulo/pull/145
Good point @keith-turner. I had a contract in mind to not change the state of the ByteBuffer.
Here is a better idea: add methods to accept a byte[], offset, and length. Users of ByteBuffers can call these methods manually via the code at the end of my previous example.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77282915
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,300 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build {@link Key}s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the {@link Key} are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
--- End diff --
This should not be 1.8. Not sure what it should be though. We have been talking about 2.0 being the next release. It should be 1.9 or 2.0.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/accumulo/pull/145
I left a few more comments on trivial things (doc cleanup, primarily), but I like how this is looking! I'm running it through a `mvn verify -Psunny` locally and would be in favor of committing this ASAP. Presently, the only branch available to land this in is `master` because semver prevents us from adding new APIs to 1.8.
@melrief if you can switch the target branch to `master`, that would be helpful.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77374539
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,300 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build {@link Key}s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the {@link Key} are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build {
+
+ /**
+ * Build a {@link Key} from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the {@link Key} built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the {@link Key} created.
+ *
+ * @param timestamp
+ * the timestamp to use for the {@link Key}
+ * @return this builder
+ */
+ Build timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the {@link Key} to the parameter.
+ *
+ * @param deleted
+ * if the {@link Key} should be marked as deleted or not
+ * @return this builder
+ */
+ Build deleted(boolean deleted);
+ }
+
+ public interface RowStep extends Build {
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final Text row);
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final byte[] row);
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final CharSequence row);
+ }
+
+ public interface ColumnFamilyStep extends ColumnVisibilityStep {
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnQualifierStep family(final byte[] columnFamily);
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnQualifierStep family(final Text columnFamily);
+
+ /**
+ * Set the column family of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnFamily
+ * the column family to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnQualifierStep family(final CharSequence columnFamily);
+ }
+
+ public interface ColumnQualifierStep extends ColumnVisibilityStep {
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final byte[] columnQualifier);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final Text columnQualifier);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnQualifier
+ * the column qualifier to use for the {@link Key}
+ * @return this builder
+ */
+ ColumnVisibilityStep qualifier(final CharSequence columnQualifier);
+ }
+
+ public interface ColumnVisibilityStep extends Build {
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the {@link Key}
+ * @return this builder
+ */
+ Build visibility(final byte[] columnVisibility);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the {@link Key}
+ * @return this builder
+ */
+ Build visibility(final Text columnVisibility);
+
+ /**
+ * Set the column qualifier of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param columnVisibility
+ * the column visibility to use for the {@link Key}
+ * @return this builder
+ */
+ Build visibility(final CharSequence columnVisibility);
+ }
+
+ static class AbstractKeyBuilder implements RowStep, ColumnFamilyStep, ColumnQualifierStep,
--- End diff --
`KeyBuilder` is the name of the enclosing class, could call it `KeyBuilderImpl`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on the issue:
https://github.com/apache/accumulo/pull/145
@dhutchis even if a copy is done, if the API accepts ByteBuffers it can help avoid forcing intermediate copies. For example user has to copy ByteBuffer to byte array, pass that byte array to KeyBuilder, and then KeyBuilder copies what it was passed.
ByteBuffers are similar to input streams in that they have a position and reading from it can change that position. One decision we would have to make if we accept ByteBuffers is if we want to change that position or not when reading. If any other parts of API accept ByteBuffer, would need to see what the behavior is. We could possibly hold off on adding ByteBuffer in this PR and open a separate issue to examine adding support for ByteBuffer the API.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/accumulo/pull/145
This is looking _great_, @melrief!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by dhutchis <gi...@git.apache.org>.
Github user dhutchis commented on the issue:
https://github.com/apache/accumulo/pull/145
Here's a thought that may be distracting. I wonder if it is easy to have a single builder that accepts Text, CharSequence, byte[], etc. Such a builder could mix and match the types; for example, a Text row and visibility, a CharSequence family and qualifier. A user might even extend such a builder to add support for building a key from a custom type.
Maybe Java 8 default methods would make this easier, [a la mixins](http://hannesdorfmann.com/android/java-mixins).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77424119
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,300 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build {@link Key}s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the {@link Key} are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build {
+
+ /**
+ * Build a {@link Key} from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the {@link Key} built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the {@link Key} created.
+ *
+ * @param timestamp
+ * the timestamp to use for the {@link Key}
+ * @return this builder
+ */
+ Build timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the {@link Key} to the parameter.
+ *
+ * @param deleted
+ * if the {@link Key} should be marked as deleted or not
+ * @return this builder
+ */
+ Build deleted(boolean deleted);
+ }
+
+ public interface RowStep extends Build {
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final Text row);
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final byte[] row);
--- End diff --
Done
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77424301
--- Diff: core/src/main/java/org/apache/accumulo/core/data/Key.java ---
@@ -51,6 +51,26 @@
protected long timestamp;
protected boolean deleted;
+ /**
+ * Create a {@link Key} builder.
+ *
+ * @param copyBytes
+ * if the bytes of the {@link Key} components should be copied
+ * @return the builder at the {@link KeyBuilder.RowStep}
+ */
+ public static KeyBuilder.RowStep builder(boolean copyBytes) {
+ return new KeyBuilder.AbstractKeyBuilder(copyBytes);
+ }
+
+ /**
+ * Create a {@link Key} builder. Copy bytes defaults to true.
+ *
+ * @return the builder at the {@link KeyBuilder.RowStep}
+ */
+ public static KeyBuilder.RowStep builder() {
--- End diff --
Done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/145#discussion_r77424114
--- Diff: core/src/main/java/org/apache/accumulo/core/data/KeyBuilder.java ---
@@ -0,0 +1,300 @@
+/*
+ * 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.accumulo.core.data;
+
+
+import org.apache.hadoop.io.Text;
+
+/**
+ * A builder used to build {@link Key}s by defining their components.
+ *
+ * The rules are:
+ * <ul>
+ * <li>All components of the {@link Key} are optional except the row</li>
+ * <li>Components not explicitly set default to empty byte array except the timestamp which
+ * defaults to <code>Long.MAX_VALUE</code></li>
+ * <li>The column qualifier can only be set if the column family has been set first</li>
+ * <li>The column visibility can only be set if at least the column family has been set first</li>
+ * </ul>
+ *
+ * The builder supports three types of components: <code>byte[]</code>, <code>Text</code> and <code>CharSequence</code>.
+ *
+ * The builder is mutable and not thread safe.
+ *
+ * @see org.apache.accumulo.core.data.Key
+ * @since 1.8
+ */
+public class KeyBuilder {
+
+ public interface Build {
+
+ /**
+ * Build a {@link Key} from this builder. copyBytes defaults to true.
+ *
+ * @return
+ * the {@link Key} built from this builder
+ */
+ Key build();
+
+ /**
+ * Change the timestamp of the {@link Key} created.
+ *
+ * @param timestamp
+ * the timestamp to use for the {@link Key}
+ * @return this builder
+ */
+ Build timestamp(long timestamp);
+
+ /**
+ * Set the deleted marker of the {@link Key} to the parameter.
+ *
+ * @param deleted
+ * if the {@link Key} should be marked as deleted or not
+ * @return this builder
+ */
+ Build deleted(boolean deleted);
+ }
+
+ public interface RowStep extends Build {
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final Text row);
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
+ *
+ * @param row
+ * the row to use for the key
+ * @return this builder
+ */
+ ColumnFamilyStep row(final byte[] row);
+
+ /**
+ * Set the row of the {@link Key} that this builder will build to the parameter.
--- End diff --
Done
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on the issue:
https://github.com/apache/accumulo/pull/145
> I'm a bit torn about names without column
I understand. I don't think there is a good solution for both types of users. Personally, I tend to lean towards making it nice for the experienced users and having good docs for the new users. New users will either stop using it or become experienced users. When they transition from new to experienced I suspect they will appreciate the less verbose code.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #145: ACCUMULO-4376 add KeyBuilder
Posted by melrief <gi...@git.apache.org>.
Github user melrief commented on the issue:
https://github.com/apache/accumulo/pull/145
Nice, thanks everybody!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #145: ACCUMULO-4376 add KeyBuilder
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/accumulo/pull/145
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---