Address feedback

concordium-sdk/src/main/java/com/concordium/sdk/crypto/CryptoJniNative.java

@@ -321,11 +321,28 @@ public static String getVerifiableCredentialBackupEncryptionKey(String seedAsHex
 
     /**
      * Check that a request is acceptable
+     *
      * @param input {@link Request} serialized as JSON.
-     * @return JSON representing {@link StringResult}. If successful the field 'result' is empty, 
-     * but if not acceptable the reason will be contained in the error.
+     * @return JSON representing {@link VoidResult}. If successful the field
+     *         'result' is empty,
+     *         but if not acceptable the reason will be contained in the error.
      */
     public static native String isAcceptableRequest(String input);
 
-    public static native String isAcceptableAtomicStatement(String input, String rangeTags, String setTags, AcceptableRequest.RawAttributeCheck check);
+    /**
+     * Check that an atomic is acceptable, with the given restrictions
+     *
+     * @param input     {@link Request} serialized as JSON.
+     * @param rangeTags the list of tags, which may be used for range statements,
+     *                  serialized as JSON.
+     * @param setTags   the list of tags, which may be used for membership
+     *                  statements, serialized as JSON.
+     * @param check     provides the function to check whether the statement value
+     *                  is acceptable.
+     * @return JSON representing {@link VoidResult}. If successful the field
+     *         'result' is empty,
+     *         but if not acceptable the reason will be contained in the error.
+     */
+    public static native String isAcceptableAtomicStatement(String input, String rangeTags, String setTags,
+            AcceptableRequest.RawAttributeCheck check);
 }

concordium-sdk/src/main/java/com/concordium/sdk/crypto/wallet/ErrorResult.java

@@ -0,0 +1,16 @@
+package com.concordium.sdk.crypto.wallet;
+
+import com.concordium.sdk.exceptions.JNIError;
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+public class ErrorResult extends StringResult {
+
+    @JsonCreator
+    ErrorResult(
+            @JsonProperty("Ok") String ok,
+            @JsonProperty("Err") JNIError err
+    ) {
+        super(ok, err);
+    }
+}

concordium-sdk/src/main/java/com/concordium/sdk/crypto/wallet/web3Id/AcceptableRequest.java

@@ -3,12 +3,12 @@
 import java.util.List;
 import com.concordium.sdk.crypto.CryptoJniNative;
 import com.concordium.sdk.crypto.NativeResolver;
+import com.concordium.sdk.crypto.wallet.ErrorResult;
 import com.concordium.sdk.crypto.wallet.StringResult;
 import com.concordium.sdk.crypto.wallet.web3Id.Statement.AtomicStatement;
 import com.concordium.sdk.exceptions.CryptoJniException;
 import com.concordium.sdk.serializing.JsonMapper;
 import com.fasterxml.jackson.core.JsonProcessingException;
-import com.fasterxml.jackson.databind.JsonMappingException;
 
 public class AcceptableRequest {
     // Static block to load native library.
@@ -17,33 +17,50 @@ public class AcceptableRequest {
     }
 
     /**
-     * Check that a request is acceptable
+     * Check that a request is acceptable:
+     * That range statements are only on the attributes "dob", "idiDocIssuedAt" or
+     * "idDocExpiredAt"
+     * That membership statement are only on the attributes "Country of residence",
+     * "Nationality", "IdDocType" or "IdDocIssuer"
+     * That attribute tags are not reused within a credentialStatement
      *
      * @param input the request that should be checked
-     * @return a Presentation serialized as JSON
+     * @throws NotAcceptableException if the request is not acceptable
+     * @Throws CryptoJniException if there an error occurs duting the check
+     * @returns if the request is acceptable, otherwise throws a
+     *          NotAcceptableException
      */
-    public static void acceptableRequest(QualifiedRequest request) {
-        StringResult result = null;
+    public static void acceptableRequest(QualifiedRequest request) throws NotAcceptableException {
+        ErrorResult result = null;
         try {
             String jsonStr = CryptoJniNative.isAcceptableRequest(JsonMapper.INSTANCE.writeValueAsString(request));
-            result = JsonMapper.INSTANCE.readValue(jsonStr, StringResult.class);
+            result = JsonMapper.INSTANCE.readValue(jsonStr, ErrorResult.class);
         } catch (JsonProcessingException e) {
             throw new RuntimeException(e);
         }
-
         if (!result.isSuccess()) {
             throw CryptoJniException.from(result.getErr());
         }
+        if (result.getResult() != null) {
+            throw new NotAcceptableException(result.getResult());
+        }
     }
 
     /**
-     * Check that a request is acceptable
+     * Check that a statement is acceptable
      *
-     * @param input the request that should be checked
-     * @return a Presentation serialized as JSON
+     * @param statement      the statement that should be checked
+     * @param rangeTag       the attribute tags that may be used for range
+     *                       statements
+     * @param setTags        the attribute tags that may be used for set statements
+     * @param attributeCheck custom check on the value of the statement.
+     * @throws NotAcceptableException if the request is not acceptable
+     * @Throws CryptoJniException if there an error occurs duting the check
+     * @returns if the request is acceptable, otherwise throws a
+     *          NotAcceptableException
      */
     public static void acceptableAtomicStatement(AtomicStatement statement, List<String> rangeTags,
-            List<String> setTags, AttributeCheck attributeCheck) {
+            List<String> setTags, AttributeCheck attributeCheck) throws NotAcceptableException {
         StringResult result = null;
         try {
 
@@ -56,10 +73,12 @@ public static void acceptableAtomicStatement(AtomicStatement statement, List<Str
         } catch (JsonProcessingException e) {
             throw new RuntimeException(e);
         }
-
         if (!result.isSuccess()) {
             throw CryptoJniException.from(result.getErr());
         }
+        if (result.getResult() != null) {
+            throw new NotAcceptableException(result.getResult());
+        }
     }
 
     public static class RawAttributeCheck {
@@ -69,8 +88,14 @@ public RawAttributeCheck(AttributeCheck checker) {
             this.checker = checker;
         }
 
-        public String check_attribute(String tag, String value) throws JsonMappingException, JsonProcessingException {
-            return checker.checkAttribute(tag, JsonMapper.INSTANCE.readValue(value, CredentialAttribute.class));
+        public void check_attribute(String tag, String value) throws Exception {
+            checker.checkAttribute(tag, JsonMapper.INSTANCE.readValue(value, CredentialAttribute.class));
+        }
+    }
+
+    public static class NotAcceptableException extends Exception {
+        public NotAcceptableException(String errorMessage) {
+            super(errorMessage);
         }
     }
 }

concordium-sdk/src/main/java/com/concordium/sdk/crypto/wallet/web3Id/AttributeCheck.java

@@ -1,5 +1,12 @@
 package com.concordium.sdk.crypto.wallet.web3Id;
 
+
 public interface AttributeCheck {
-    public String checkAttribute(String tag, CredentialAttribute value);
+    /**
+     * This check should throw an exception if the given attribute value is not valid.
+     * @param tag the attribute tag for the statement
+     * @param value the attribute value used in the statement
+     * @throws Exception to indicate the attribute is not valid.
+    */
+    public void checkAttribute(String tag, CredentialAttribute value) throws Exception;
 }

concordium-sdk/src/test/java/com/concordium/sdk/crypto/wallet/web3Id/RequestStatementTest.java

@@ -13,6 +13,7 @@
 import com.concordium.sdk.crypto.wallet.FileHelpers;
 import com.concordium.sdk.crypto.wallet.Network;
 import com.concordium.sdk.crypto.wallet.identityobject.IdentityObject;
+import com.concordium.sdk.crypto.wallet.web3Id.AcceptableRequest.NotAcceptableException;
 import com.concordium.sdk.crypto.wallet.web3Id.CredentialAttribute.CredentialAttributeType;
 import com.concordium.sdk.crypto.wallet.web3Id.Statement.AtomicStatement;
 import com.concordium.sdk.crypto.wallet.web3Id.Statement.QualifiedRequestStatement;
@@ -22,7 +23,6 @@
 import com.concordium.sdk.serializing.JsonMapper;
 import com.concordium.sdk.transactions.CredentialRegistrationId;
 import com.concordium.sdk.types.ContractAddress;
-import com.fasterxml.jackson.core.JsonProcessingException;
 
 public class RequestStatementTest {
 
@@ -62,19 +62,42 @@ public void testIsAcceptableRequest() throws Exception {
     }
 
     @Test
-    public void testIsAcceptableAtomicStatement() {
+    public void testIsAcceptableAtomicStatement() throws NotAcceptableException {
         AtomicStatement statement = RangeStatement.builder().attributeTag("dob")
                 .lower(CredentialAttribute.builder().type(CredentialAttributeType.STRING).value("20140112").build())
                 .upper(CredentialAttribute.builder().type(CredentialAttributeType.STRING).value("20150112").build())
                 .build();
 
         AcceptableRequest.acceptableAtomicStatement(statement, Collections.singletonList("dob"), Collections.emptyList(), new AttributeCheck() {
             @Override
-            public String checkAttribute(String tag, CredentialAttribute value) {
-                return "Test";
-            }});
+            public void checkAttribute(String tag, CredentialAttribute value) {}});
+    }
+
+    @Test(expected = AcceptableRequest.NotAcceptableException.class)
+    public void testIllegalTagStatementThrowsNotAcceptable() throws NotAcceptableException {
+        AtomicStatement statement = RangeStatement.builder().attributeTag("dob")
+                .lower(CredentialAttribute.builder().type(CredentialAttributeType.STRING).value("20140112").build())
+                .upper(CredentialAttribute.builder().type(CredentialAttributeType.STRING).value("20150112").build())
+                .build();
+
+        AcceptableRequest.acceptableAtomicStatement(statement, Collections.emptyList(), Collections.emptyList(), new AttributeCheck() {
+            @Override
+            public void checkAttribute(String tag, CredentialAttribute value) {}});
     }
 
+    @Test(expected = AcceptableRequest.NotAcceptableException.class)
+    public void testAttributeCheckThrowingErrorReturnsNotAcceptable() throws NotAcceptableException {
+        AtomicStatement statement = RangeStatement.builder().attributeTag("dob")
+                .lower(CredentialAttribute.builder().type(CredentialAttributeType.STRING).value("20140112").build())
+                .upper(CredentialAttribute.builder().type(CredentialAttributeType.STRING).value("20150112").build())
+                .build();
+
+        AcceptableRequest.acceptableAtomicStatement(statement, Collections.emptyList(), Collections.emptyList(), new AttributeCheck() {
+            @Override
+            public void checkAttribute(String tag, CredentialAttribute value) throws Exception {
+                throw new Exception("Not okay");
+            }});
+    }
     @Test
     public void testCanQualifyStatement() throws Exception {
         // Arrange
@@ -85,6 +108,7 @@ public void testCanQualifyStatement() throws Exception {
                 "8a3a87f3f38a7a507d1e85dc02a92b8bcaa859f5cf56accb3c1bc7c40e1789b4933875a38dd4c0646ca3e940a02c42d8");
         ContractAddress contractAddress = ContractAddress.from(1232, 3);
         ED25519PublicKey publicKey = ED25519PublicKey
+
                 .from("16afdb3cb3568b5ad8f9a0fa3c741b065642de8c53e58f7920bf449e63ff2bf9");
 
         // Act

crypto-jni/src/lib.rs

@@ -1338,8 +1338,11 @@ impl<T> From<wallet_library::statement::RequestCheckError> for CryptoJniResult<T
     }
 }
 
+/// Encodes a potential error message inside the result, that differentiates from the JniError
+type ErrorResult = CryptoJniResult<Option<String>>;
+
 /// The JNI wrapper for checking that a request is acceptable.
-/// * `input` - the JSON string of [`wallet_library::proofs::RequestStatement`]
+/// * `input` - the JSON string of [`concordium_base::web3id::Request`]
 #[no_mangle]
 #[allow(non_snake_case)]
 pub extern "system" fn Java_com_concordium_sdk_crypto_CryptoJniNative_isAcceptableRequest(
@@ -1349,66 +1352,69 @@ pub extern "system" fn Java_com_concordium_sdk_crypto_CryptoJniNative_isAcceptab
 ) -> jstring {
     let input_string = match get_string(env, input) {
         Ok(s) => s,
-        Err(err) => return StringResult::Err(err).to_jstring(&env),
+        Err(err) => return ErrorResult::Err(err).to_jstring(&env),
     };
 
     let request: Request<constants::ArCurve, AttributeKind> =
         match serde_json::from_str(&input_string) {
             Ok(req) => req,
-            Err(err) => return StringResult::from(err).to_jstring(&env),
+            Err(err) => return ErrorResult::from(err).to_jstring(&env),
         };
 
     match request.acceptable_request(&wallet_library::default_wallet_config::default_wallet_config()) {
         Ok(r) => r,
-        Err(err) => return StringResult::from(err).to_jstring(&env),
+        Err(err) => return ErrorResult::Ok(Some(err.to_string())).to_jstring(&env),
     };
 
-    CryptoJniResult::Ok(()).to_jstring(&env)
+    ErrorResult::Ok(None).to_jstring(&env)
 }
 
-/// The JNI wrapper for checking that a request is acceptable.
-/// * `input` - the JSON string of [`wallet_library::proofs::RequestStatement`]
+/// The JNI wrapper for checking that an atomic statement is acceptable, according to the given rules.
+/// * `statement_input` - the JSON string of [`concordium_base::id::id_proof_types::AtomicStatement`]
+/// * `range_tags_input` - the JSON string for a list of strings, that represent attribute tags allowed for range statements
+/// * `set_tags_input` - the JSON string for a list of strings, that represent attribute tags allowed for membership statements
+/// * `attribute_check` - an object implementing the "check_attribute" function, to do a custom check for the attribute
 #[no_mangle]
 #[allow(non_snake_case)]
 pub extern "system" fn Java_com_concordium_sdk_crypto_CryptoJniNative_isAcceptableAtomicStatement<
     'a,
 >(
     env: JNIEnv<'a>,
     _: JClass,
-    request_input: JString,
+    statement_input: JString,
     range_tags_input: JString,
     set_tags_input: JString,
     attribute_check: JObject<'a>,
 ) -> jstring {
-    let input_string = match get_string(env, request_input) {
+    let input_string = match get_string(env, statement_input) {
         Ok(s) => s,
-        Err(err) => return StringResult::Err(err).to_jstring(&env),
+        Err(err) => return ErrorResult::Err(err).to_jstring(&env),
     };
 
-    let request: AtomicStatement<constants::ArCurve, String, Web3IdAttribute> =
+    let statement: AtomicStatement<constants::ArCurve, String, Web3IdAttribute> =
         match serde_json::from_str(&input_string) {
             Ok(req) => req,
-            Err(err) => return StringResult::from(err).to_jstring(&env),
+            Err(err) => return ErrorResult::from(err).to_jstring(&env),
         };
 
     let input_string = match get_string(env, set_tags_input) {
         Ok(s) => s,
-        Err(err) => return StringResult::Err(err).to_jstring(&env),
+        Err(err) => return ErrorResult::Err(err).to_jstring(&env),
     };
 
     let set_tags: HashSet<String> = match serde_json::from_str(&input_string) {
         Ok(req) => req,
-        Err(err) => return StringResult::from(err).to_jstring(&env),
+        Err(err) => return ErrorResult::from(err).to_jstring(&env),
     };
 
     let input_string = match get_string(env, range_tags_input) {
         Ok(s) => s,
-        Err(err) => return StringResult::Err(err).to_jstring(&env),
+        Err(err) => return ErrorResult::Err(err).to_jstring(&env),
     };
 
     let range_tags: HashSet<String> = match serde_json::from_str(&input_string) {
         Ok(req) => req,
-        Err(err) => return StringResult::from(err).to_jstring(&env),
+        Err(err) => return ErrorResult::from(err).to_jstring(&env),
     };
 
     let check = |tag: &String, attribute: &Web3IdAttribute| {
@@ -1442,7 +1448,7 @@ pub extern "system" fn Java_com_concordium_sdk_crypto_CryptoJniNative_isAcceptab
         match env.call_method(
             attribute_check,
             "check_attribute",
-            "(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;",
+            "(Ljava/lang/String;Ljava/lang/String;)V",
             args,
         ) {
             Ok(_) => Ok(()),
@@ -1457,10 +1463,10 @@ pub extern "system" fn Java_com_concordium_sdk_crypto_CryptoJniNative_isAcceptab
         _marker: std::marker::PhantomData,
     };
 
-    match request.acceptable_atomic_statement(Some(rules).as_ref()) {
+    match statement.acceptable_atomic_statement(Some(rules).as_ref()) {
         Ok(r) => r,
-        Err(err) => return StringResult::from(err).to_jstring(&env),
+        Err(err) => return ErrorResult::Ok(Some(err.to_string())).to_jstring(&env),
     };
 
-    CryptoJniResult::Ok(()).to_jstring(&env)
+    ErrorResult::Ok(None).to_jstring(&env)
 }