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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to