You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2022/06/29 13:47:20 UTC
[GitHub] [ignite-3] ibessonov commented on a diff in pull request #904: IGNITE-14931 Added error groups
ibessonov commented on code in PR #904:
URL: https://github.com/apache/ignite-3/pull/904#discussion_r909652363
##########
modules/core/src/main/java/org/apache/ignite/lang/ErrorGroup.java:
##########
@@ -0,0 +1,189 @@
+/*
+ * 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.ignite.lang;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Locale;
+import java.util.Optional;
+import java.util.Set;
+import java.util.UUID;
+
+/**
+ * This class represents a concept of error group. Error group defines a collection of errors that belong to a single semantic component.
+ * Each group can be identified by a name and an integer number that both must be unique across all error groups.
+ */
+public class ErrorGroup {
+ /** List of all registered error groups. */
+ private static final List<ErrorGroup> registeredGroups = new ArrayList<>();
+
+ /** Group name. */
+ private final String groupName;
+
+ /** Group code. */
+ private final int groupCode;
+
+ /** Contains error codes for this error group. */
+ private final Set<Integer> codes = new HashSet<>();
+
+ /**
+ * Creates a new error group with the specified name and corresponding code.
+ *
+ * @param groupName Group name.
+ * @param groupCode Group code.
+ */
+ private ErrorGroup(String groupName, int groupCode) {
+ this.groupName = groupName;
+ this.groupCode = groupCode;
+ }
+
+ /**
+ * Returns a name of this group.
+ *
+ * @return Group name.
+ */
+ public String name() {
+ return groupName;
+ }
+
+ /**
+ * Returns a code of this group.
+ *
+ * @return Group code.
+ */
+ public int code() {
+ return groupCode;
+ }
+
+ /**
+ * Registers a new error code within this error group.
+ *
+ * @param errorCode Error code to be registered.
+ * @return Full error code which includes group code and specific error code.
+ * @throws IllegalArgumentException If the given {@code errorCode} is already registered
+ * or {@code errorCode} is greater than 0xFFFF or less than or equal to 0.
+ */
+ public int registerErrorCode(int errorCode) {
+ if (errorCode < 0 || errorCode > 0xFFFF) {
+ throw new IllegalArgumentException("Error code should be greater than 0 and less than or equal to 0xFFFF");
+ }
+
+ if (codes.contains(errorCode)) {
+ throw new IllegalArgumentException("Error code already registered [errorCode=" + errorCode + ", group=" + name() + ']');
+ }
+
+ codes.add(errorCode);
+
+ return (code() << 16) | (errorCode & 0xFFFF);
+ }
+
+ /**
+ * Checks that the given {@code code} is registered for this error group.
+ *
+ * @param code Full error code to be tested.
+ * @return {@code true} If the given {@code code} is registered for this error group.
+ */
+ public boolean isRegistered(ErrorGroup group, int code) {
+ return group.codes.contains(code);
+ }
+
+ @Override
+ public String toString() {
+ return "ErrorGroup [name=" + name() + ", code=" + code() + ']';
+ }
+
+ /**
+ * Creates a new error group with the given {@code groupName} and {@code groupCode}.
+ *
+ * @param groupName Group name to be created.
+ * @param groupCode Group code to be created.
+ * @return New error group.
+ * @throws IllegalArgumentException If the specified name or group code already registered.
+ * or {@code groupCode} is greater than 0xFFFF or less than or equal to 0.
+ */
+ public static ErrorGroup newGroup(String groupName, int groupCode) {
Review Comment:
Method isn't synchronized in any way, it's unsafe
##########
modules/api/src/main/java/org/apache/ignite/lang/TableNotFoundException.java:
##########
@@ -27,6 +30,6 @@ public class TableNotFoundException extends IgniteException {
* @param name Table name.
*/
public TableNotFoundException(String name) {
- super(IgniteStringFormatter.format("Table does not exist [name={}]", name));
+ super(TABLE_ERR_GROUP.name(), TABLE_ALREADY_EXISTS_ERR, "Table does not exist [name=" + name + ']');
Review Comment:
Wrong error code
##########
modules/client-common/src/test/java/org/apache/ignite/internal/client/proto/ClientMessageDecoderTest.java:
##########
@@ -51,8 +51,8 @@ void testInvalidMagicThrowsException() {
var t = assertThrows(IgniteException.class, () -> decode(buf));
- assertEquals("Invalid magic header in thin client connection. Expected 'IGNI', but was 'BEEF'.",
- t.getMessage());
+ String expected = "Invalid magic header in thin client connection. Expected 'IGNI', but was 'BEEF'.";
+ assertTrue(t.getMessage().contains(expected), "Expected: " + expected + ", actual: " + t.getMessage());
Review Comment:
Mabe "assertThat(..., contains(...))" will produce better message automatically, can you check?
##########
modules/core/src/main/java/org/apache/ignite/lang/ErrorGroups.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.ignite.lang;
+
+/**
+ * Defines error groups and its errors.
+ */
+public class ErrorGroups {
+ /** Common error group. */
+ public static class Common {
+ /** Unknown error group. */
+ public static final ErrorGroup UNKNOWN_ERR_GROUP = ErrorGroup.newGroup("UNK", 1);
+
+ /** Unknown error. */
+ public static int UNKNOWN_ERR = UNKNOWN_ERR_GROUP.registerErrorCode(1);
+ }
+
+ /** Tables error group. */
+ public static class Table {
+ /** Table error group. */
+ public static final ErrorGroup TABLE_ERR_GROUP = ErrorGroup.newGroup("TBL", 3);
Review Comment:
Where's 2?
##########
modules/core/src/main/java/org/apache/ignite/lang/ErrorGroup.java:
##########
@@ -0,0 +1,189 @@
+/*
+ * 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.ignite.lang;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Locale;
+import java.util.Optional;
+import java.util.Set;
+import java.util.UUID;
+
+/**
+ * This class represents a concept of error group. Error group defines a collection of errors that belong to a single semantic component.
+ * Each group can be identified by a name and an integer number that both must be unique across all error groups.
+ */
+public class ErrorGroup {
+ /** List of all registered error groups. */
+ private static final List<ErrorGroup> registeredGroups = new ArrayList<>();
+
+ /** Group name. */
+ private final String groupName;
+
+ /** Group code. */
+ private final int groupCode;
+
+ /** Contains error codes for this error group. */
+ private final Set<Integer> codes = new HashSet<>();
+
+ /**
+ * Creates a new error group with the specified name and corresponding code.
+ *
+ * @param groupName Group name.
+ * @param groupCode Group code.
+ */
+ private ErrorGroup(String groupName, int groupCode) {
+ this.groupName = groupName;
+ this.groupCode = groupCode;
+ }
+
+ /**
+ * Returns a name of this group.
+ *
+ * @return Group name.
+ */
+ public String name() {
+ return groupName;
+ }
+
+ /**
+ * Returns a code of this group.
+ *
+ * @return Group code.
+ */
+ public int code() {
+ return groupCode;
+ }
+
+ /**
+ * Registers a new error code within this error group.
+ *
+ * @param errorCode Error code to be registered.
+ * @return Full error code which includes group code and specific error code.
+ * @throws IllegalArgumentException If the given {@code errorCode} is already registered
+ * or {@code errorCode} is greater than 0xFFFF or less than or equal to 0.
+ */
+ public int registerErrorCode(int errorCode) {
+ if (errorCode < 0 || errorCode > 0xFFFF) {
+ throw new IllegalArgumentException("Error code should be greater than 0 and less than or equal to 0xFFFF");
+ }
+
+ if (codes.contains(errorCode)) {
+ throw new IllegalArgumentException("Error code already registered [errorCode=" + errorCode + ", group=" + name() + ']');
+ }
+
+ codes.add(errorCode);
+
+ return (code() << 16) | (errorCode & 0xFFFF);
+ }
+
+ /**
+ * Checks that the given {@code code} is registered for this error group.
+ *
+ * @param code Full error code to be tested.
+ * @return {@code true} If the given {@code code} is registered for this error group.
+ */
+ public boolean isRegistered(ErrorGroup group, int code) {
+ return group.codes.contains(code);
+ }
+
+ @Override
+ public String toString() {
+ return "ErrorGroup [name=" + name() + ", code=" + code() + ']';
+ }
+
+ /**
+ * Creates a new error group with the given {@code groupName} and {@code groupCode}.
+ *
+ * @param groupName Group name to be created.
+ * @param groupCode Group code to be created.
+ * @return New error group.
+ * @throws IllegalArgumentException If the specified name or group code already registered.
+ * or {@code groupCode} is greater than 0xFFFF or less than or equal to 0.
+ */
+ public static ErrorGroup newGroup(String groupName, int groupCode) {
+ String grpName = groupName.toUpperCase(Locale.ENGLISH);
+
+ Optional<ErrorGroup> grp = registeredGroups.stream().filter(g -> g.name().equalsIgnoreCase(grpName)).findFirst();
Review Comment:
Why "equalsIgnoreCase"? It's guaranteed to be in upper case, as far as I see
##########
modules/network/src/test/java/org/apache/ignite/internal/network/serialization/marshal/BestEffortInstantiationTest.java:
##########
@@ -92,6 +93,6 @@ void whenOnlySecondDelegateSupportsThenItIsUsedForInstantiation() throws Excepti
@Test
void whenNoDelegateSupportsThenInstantiationFails() {
InstantiationException ex = assertThrows(InstantiationException.class, () -> instantiation.newInstance(Object.class));
- assertThat(ex.getMessage(), is("No delegate supports " + Object.class));
+ assertThat(ex.getMessage(), containsString("No delegate supports " + Object.class));
Review Comment:
Yes, this is what I meant
##########
modules/core/src/main/java/org/apache/ignite/lang/ErrorGroup.java:
##########
@@ -0,0 +1,189 @@
+/*
+ * 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.ignite.lang;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Locale;
+import java.util.Optional;
+import java.util.Set;
+import java.util.UUID;
+
+/**
+ * This class represents a concept of error group. Error group defines a collection of errors that belong to a single semantic component.
+ * Each group can be identified by a name and an integer number that both must be unique across all error groups.
+ */
+public class ErrorGroup {
+ /** List of all registered error groups. */
+ private static final List<ErrorGroup> registeredGroups = new ArrayList<>();
+
+ /** Group name. */
+ private final String groupName;
+
+ /** Group code. */
+ private final int groupCode;
+
+ /** Contains error codes for this error group. */
+ private final Set<Integer> codes = new HashSet<>();
+
+ /**
+ * Creates a new error group with the specified name and corresponding code.
+ *
+ * @param groupName Group name.
+ * @param groupCode Group code.
+ */
+ private ErrorGroup(String groupName, int groupCode) {
+ this.groupName = groupName;
+ this.groupCode = groupCode;
+ }
+
+ /**
+ * Returns a name of this group.
+ *
+ * @return Group name.
+ */
+ public String name() {
+ return groupName;
+ }
+
+ /**
+ * Returns a code of this group.
+ *
+ * @return Group code.
+ */
+ public int code() {
+ return groupCode;
+ }
+
+ /**
+ * Registers a new error code within this error group.
+ *
+ * @param errorCode Error code to be registered.
+ * @return Full error code which includes group code and specific error code.
+ * @throws IllegalArgumentException If the given {@code errorCode} is already registered
+ * or {@code errorCode} is greater than 0xFFFF or less than or equal to 0.
+ */
+ public int registerErrorCode(int errorCode) {
+ if (errorCode < 0 || errorCode > 0xFFFF) {
+ throw new IllegalArgumentException("Error code should be greater than 0 and less than or equal to 0xFFFF");
Review Comment:
Message contradicts the condition, I think it must be "<= 0" instead of strict "<"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org