You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2021/07/23 11:44:11 UTC

[GitHub] [james-project] vttranlina opened a new pull request #551: [WIP] [JAMES-3544] UploadRepository contract & InMemory implementationtemp

vttranlina opened a new pull request #551:
URL: https://github.com/apache/james-project/pull/551


   Ref: https://issues.apache.org/jira/browse/JAMES-3544


-- 
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@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] vttranlina commented on a change in pull request #551: [WIP] [JAMES-3544] UploadRepository contract & InMemory implementationtemp

Posted by GitBox <gi...@apache.org>.
vttranlina commented on a change in pull request #551:
URL: https://github.com/apache/james-project/pull/551#discussion_r676279468



##########
File path: server/data/data-jmap/src/main/java/org/apache/james/jmap/api/model/Upload.scala
##########
@@ -0,0 +1,67 @@
+/****************************************************************
+ * 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.james.jmap.api.model
+
+import com.google.common.hash.Hashing
+import eu.timepit.refined.api.Refined
+import eu.timepit.refined.auto._
+import eu.timepit.refined.numeric.NonNegative
+import eu.timepit.refined.refineV
+import org.apache.james.jmap.api.model.Upload.Size
+import org.apache.james.mailbox.model.ContentType
+import org.slf4j.{Logger, LoggerFactory}
+
+import java.io.{ByteArrayInputStream, InputStream}
+
+object Upload {
+  private val logger: Logger = LoggerFactory.getLogger(classOf[Upload])
+  type Size = Long Refined NonNegative
+  val Zero: Size = 0L
+
+  def sanitizeSize(value: Long): Size = {
+    val size: Either[String, Size] = refineV[NonNegative](value)
+    size.fold(e => {
+      logger.error(s"Encountered an invalid Upload size: $e")
+      Zero
+    },
+      refinedValue => refinedValue)
+  }
+
+  def from(metaData: UploadMetaData, content: Array[Byte]): Upload =
+    Upload(uploadId = metaData.getUploadId,
+      size = Upload.sanitizeSize(metaData.getSize),

Review comment:
       `metaData.getSize` is not set by the client, It is set by the server when store upload (`content.length`). This purpose `def` is to convert data in the database to client response.




-- 
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@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] vttranlina commented on pull request #551: [JAMES-3544] UploadRepository contract & InMemory implementationtemp

Posted by GitBox <gi...@apache.org>.
vttranlina commented on pull request #551:
URL: https://github.com/apache/james-project/pull/551#issuecomment-888751046


   > Can you have a look @vttranlina ?
   
   yes, few moments


-- 
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@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] Arsnael commented on pull request #551: [JAMES-3544] UploadRepository contract & InMemory implementationtemp

Posted by GitBox <gi...@apache.org>.
Arsnael commented on pull request #551:
URL: https://github.com/apache/james-project/pull/551#issuecomment-888869558


   Can you squash your fixups please? :)


-- 
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@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a change in pull request #551: [WIP] [JAMES-3544] UploadRepository contract & InMemory implementationtemp

Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #551:
URL: https://github.com/apache/james-project/pull/551#discussion_r675989183



##########
File path: server/data/data-jmap/src/main/java/org/apache/james/jmap/api/model/Upload.scala
##########
@@ -0,0 +1,67 @@
+/****************************************************************
+ * 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.james.jmap.api.model
+
+import com.google.common.hash.Hashing
+import eu.timepit.refined.api.Refined
+import eu.timepit.refined.auto._
+import eu.timepit.refined.numeric.NonNegative
+import eu.timepit.refined.refineV
+import org.apache.james.jmap.api.model.Upload.Size
+import org.apache.james.mailbox.model.ContentType
+import org.slf4j.{Logger, LoggerFactory}
+
+import java.io.{ByteArrayInputStream, InputStream}
+
+object Upload {
+  private val logger: Logger = LoggerFactory.getLogger(classOf[Upload])
+  type Size = Long Refined NonNegative
+  val Zero: Size = 0L
+
+  def sanitizeSize(value: Long): Size = {
+    val size: Either[String, Size] = refineV[NonNegative](value)
+    size.fold(e => {
+      logger.error(s"Encountered an invalid Upload size: $e")
+      Zero
+    },
+      refinedValue => refinedValue)
+  }
+
+  def from(metaData: UploadMetaData, content: Array[Byte]): Upload =
+    Upload(uploadId = metaData.getUploadId,
+      size = Upload.sanitizeSize(metaData.getSize),
+      contentType = metaData.getContentType,
+      content = new ByteArrayInputStream(content))
+}
+
+case class Upload(uploadId: UploadId,
+                  size: Size,
+                  contentType: ContentType,
+                  content: InputStream)
+
+object UploadHashContent {
+  def from(content: Array[Byte]): UploadHashContent =
+    UploadHashContent(Hashing.sha256.hashBytes(content).toString)
+
+}
+
+case class UploadHashContent(value: String)

Review comment:
       What is this for?

##########
File path: server/data/data-jmap/src/main/java/org/apache/james/jmap/api/model/UploadId.java
##########
@@ -0,0 +1,81 @@
+/****************************************************************
+ * 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.james.jmap.api.model;
+
+import org.apache.commons.text.RandomStringGenerator;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Objects;
+import com.google.common.base.Preconditions;
+import com.google.common.hash.Hashing;
+
+public class UploadId {
+    public static final RandomStringGenerator RANDOM_STRING_GENERATOR = new RandomStringGenerator.Builder()
+        .withinRange('a', 'z')
+        .build();
+
+    public static UploadId random() {
+        return new UploadId(RANDOM_STRING_GENERATOR.generate(20));
+    }
+
+    public static UploadId from(String id) {
+        Preconditions.checkNotNull(id);
+        Preconditions.checkArgument(!id.isEmpty());
+        return new UploadId(id);
+    }
+
+    public static UploadId from(byte[] bytes) {
+        Preconditions.checkNotNull(bytes);
+        return new UploadId(Hashing.sha256().hashBytes(bytes).toString());

Review comment:
       We should remove this: hashing and dedup is a blob store concern. We should only use the `random()` method in our generation logic.

##########
File path: server/data/data-jmap/src/main/java/org/apache/james/jmap/api/model/Upload.scala
##########
@@ -0,0 +1,67 @@
+/****************************************************************
+ * 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.james.jmap.api.model
+
+import com.google.common.hash.Hashing
+import eu.timepit.refined.api.Refined
+import eu.timepit.refined.auto._
+import eu.timepit.refined.numeric.NonNegative
+import eu.timepit.refined.refineV
+import org.apache.james.jmap.api.model.Upload.Size
+import org.apache.james.mailbox.model.ContentType
+import org.slf4j.{Logger, LoggerFactory}
+
+import java.io.{ByteArrayInputStream, InputStream}
+
+object Upload {
+  private val logger: Logger = LoggerFactory.getLogger(classOf[Upload])
+  type Size = Long Refined NonNegative

Review comment:
       I think we already have a scala type for Size somewhere.
   
   Time to factorize?

##########
File path: server/data/data-jmap/src/main/java/org/apache/james/jmap/api/exception/UploadNotFoundException.java
##########
@@ -0,0 +1,44 @@
+/****************************************************************
+ * 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.james.jmap.api.exception;

Review comment:
       Why do we mix java and scala in the same package?
   
   Can't we just write scala here?

##########
File path: server/data/data-jmap/src/main/java/org/apache/james/jmap/api/model/Upload.scala
##########
@@ -0,0 +1,67 @@
+/****************************************************************
+ * 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.james.jmap.api.model
+
+import com.google.common.hash.Hashing
+import eu.timepit.refined.api.Refined
+import eu.timepit.refined.auto._
+import eu.timepit.refined.numeric.NonNegative
+import eu.timepit.refined.refineV
+import org.apache.james.jmap.api.model.Upload.Size
+import org.apache.james.mailbox.model.ContentType
+import org.slf4j.{Logger, LoggerFactory}
+
+import java.io.{ByteArrayInputStream, InputStream}
+
+object Upload {
+  private val logger: Logger = LoggerFactory.getLogger(classOf[Upload])
+  type Size = Long Refined NonNegative
+  val Zero: Size = 0L
+
+  def sanitizeSize(value: Long): Size = {
+    val size: Either[String, Size] = refineV[NonNegative](value)
+    size.fold(e => {
+      logger.error(s"Encountered an invalid Upload size: $e")
+      Zero
+    },
+      refinedValue => refinedValue)
+  }
+
+  def from(metaData: UploadMetaData, content: Array[Byte]): Upload =
+    Upload(uploadId = metaData.getUploadId,
+      size = Upload.sanitizeSize(metaData.getSize),
+      contentType = metaData.getContentType,
+      content = new ByteArrayInputStream(content))
+}
+
+case class Upload(uploadId: UploadId,
+                  size: Size,
+                  contentType: ContentType,
+                  content: InputStream)

Review comment:
       I've seen disasters with passing statefull parameters to classes (fun fact it was for upload).
   
   I'd prefer a supplier of stream, it makes also the responsibility of `who closes the stream` simpler... The one calling the supplier ;-)

##########
File path: server/data/data-jmap/src/main/java/org/apache/james/jmap/memory/upload/InMemoryUploadRepository.java
##########
@@ -0,0 +1,149 @@
+/****************************************************************
+ * 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.james.jmap.memory.upload;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.UncheckedIOException;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+
+import javax.inject.Inject;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.james.blob.api.BlobId;
+import org.apache.james.blob.api.BlobStore;
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.core.Username;
+import org.apache.james.jmap.api.exception.UploadNotFoundException;
+import org.apache.james.jmap.api.model.Upload;
+import org.apache.james.jmap.api.model.UploadId;
+import org.apache.james.jmap.api.model.UploadMetaData;
+import org.apache.james.jmap.api.upload.UploadRepository;
+import org.apache.james.mailbox.model.ContentType;
+import org.reactivestreams.Publisher;
+
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Mono;
+
+public class InMemoryUploadRepository implements UploadRepository {
+
+    private static final Map<UploadId, ImmutablePair<Username, BlobId>> uploadStore = new HashMap<>();
+    private static final Map<UploadId, UploadMetaData> uploadMetaDataStore = new HashMap<>();

Review comment:
       Why do we need two maps? Can't we get one?

##########
File path: server/data/data-jmap/src/main/java/org/apache/james/jmap/memory/upload/InMemoryUploadRepository.java
##########
@@ -0,0 +1,149 @@
+/****************************************************************
+ * 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.james.jmap.memory.upload;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.UncheckedIOException;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+
+import javax.inject.Inject;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.james.blob.api.BlobId;
+import org.apache.james.blob.api.BlobStore;
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.core.Username;
+import org.apache.james.jmap.api.exception.UploadNotFoundException;
+import org.apache.james.jmap.api.model.Upload;
+import org.apache.james.jmap.api.model.UploadId;
+import org.apache.james.jmap.api.model.UploadMetaData;
+import org.apache.james.jmap.api.upload.UploadRepository;
+import org.apache.james.mailbox.model.ContentType;
+import org.reactivestreams.Publisher;
+
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Mono;
+
+public class InMemoryUploadRepository implements UploadRepository {
+
+    private static final Map<UploadId, ImmutablePair<Username, BlobId>> uploadStore = new HashMap<>();
+    private static final Map<UploadId, UploadMetaData> uploadMetaDataStore = new HashMap<>();
+
+    static class UploadContext {
+        private final UploadId uploadId;
+        private final boolean exists;
+        private final byte[] data;
+
+        private UploadContext(UploadId uploadId,
+                              boolean exists,
+                              byte[] data) {
+            this.uploadId = uploadId;
+            this.exists = exists;
+            this.data = data;
+        }
+    }
+
+
+    private final BlobStore blobStore;
+    private final BucketName bucketName;
+
+    @Inject
+    public InMemoryUploadRepository(BlobStore blobStore) {
+        this.blobStore = blobStore;
+        this.bucketName = blobStore.getDefaultBucketName();
+    }
+
+    @Override
+    public Publisher<UploadId> upload(InputStream data, ContentType contentType, Username user) {
+        return Mono.just(checkUploadExits(data, user))
+            .flatMap(uploadContext -> {
+                if (!uploadContext.exists) {
+                    return store(uploadContext.data, uploadContext.uploadId, contentType, user);
+                } else {
+                    return Mono.just(uploadContext.uploadId);
+                }
+            });
+    }
+
+    @Override
+    public Publisher<Upload> retrieve(UploadId id, Username user) {
+        Preconditions.checkNotNull(id);
+        Preconditions.checkNotNull(user);
+
+        return retrieveBlobId(id, user)
+            .flatMap(blobId -> retrieveUpload(id, blobId));
+    }
+
+    private Mono<BlobId> retrieveBlobId(UploadId id, Username user) {
+        return Mono.justOrEmpty(uploadStore.get(id))
+            .filter(pair -> userHasAccessToUpload(user, pair.left))
+            .map(userAndBlobId -> userAndBlobId.right)
+            .switchIfEmpty(Mono.error(() -> new UploadNotFoundException(id)));
+    }
+
+    private Mono<Upload> retrieveUpload(UploadId id, BlobId blobId) {
+        return Mono.zip(Mono.from(blobStore.readBytes(bucketName, blobId)),
+            Mono.just(uploadMetaDataStore.get(id)))
+            .map(bytesAndMetadata -> Upload.from(bytesAndMetadata.getT2(), bytesAndMetadata.getT1()));
+    }
+
+    private boolean userHasAccessToUpload(Username userRequest, Username owner) {
+        return userRequest.equals(owner);
+    }
+
+    private UploadContext checkUploadExits(InputStream data, Username username) {
+        byte[] dataAsBytes = toByteArray(data);
+        UploadId uploadId = UploadId.from(dataAsBytes);
+        boolean isExists = Optional.ofNullable(uploadStore.get(uploadId))
+            .filter(pair -> !userHasAccessToUpload(username, pair.left))
+            .isEmpty();
+        return new UploadContext(uploadId, isExists, dataAsBytes);
+    }
+
+    private Mono<UploadId> store(byte[] data, UploadId uploadId, ContentType contentType, Username username) {
+        return Mono.from(blobStore.save(bucketName, data, BlobStore.StoragePolicy.LOW_COST))
+            .map(blobId -> {
+                storeMapping(blobId, username, UploadMetaData.builder()
+                    .setContentType(contentType)
+                    .setUploadId(uploadId)
+                    .setSize(data.length)
+                    .build());
+                return uploadId;
+            });
+    }
+
+    private void storeMapping(BlobId blobId, Username username, UploadMetaData metaData) {

Review comment:
       Let's find a better name... (`storeMapping`)

##########
File path: server/data/data-jmap/src/main/java/org/apache/james/jmap/api/model/UploadMetaData.java
##########
@@ -0,0 +1,114 @@
+/****************************************************************
+ * 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.james.jmap.api.model;
+
+import org.apache.james.mailbox.model.ContentType;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Objects;
+import com.google.common.base.Preconditions;
+
+public class UploadMetaData {

Review comment:
       scala to reduce the boiler plate?

##########
File path: server/data/data-jmap/src/main/java/org/apache/james/jmap/memory/upload/InMemoryUploadRepository.java
##########
@@ -0,0 +1,149 @@
+/****************************************************************
+ * 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.james.jmap.memory.upload;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.UncheckedIOException;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+
+import javax.inject.Inject;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.james.blob.api.BlobId;
+import org.apache.james.blob.api.BlobStore;
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.core.Username;
+import org.apache.james.jmap.api.exception.UploadNotFoundException;
+import org.apache.james.jmap.api.model.Upload;
+import org.apache.james.jmap.api.model.UploadId;
+import org.apache.james.jmap.api.model.UploadMetaData;
+import org.apache.james.jmap.api.upload.UploadRepository;
+import org.apache.james.mailbox.model.ContentType;
+import org.reactivestreams.Publisher;
+
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Mono;
+
+public class InMemoryUploadRepository implements UploadRepository {
+
+    private static final Map<UploadId, ImmutablePair<Username, BlobId>> uploadStore = new HashMap<>();
+    private static final Map<UploadId, UploadMetaData> uploadMetaDataStore = new HashMap<>();
+
+    static class UploadContext {
+        private final UploadId uploadId;
+        private final boolean exists;
+        private final byte[] data;
+
+        private UploadContext(UploadId uploadId,
+                              boolean exists,
+                              byte[] data) {
+            this.uploadId = uploadId;
+            this.exists = exists;
+            this.data = data;
+        }
+    }
+
+
+    private final BlobStore blobStore;
+    private final BucketName bucketName;
+
+    @Inject
+    public InMemoryUploadRepository(BlobStore blobStore) {
+        this.blobStore = blobStore;
+        this.bucketName = blobStore.getDefaultBucketName();
+    }
+
+    @Override
+    public Publisher<UploadId> upload(InputStream data, ContentType contentType, Username user) {
+        return Mono.just(checkUploadExits(data, user))
+            .flatMap(uploadContext -> {
+                if (!uploadContext.exists) {
+                    return store(uploadContext.data, uploadContext.uploadId, contentType, user);
+                } else {
+                    return Mono.just(uploadContext.uploadId);
+                }
+            });
+    }
+
+    @Override
+    public Publisher<Upload> retrieve(UploadId id, Username user) {
+        Preconditions.checkNotNull(id);
+        Preconditions.checkNotNull(user);
+
+        return retrieveBlobId(id, user)
+            .flatMap(blobId -> retrieveUpload(id, blobId));
+    }
+
+    private Mono<BlobId> retrieveBlobId(UploadId id, Username user) {
+        return Mono.justOrEmpty(uploadStore.get(id))
+            .filter(pair -> userHasAccessToUpload(user, pair.left))
+            .map(userAndBlobId -> userAndBlobId.right)
+            .switchIfEmpty(Mono.error(() -> new UploadNotFoundException(id)));
+    }
+
+    private Mono<Upload> retrieveUpload(UploadId id, BlobId blobId) {
+        return Mono.zip(Mono.from(blobStore.readBytes(bucketName, blobId)),
+            Mono.just(uploadMetaDataStore.get(id)))
+            .map(bytesAndMetadata -> Upload.from(bytesAndMetadata.getT2(), bytesAndMetadata.getT1()));
+    }
+
+    private boolean userHasAccessToUpload(Username userRequest, Username owner) {

Review comment:
       Is this method needed? Added value seems small to me...

##########
File path: server/data/data-jmap/src/main/java/org/apache/james/jmap/api/model/Upload.scala
##########
@@ -0,0 +1,67 @@
+/****************************************************************
+ * 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.james.jmap.api.model
+
+import com.google.common.hash.Hashing
+import eu.timepit.refined.api.Refined
+import eu.timepit.refined.auto._
+import eu.timepit.refined.numeric.NonNegative
+import eu.timepit.refined.refineV
+import org.apache.james.jmap.api.model.Upload.Size
+import org.apache.james.mailbox.model.ContentType
+import org.slf4j.{Logger, LoggerFactory}
+
+import java.io.{ByteArrayInputStream, InputStream}
+
+object Upload {
+  private val logger: Logger = LoggerFactory.getLogger(classOf[Upload])
+  type Size = Long Refined NonNegative
+  val Zero: Size = 0L
+
+  def sanitizeSize(value: Long): Size = {
+    val size: Either[String, Size] = refineV[NonNegative](value)
+    size.fold(e => {
+      logger.error(s"Encountered an invalid Upload size: $e")
+      Zero
+    },
+      refinedValue => refinedValue)
+  }
+
+  def from(metaData: UploadMetaData, content: Array[Byte]): Upload =

Review comment:
       Please don't enforce the use of a byte array at the heart of the data model...
   
   (I prefer supplier of inutStream...)

##########
File path: server/data/data-jmap/src/main/java/org/apache/james/jmap/api/model/Upload.scala
##########
@@ -0,0 +1,67 @@
+/****************************************************************
+ * 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.james.jmap.api.model
+
+import com.google.common.hash.Hashing
+import eu.timepit.refined.api.Refined
+import eu.timepit.refined.auto._
+import eu.timepit.refined.numeric.NonNegative
+import eu.timepit.refined.refineV
+import org.apache.james.jmap.api.model.Upload.Size
+import org.apache.james.mailbox.model.ContentType
+import org.slf4j.{Logger, LoggerFactory}
+
+import java.io.{ByteArrayInputStream, InputStream}
+
+object Upload {
+  private val logger: Logger = LoggerFactory.getLogger(classOf[Upload])
+  type Size = Long Refined NonNegative
+  val Zero: Size = 0L
+
+  def sanitizeSize(value: Long): Size = {
+    val size: Either[String, Size] = refineV[NonNegative](value)
+    size.fold(e => {
+      logger.error(s"Encountered an invalid Upload size: $e")
+      Zero
+    },
+      refinedValue => refinedValue)
+  }
+
+  def from(metaData: UploadMetaData, content: Array[Byte]): Upload =
+    Upload(uploadId = metaData.getUploadId,
+      size = Upload.sanitizeSize(metaData.getSize),

Review comment:
       If we have the content at hand we should trust the content size, not the metadata handed from the client.
   
   
   (Should we throw if the size specified by the client is wrong?)

##########
File path: server/data/data-jmap/src/main/java/org/apache/james/jmap/memory/upload/InMemoryUploadRepository.java
##########
@@ -0,0 +1,149 @@
+/****************************************************************
+ * 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.james.jmap.memory.upload;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.UncheckedIOException;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+
+import javax.inject.Inject;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.james.blob.api.BlobId;
+import org.apache.james.blob.api.BlobStore;
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.core.Username;
+import org.apache.james.jmap.api.exception.UploadNotFoundException;
+import org.apache.james.jmap.api.model.Upload;
+import org.apache.james.jmap.api.model.UploadId;
+import org.apache.james.jmap.api.model.UploadMetaData;
+import org.apache.james.jmap.api.upload.UploadRepository;
+import org.apache.james.mailbox.model.ContentType;
+import org.reactivestreams.Publisher;
+
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Mono;
+
+public class InMemoryUploadRepository implements UploadRepository {
+
+    private static final Map<UploadId, ImmutablePair<Username, BlobId>> uploadStore = new HashMap<>();
+    private static final Map<UploadId, UploadMetaData> uploadMetaDataStore = new HashMap<>();
+
+    static class UploadContext {
+        private final UploadId uploadId;
+        private final boolean exists;
+        private final byte[] data;
+
+        private UploadContext(UploadId uploadId,
+                              boolean exists,
+                              byte[] data) {
+            this.uploadId = uploadId;
+            this.exists = exists;
+            this.data = data;
+        }
+    }
+
+
+    private final BlobStore blobStore;
+    private final BucketName bucketName;
+
+    @Inject
+    public InMemoryUploadRepository(BlobStore blobStore) {
+        this.blobStore = blobStore;
+        this.bucketName = blobStore.getDefaultBucketName();
+    }
+
+    @Override
+    public Publisher<UploadId> upload(InputStream data, ContentType contentType, Username user) {
+        return Mono.just(checkUploadExits(data, user))
+            .flatMap(uploadContext -> {
+                if (!uploadContext.exists) {
+                    return store(uploadContext.data, uploadContext.uploadId, contentType, user);
+                } else {
+                    return Mono.just(uploadContext.uploadId);
+                }
+            });
+    }
+
+    @Override
+    public Publisher<Upload> retrieve(UploadId id, Username user) {
+        Preconditions.checkNotNull(id);
+        Preconditions.checkNotNull(user);
+
+        return retrieveBlobId(id, user)
+            .flatMap(blobId -> retrieveUpload(id, blobId));
+    }
+
+    private Mono<BlobId> retrieveBlobId(UploadId id, Username user) {
+        return Mono.justOrEmpty(uploadStore.get(id))
+            .filter(pair -> userHasAccessToUpload(user, pair.left))
+            .map(userAndBlobId -> userAndBlobId.right)
+            .switchIfEmpty(Mono.error(() -> new UploadNotFoundException(id)));
+    }
+
+    private Mono<Upload> retrieveUpload(UploadId id, BlobId blobId) {
+        return Mono.zip(Mono.from(blobStore.readBytes(bucketName, blobId)),
+            Mono.just(uploadMetaDataStore.get(id)))
+            .map(bytesAndMetadata -> Upload.from(bytesAndMetadata.getT2(), bytesAndMetadata.getT1()));
+    }
+
+    private boolean userHasAccessToUpload(Username userRequest, Username owner) {
+        return userRequest.equals(owner);
+    }
+
+    private UploadContext checkUploadExits(InputStream data, Username username) {

Review comment:
       Let's not care about deduplication at this level as the blobStore does take care of it for us.
   
   Given that UploadId is randomly generated, we will never have conflicts. 
   
   We can thus greatly simplify the code... (And get rid of `checkUploadExits`)

##########
File path: server/data/data-jmap/src/test/java/org/apache/james/jmap/api/upload/UploadRepositoryContract.scala
##########
@@ -0,0 +1,108 @@
+ /***************************************************************
+ * 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.james.jmap.api.upload
+
+import org.apache.commons.io.IOUtils
+import org.apache.james.core.Username
+import org.apache.james.jmap.api.exception.UploadNotFoundException
+import org.apache.james.jmap.api.model.{Upload, UploadId}
+import org.apache.james.jmap.api.upload.UploadRepositoryContract.{CONTENT_TYPE, DATA, DATA_STRING, USER}
+import org.apache.james.mailbox.model.ContentType
+import org.assertj.core.api.Assertions.{assertThat, assertThatThrownBy}
+import org.junit.jupiter.api.Test
+import reactor.core.scala.publisher.SMono
+
+import java.io.InputStream
+import java.nio.charset.StandardCharsets
+
+
+object UploadRepositoryContract {
+  private lazy val CONTENT_TYPE: ContentType = ContentType
+    .of("text/html")
+  private lazy val DATA_STRING: String = "123321"
+  private lazy val DATA: InputStream = IOUtils.toInputStream(DATA_STRING, StandardCharsets.UTF_8)
+  private lazy val USER: Username = Username.of("Bob")
+}
+
+
+trait UploadRepositoryContract {
+
+  def testee: UploadRepository
+
+  @Test
+  def uploadShouldSuccess(): Unit = {
+    val uploadId: UploadId = SMono.fromPublisher(testee.upload(DATA, CONTENT_TYPE, USER)).block()
+
+    assertThat(SMono.fromPublisher(testee.retrieve(uploadId, USER)).block())
+      .isNotNull
+  }
+
+  @Test
+  def uploadShouldReturnSameIdWhenReUploadSameContent(): Unit = {

Review comment:
       IMO not needed. Dedup should be a blob store concern...




-- 
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@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] vttranlina commented on a change in pull request #551: [WIP] [JAMES-3544] UploadRepository contract & InMemory implementationtemp

Posted by GitBox <gi...@apache.org>.
vttranlina commented on a change in pull request #551:
URL: https://github.com/apache/james-project/pull/551#discussion_r676282205



##########
File path: server/data/data-jmap/src/main/java/org/apache/james/jmap/memory/upload/InMemoryUploadRepository.java
##########
@@ -0,0 +1,149 @@
+/****************************************************************
+ * 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.james.jmap.memory.upload;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.UncheckedIOException;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+
+import javax.inject.Inject;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.james.blob.api.BlobId;
+import org.apache.james.blob.api.BlobStore;
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.core.Username;
+import org.apache.james.jmap.api.exception.UploadNotFoundException;
+import org.apache.james.jmap.api.model.Upload;
+import org.apache.james.jmap.api.model.UploadId;
+import org.apache.james.jmap.api.model.UploadMetaData;
+import org.apache.james.jmap.api.upload.UploadRepository;
+import org.apache.james.mailbox.model.ContentType;
+import org.reactivestreams.Publisher;
+
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Mono;
+
+public class InMemoryUploadRepository implements UploadRepository {
+
+    private static final Map<UploadId, ImmutablePair<Username, BlobId>> uploadStore = new HashMap<>();
+    private static final Map<UploadId, UploadMetaData> uploadMetaDataStore = new HashMap<>();
+
+    static class UploadContext {
+        private final UploadId uploadId;
+        private final boolean exists;
+        private final byte[] data;
+
+        private UploadContext(UploadId uploadId,
+                              boolean exists,
+                              byte[] data) {
+            this.uploadId = uploadId;
+            this.exists = exists;
+            this.data = data;
+        }
+    }
+
+
+    private final BlobStore blobStore;
+    private final BucketName bucketName;
+
+    @Inject
+    public InMemoryUploadRepository(BlobStore blobStore) {
+        this.blobStore = blobStore;
+        this.bucketName = blobStore.getDefaultBucketName();
+    }
+
+    @Override
+    public Publisher<UploadId> upload(InputStream data, ContentType contentType, Username user) {
+        return Mono.just(checkUploadExits(data, user))
+            .flatMap(uploadContext -> {
+                if (!uploadContext.exists) {
+                    return store(uploadContext.data, uploadContext.uploadId, contentType, user);
+                } else {
+                    return Mono.just(uploadContext.uploadId);
+                }
+            });
+    }
+
+    @Override
+    public Publisher<Upload> retrieve(UploadId id, Username user) {
+        Preconditions.checkNotNull(id);
+        Preconditions.checkNotNull(user);
+
+        return retrieveBlobId(id, user)
+            .flatMap(blobId -> retrieveUpload(id, blobId));
+    }
+
+    private Mono<BlobId> retrieveBlobId(UploadId id, Username user) {
+        return Mono.justOrEmpty(uploadStore.get(id))
+            .filter(pair -> userHasAccessToUpload(user, pair.left))
+            .map(userAndBlobId -> userAndBlobId.right)
+            .switchIfEmpty(Mono.error(() -> new UploadNotFoundException(id)));
+    }
+
+    private Mono<Upload> retrieveUpload(UploadId id, BlobId blobId) {
+        return Mono.zip(Mono.from(blobStore.readBytes(bucketName, blobId)),
+            Mono.just(uploadMetaDataStore.get(id)))
+            .map(bytesAndMetadata -> Upload.from(bytesAndMetadata.getT2(), bytesAndMetadata.getT1()));
+    }
+
+    private boolean userHasAccessToUpload(Username userRequest, Username owner) {

Review comment:
       I thought we can extend logic in the future, so here is a reserve. 
   I will implement checking logic in-line. 




-- 
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@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on pull request #551: [JAMES-3544] UploadRepository contract & InMemory implementationtemp

Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #551:
URL: https://github.com/apache/james-project/pull/551#issuecomment-888750436


   Can you have a look @vttranlina ?


-- 
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@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] vttranlina commented on a change in pull request #551: [WIP] [JAMES-3544] UploadRepository contract & InMemory implementationtemp

Posted by GitBox <gi...@apache.org>.
vttranlina commented on a change in pull request #551:
URL: https://github.com/apache/james-project/pull/551#discussion_r676282584



##########
File path: server/data/data-jmap/src/test/java/org/apache/james/jmap/api/upload/UploadRepositoryContract.scala
##########
@@ -0,0 +1,108 @@
+ /***************************************************************
+ * 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.james.jmap.api.upload
+
+import org.apache.commons.io.IOUtils
+import org.apache.james.core.Username
+import org.apache.james.jmap.api.exception.UploadNotFoundException
+import org.apache.james.jmap.api.model.{Upload, UploadId}
+import org.apache.james.jmap.api.upload.UploadRepositoryContract.{CONTENT_TYPE, DATA, DATA_STRING, USER}
+import org.apache.james.mailbox.model.ContentType
+import org.assertj.core.api.Assertions.{assertThat, assertThatThrownBy}
+import org.junit.jupiter.api.Test
+import reactor.core.scala.publisher.SMono
+
+import java.io.InputStream
+import java.nio.charset.StandardCharsets
+
+
+object UploadRepositoryContract {
+  private lazy val CONTENT_TYPE: ContentType = ContentType
+    .of("text/html")
+  private lazy val DATA_STRING: String = "123321"
+  private lazy val DATA: InputStream = IOUtils.toInputStream(DATA_STRING, StandardCharsets.UTF_8)
+  private lazy val USER: Username = Username.of("Bob")
+}
+
+
+trait UploadRepositoryContract {
+
+  def testee: UploadRepository
+
+  @Test
+  def uploadShouldSuccess(): Unit = {
+    val uploadId: UploadId = SMono.fromPublisher(testee.upload(DATA, CONTENT_TYPE, USER)).block()
+
+    assertThat(SMono.fromPublisher(testee.retrieve(uploadId, USER)).block())
+      .isNotNull
+  }
+
+  @Test
+  def uploadShouldReturnSameIdWhenReUploadSameContent(): Unit = {

Review comment:
       What about ADR? 
   (https://github.com/apache/james-project/pull/544/files#L22)
   ```
   if reuploaded, the same blobId MAY be returned, but this SHOULD reset the expiry time.
   ```




-- 
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@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] vttranlina commented on a change in pull request #551: [WIP] [JAMES-3544] UploadRepository contract & InMemory implementationtemp

Posted by GitBox <gi...@apache.org>.
vttranlina commented on a change in pull request #551:
URL: https://github.com/apache/james-project/pull/551#discussion_r676281553



##########
File path: server/data/data-jmap/src/main/java/org/apache/james/jmap/memory/upload/InMemoryUploadRepository.java
##########
@@ -0,0 +1,149 @@
+/****************************************************************
+ * 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.james.jmap.memory.upload;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.UncheckedIOException;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+
+import javax.inject.Inject;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.james.blob.api.BlobId;
+import org.apache.james.blob.api.BlobStore;
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.core.Username;
+import org.apache.james.jmap.api.exception.UploadNotFoundException;
+import org.apache.james.jmap.api.model.Upload;
+import org.apache.james.jmap.api.model.UploadId;
+import org.apache.james.jmap.api.model.UploadMetaData;
+import org.apache.james.jmap.api.upload.UploadRepository;
+import org.apache.james.mailbox.model.ContentType;
+import org.reactivestreams.Publisher;
+
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Mono;
+
+public class InMemoryUploadRepository implements UploadRepository {
+
+    private static final Map<UploadId, ImmutablePair<Username, BlobId>> uploadStore = new HashMap<>();
+    private static final Map<UploadId, UploadMetaData> uploadMetaDataStore = new HashMap<>();
+
+    static class UploadContext {
+        private final UploadId uploadId;
+        private final boolean exists;
+        private final byte[] data;
+
+        private UploadContext(UploadId uploadId,
+                              boolean exists,
+                              byte[] data) {
+            this.uploadId = uploadId;
+            this.exists = exists;
+            this.data = data;
+        }
+    }
+
+
+    private final BlobStore blobStore;
+    private final BucketName bucketName;
+
+    @Inject
+    public InMemoryUploadRepository(BlobStore blobStore) {
+        this.blobStore = blobStore;
+        this.bucketName = blobStore.getDefaultBucketName();
+    }
+
+    @Override
+    public Publisher<UploadId> upload(InputStream data, ContentType contentType, Username user) {
+        return Mono.just(checkUploadExits(data, user))
+            .flatMap(uploadContext -> {
+                if (!uploadContext.exists) {
+                    return store(uploadContext.data, uploadContext.uploadId, contentType, user);
+                } else {
+                    return Mono.just(uploadContext.uploadId);
+                }
+            });
+    }
+
+    @Override
+    public Publisher<Upload> retrieve(UploadId id, Username user) {
+        Preconditions.checkNotNull(id);
+        Preconditions.checkNotNull(user);
+
+        return retrieveBlobId(id, user)
+            .flatMap(blobId -> retrieveUpload(id, blobId));
+    }
+
+    private Mono<BlobId> retrieveBlobId(UploadId id, Username user) {
+        return Mono.justOrEmpty(uploadStore.get(id))
+            .filter(pair -> userHasAccessToUpload(user, pair.left))
+            .map(userAndBlobId -> userAndBlobId.right)
+            .switchIfEmpty(Mono.error(() -> new UploadNotFoundException(id)));
+    }
+
+    private Mono<Upload> retrieveUpload(UploadId id, BlobId blobId) {
+        return Mono.zip(Mono.from(blobStore.readBytes(bucketName, blobId)),
+            Mono.just(uploadMetaDataStore.get(id)))
+            .map(bytesAndMetadata -> Upload.from(bytesAndMetadata.getT2(), bytesAndMetadata.getT1()));
+    }
+
+    private boolean userHasAccessToUpload(Username userRequest, Username owner) {
+        return userRequest.equals(owner);
+    }
+
+    private UploadContext checkUploadExits(InputStream data, Username username) {

Review comment:
       is it in conflict with the wrote you did before? 
   https://github.com/linagora/james-project/issues/4349#issuecomment-885495718




-- 
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@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] vttranlina commented on a change in pull request #551: [WIP] [JAMES-3544] UploadRepository contract & InMemory implementationtemp

Posted by GitBox <gi...@apache.org>.
vttranlina commented on a change in pull request #551:
URL: https://github.com/apache/james-project/pull/551#discussion_r676395809



##########
File path: server/data/data-jmap/src/main/java/org/apache/james/jmap/api/model/Upload.scala
##########
@@ -19,49 +19,38 @@
 
 package org.apache.james.jmap.api.model
 
-import com.google.common.hash.Hashing
-import eu.timepit.refined.api.Refined
-import eu.timepit.refined.auto._
-import eu.timepit.refined.numeric.NonNegative
-import eu.timepit.refined.refineV
-import org.apache.james.jmap.api.model.Upload.Size
+import org.apache.james.blob.api.BlobId
+import org.apache.james.jmap.api.model.Size.Size
 import org.apache.james.mailbox.model.ContentType
-import org.slf4j.{Logger, LoggerFactory}
 
-import java.io.{ByteArrayInputStream, InputStream}
+import java.io.InputStream
 
 object Upload {
-  private val logger: Logger = LoggerFactory.getLogger(classOf[Upload])
-  type Size = Long Refined NonNegative
-  val Zero: Size = 0L
 
-  def sanitizeSize(value: Long): Size = {
-    val size: Either[String, Size] = refineV[NonNegative](value)
-    size.fold(e => {
-      logger.error(s"Encountered an invalid Upload size: $e")
-      Zero
-    },
-      refinedValue => refinedValue)
-  }
-
-  def from(metaData: UploadMetaData, content: Array[Byte]): Upload =
-    Upload(uploadId = metaData.getUploadId,
-      size = Upload.sanitizeSize(metaData.getSize),
-      contentType = metaData.getContentType,
-      content = new ByteArrayInputStream(content))
+  def from(metaData: UploadMetaData, content: InputStream): Upload =
+    Upload(uploadId = metaData.uploadId,
+      size = metaData.size,
+      contentType = metaData.contentType,
+      content = content)
 }
 
 case class Upload(uploadId: UploadId,
                   size: Size,
                   contentType: ContentType,
                   content: InputStream)
 
-object UploadHashContent {
-  def from(content: Array[Byte]): UploadHashContent =
-    UploadHashContent(Hashing.sha256.hashBytes(content).toString)
+case class UploadNotFoundException(uploadId: UploadId) extends RuntimeException(s"Upload not found $uploadId")
 
+object UploadMetaData {
+  def from(uploadId: UploadId, contentType: ContentType, size: Long, blobId: BlobId): UploadMetaData =
+    UploadMetaData(uploadId = uploadId,
+      contentType = contentType,
+      size = Size.sanitizeSize(size),

Review comment:
       I got the error `No such instance method`, when I use `Size.sanitizeSize(size)` in Java class. 
   weird




-- 
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@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a change in pull request #551: [WIP] [JAMES-3544] UploadRepository contract & InMemory implementationtemp

Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #551:
URL: https://github.com/apache/james-project/pull/551#discussion_r676287660



##########
File path: server/data/data-jmap/src/main/java/org/apache/james/jmap/memory/upload/InMemoryUploadRepository.java
##########
@@ -0,0 +1,149 @@
+/****************************************************************
+ * 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.james.jmap.memory.upload;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.UncheckedIOException;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+
+import javax.inject.Inject;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.james.blob.api.BlobId;
+import org.apache.james.blob.api.BlobStore;
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.core.Username;
+import org.apache.james.jmap.api.exception.UploadNotFoundException;
+import org.apache.james.jmap.api.model.Upload;
+import org.apache.james.jmap.api.model.UploadId;
+import org.apache.james.jmap.api.model.UploadMetaData;
+import org.apache.james.jmap.api.upload.UploadRepository;
+import org.apache.james.mailbox.model.ContentType;
+import org.reactivestreams.Publisher;
+
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Mono;
+
+public class InMemoryUploadRepository implements UploadRepository {
+
+    private static final Map<UploadId, ImmutablePair<Username, BlobId>> uploadStore = new HashMap<>();
+    private static final Map<UploadId, UploadMetaData> uploadMetaDataStore = new HashMap<>();
+
+    static class UploadContext {
+        private final UploadId uploadId;
+        private final boolean exists;
+        private final byte[] data;
+
+        private UploadContext(UploadId uploadId,
+                              boolean exists,
+                              byte[] data) {
+            this.uploadId = uploadId;
+            this.exists = exists;
+            this.data = data;
+        }
+    }
+
+
+    private final BlobStore blobStore;
+    private final BucketName bucketName;
+
+    @Inject
+    public InMemoryUploadRepository(BlobStore blobStore) {
+        this.blobStore = blobStore;
+        this.bucketName = blobStore.getDefaultBucketName();
+    }
+
+    @Override
+    public Publisher<UploadId> upload(InputStream data, ContentType contentType, Username user) {
+        return Mono.just(checkUploadExits(data, user))
+            .flatMap(uploadContext -> {
+                if (!uploadContext.exists) {
+                    return store(uploadContext.data, uploadContext.uploadId, contentType, user);
+                } else {
+                    return Mono.just(uploadContext.uploadId);
+                }
+            });
+    }
+
+    @Override
+    public Publisher<Upload> retrieve(UploadId id, Username user) {
+        Preconditions.checkNotNull(id);
+        Preconditions.checkNotNull(user);
+
+        return retrieveBlobId(id, user)
+            .flatMap(blobId -> retrieveUpload(id, blobId));
+    }
+
+    private Mono<BlobId> retrieveBlobId(UploadId id, Username user) {
+        return Mono.justOrEmpty(uploadStore.get(id))
+            .filter(pair -> userHasAccessToUpload(user, pair.left))
+            .map(userAndBlobId -> userAndBlobId.right)
+            .switchIfEmpty(Mono.error(() -> new UploadNotFoundException(id)));
+    }
+
+    private Mono<Upload> retrieveUpload(UploadId id, BlobId blobId) {
+        return Mono.zip(Mono.from(blobStore.readBytes(bucketName, blobId)),
+            Mono.just(uploadMetaDataStore.get(id)))
+            .map(bytesAndMetadata -> Upload.from(bytesAndMetadata.getT2(), bytesAndMetadata.getT1()));
+    }
+
+    private boolean userHasAccessToUpload(Username userRequest, Username owner) {
+        return userRequest.equals(owner);
+    }
+
+    private UploadContext checkUploadExits(InputStream data, Username username) {

Review comment:
       MAY means that we are able to do it if we want, in no way that we have to do it ;-).
   
   So no, no incompatibility at all ;-)
   
   (And it is a quote of the JMAP spec serving another purpose there....)




-- 
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@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa merged pull request #551: [JAMES-3544] UploadRepository contract & InMemory implementationtemp

Posted by GitBox <gi...@apache.org>.
chibenwa merged pull request #551:
URL: https://github.com/apache/james-project/pull/551


   


-- 
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@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] vttranlina commented on pull request #551: [JAMES-3544] UploadRepository contract & InMemory implementationtemp

Posted by GitBox <gi...@apache.org>.
vttranlina commented on pull request #551:
URL: https://github.com/apache/james-project/pull/551#issuecomment-888874141


   rebase master & squash fixup


-- 
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@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on pull request #551: [JAMES-3544] UploadRepository contract & InMemory implementationtemp

Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #551:
URL: https://github.com/apache/james-project/pull/551#issuecomment-887213838


   `08:29:42,205 [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project james-server-data-jmap: Compilation failure: Compilation failure`
   
   Can we check that we have the scala compilation plugin?


-- 
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@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a change in pull request #551: [WIP] [JAMES-3544] UploadRepository contract & InMemory implementationtemp

Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #551:
URL: https://github.com/apache/james-project/pull/551#discussion_r676287945



##########
File path: server/data/data-jmap/src/main/java/org/apache/james/jmap/memory/upload/InMemoryUploadRepository.java
##########
@@ -0,0 +1,149 @@
+/****************************************************************
+ * 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.james.jmap.memory.upload;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.UncheckedIOException;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+
+import javax.inject.Inject;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.james.blob.api.BlobId;
+import org.apache.james.blob.api.BlobStore;
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.core.Username;
+import org.apache.james.jmap.api.exception.UploadNotFoundException;
+import org.apache.james.jmap.api.model.Upload;
+import org.apache.james.jmap.api.model.UploadId;
+import org.apache.james.jmap.api.model.UploadMetaData;
+import org.apache.james.jmap.api.upload.UploadRepository;
+import org.apache.james.mailbox.model.ContentType;
+import org.reactivestreams.Publisher;
+
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Mono;
+
+public class InMemoryUploadRepository implements UploadRepository {
+
+    private static final Map<UploadId, ImmutablePair<Username, BlobId>> uploadStore = new HashMap<>();
+    private static final Map<UploadId, UploadMetaData> uploadMetaDataStore = new HashMap<>();
+
+    static class UploadContext {
+        private final UploadId uploadId;
+        private final boolean exists;
+        private final byte[] data;
+
+        private UploadContext(UploadId uploadId,
+                              boolean exists,
+                              byte[] data) {
+            this.uploadId = uploadId;
+            this.exists = exists;
+            this.data = data;
+        }
+    }
+
+
+    private final BlobStore blobStore;
+    private final BucketName bucketName;
+
+    @Inject
+    public InMemoryUploadRepository(BlobStore blobStore) {
+        this.blobStore = blobStore;
+        this.bucketName = blobStore.getDefaultBucketName();
+    }
+
+    @Override
+    public Publisher<UploadId> upload(InputStream data, ContentType contentType, Username user) {
+        return Mono.just(checkUploadExits(data, user))
+            .flatMap(uploadContext -> {
+                if (!uploadContext.exists) {
+                    return store(uploadContext.data, uploadContext.uploadId, contentType, user);
+                } else {
+                    return Mono.just(uploadContext.uploadId);
+                }
+            });
+    }
+
+    @Override
+    public Publisher<Upload> retrieve(UploadId id, Username user) {
+        Preconditions.checkNotNull(id);
+        Preconditions.checkNotNull(user);
+
+        return retrieveBlobId(id, user)
+            .flatMap(blobId -> retrieveUpload(id, blobId));
+    }
+
+    private Mono<BlobId> retrieveBlobId(UploadId id, Username user) {
+        return Mono.justOrEmpty(uploadStore.get(id))
+            .filter(pair -> userHasAccessToUpload(user, pair.left))
+            .map(userAndBlobId -> userAndBlobId.right)
+            .switchIfEmpty(Mono.error(() -> new UploadNotFoundException(id)));
+    }
+
+    private Mono<Upload> retrieveUpload(UploadId id, BlobId blobId) {
+        return Mono.zip(Mono.from(blobStore.readBytes(bucketName, blobId)),
+            Mono.just(uploadMetaDataStore.get(id)))
+            .map(bytesAndMetadata -> Upload.from(bytesAndMetadata.getT2(), bytesAndMetadata.getT1()));
+    }
+
+    private boolean userHasAccessToUpload(Username userRequest, Username owner) {

Review comment:
       I prefer having user.equals(owner) directly, I would find it clearer.
   
   We can extract to a method later, when it makes sense IMO...

##########
File path: server/data/data-jmap/src/test/java/org/apache/james/jmap/api/upload/UploadRepositoryContract.scala
##########
@@ -0,0 +1,108 @@
+ /***************************************************************
+ * 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.james.jmap.api.upload
+
+import org.apache.commons.io.IOUtils
+import org.apache.james.core.Username
+import org.apache.james.jmap.api.exception.UploadNotFoundException
+import org.apache.james.jmap.api.model.{Upload, UploadId}
+import org.apache.james.jmap.api.upload.UploadRepositoryContract.{CONTENT_TYPE, DATA, DATA_STRING, USER}
+import org.apache.james.mailbox.model.ContentType
+import org.assertj.core.api.Assertions.{assertThat, assertThatThrownBy}
+import org.junit.jupiter.api.Test
+import reactor.core.scala.publisher.SMono
+
+import java.io.InputStream
+import java.nio.charset.StandardCharsets
+
+
+object UploadRepositoryContract {
+  private lazy val CONTENT_TYPE: ContentType = ContentType
+    .of("text/html")
+  private lazy val DATA_STRING: String = "123321"
+  private lazy val DATA: InputStream = IOUtils.toInputStream(DATA_STRING, StandardCharsets.UTF_8)
+  private lazy val USER: Username = Username.of("Bob")
+}
+
+
+trait UploadRepositoryContract {
+
+  def testee: UploadRepository
+
+  @Test
+  def uploadShouldSuccess(): Unit = {
+    val uploadId: UploadId = SMono.fromPublisher(testee.upload(DATA, CONTENT_TYPE, USER)).block()
+
+    assertThat(SMono.fromPublisher(testee.retrieve(uploadId, USER)).block())
+      .isNotNull
+  }
+
+  @Test
+  def uploadShouldReturnSameIdWhenReUploadSameContent(): Unit = {

Review comment:
       Idem




-- 
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@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on pull request #551: [JAMES-3544] UploadRepository contract & InMemory implementationtemp

Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #551:
URL: https://github.com/apache/james-project/pull/551#issuecomment-888750347


   ```
   08:32:16,512 [ERROR] Failed to execute goal net.alchim31.maven:scala-maven-plugin:3.4.6:compile (scala-compile-first) on project james-server-jmap-rfc-8621: wrap: org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1) -> [Help 1]
   org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal net.alchim31.maven:scala-maven-plugin:3.4.6:compile (scala-compile-first) on project james-server-jmap-rfc-8621: wrap: org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1)
   ```


-- 
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@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org