Merge pull request #1065 from amvanbaren/bugfix/issue-1064

Fix change namespace action
eclipse · Dec 9, 2024 · 0f5315d · 0f5315d
2 parents c1f83fd + e7ad16b
commit 0f5315d
Show file tree

Hide file tree

Showing 13 changed files with 151 additions and 56 deletions.
diff --git a/server/src/main/java/org/eclipse/openvsx/UserAPI.java b/server/src/main/java/org/eclipse/openvsx/UserAPI.java
@@ -264,7 +264,6 @@ public ResponseEntity<ResultJson> updateNamespaceDetailsLogo(
     ) {
         try {
             return ResponseEntity.ok()
-                    .cacheControl(CacheControl.maxAge(10, TimeUnit.MINUTES).cachePublic())
                     .body(users.updateNamespaceDetailsLogo(namespace, file));
         } catch (ErrorResultException exc) {
             return exc.toResponseEntity(ResultJson.class);

diff --git a/server/src/main/java/org/eclipse/openvsx/UserService.java b/server/src/main/java/org/eclipse/openvsx/UserService.java
@@ -38,6 +38,7 @@
 
 import java.io.IOException;
 import java.nio.file.Files;
+import java.util.List;
 import java.util.Objects;
 import java.util.UUID;
 
@@ -237,6 +238,12 @@ public ResultJson updateNamespaceDetails(NamespaceDetailsJson details) {
         if(!Objects.equals(details.getSocialLinks(), namespace.getSocialLinks())) {
             namespace.setSocialLinks(details.getSocialLinks());
         }
+        if(StringUtils.isEmpty(details.getLogo()) && StringUtils.isNotEmpty(namespace.getLogoName())) {
+            storageUtil.removeNamespaceLogo(namespace);
+            namespace.clearLogoBytes();
+            namespace.setLogoName(null);
+            namespace.setLogoStorageType(null);
+        }
 
         return ResultJson.success("Updated details for namespace " + details.getName());
     }
@@ -257,15 +264,12 @@ public ResultJson updateNamespaceDetailsLogo(String namespaceName, MultipartFile
             var tika = new Tika();
             var detectedType = tika.detect(file.getInputStream(), file.getOriginalFilename());
             var logoType = MimeTypes.getDefaultMimeTypes().getRegisteredMimeType(detectedType);
-            if(logoType != null) {
-                if(!logoType.getType().equals(MediaType.image("png")) && !logoType.getType().equals(MediaType.image("jpg"))) {
-                    throw new ErrorResultException("Namespace logo should be of png or jpg type");
-                }
-
-                var logoName = "logo-" + namespace.getName() + "-" + System.currentTimeMillis() + logoType.getExtension();
-                namespace.setLogoName(logoName);
+            var expectedLogoTypes = List.of(MediaType.image("png"), MediaType.image("jpg"));
+            if(logoType == null || !expectedLogoTypes.contains(logoType.getType())) {
+                throw new ErrorResultException("Namespace logo should be a png or jpg file");
             }
 
+            namespace.setLogoName(NamingUtil.toLogoName(namespace, logoType));
             file.getInputStream().transferTo(out);
             logoFile.setNamespace(namespace);
             storageUtil.uploadNamespaceLogo(logoFile);

diff --git a/server/src/main/java/org/eclipse/openvsx/admin/ChangeNamespaceJobRequestHandler.java b/server/src/main/java/org/eclipse/openvsx/admin/ChangeNamespaceJobRequestHandler.java
@@ -9,6 +9,7 @@
  * ****************************************************************************** */
 package org.eclipse.openvsx.admin;
 
+import org.apache.commons.lang3.StringUtils;
 import org.eclipse.openvsx.ExtensionValidator;
 import org.eclipse.openvsx.entities.Extension;
 import org.eclipse.openvsx.entities.ExtensionVersion;
@@ -104,6 +105,11 @@ private void execute(ChangeNamespaceJobRequest jobRequest) {
                 })
                 .collect(Collectors.toList());
 
+        if(StringUtils.isNotEmpty(oldNamespace.getLogoName())) {
+            newNamespace.setLogoName(NamingUtil.changeLogoName(oldNamespace, newNamespace));
+            storageUtil.copyNamespaceLogo(oldNamespace, newNamespace);
+        }
+
         service.changeNamespaceInDatabase(newNamespace, oldNamespace, updatedResources, createNewNamespace, json.removeOldNamespace());
 
         // remove the old resources from external storage

diff --git a/server/src/main/java/org/eclipse/openvsx/admin/ChangeNamespaceService.java b/server/src/main/java/org/eclipse/openvsx/admin/ChangeNamespaceService.java
@@ -72,6 +72,8 @@ public void changeNamespaceInDatabase(
             entityManager.remove(oldNamespace);
         }
 
+        cache.evictSitemap();
+        cache.evictNamespaceDetails(oldNamespace);
         search.updateSearchEntries(extensions.toList());
     }
 

diff --git a/server/src/main/java/org/eclipse/openvsx/cache/CacheService.java b/server/src/main/java/org/eclipse/openvsx/cache/CacheService.java
@@ -9,10 +9,7 @@
  * ****************************************************************************** */
 package org.eclipse.openvsx.cache;
 
-import org.eclipse.openvsx.entities.Extension;
-import org.eclipse.openvsx.entities.ExtensionVersion;
-import org.eclipse.openvsx.entities.FileResource;
-import org.eclipse.openvsx.entities.UserData;
+import org.eclipse.openvsx.entities.*;
 import org.eclipse.openvsx.repositories.RepositoryService;
 import org.eclipse.openvsx.util.TargetPlatform;
 import org.eclipse.openvsx.util.VersionAlias;
@@ -59,17 +56,28 @@ public CacheService(
         this.filesCacheKeyGenerator = filesCacheKeyGenerator;
     }
 
+    public void evictSitemap() {
+        invalidateCache(CACHE_SITEMAP);
+    }
+
     public void evictNamespaceDetails() {
         invalidateCache(CACHE_NAMESPACE_DETAILS_JSON);
     }
 
+    public void evictNamespaceDetails(Namespace namespace) {
+        evictNamespaceDetails(namespace.getName());
+    }
+
     public void evictNamespaceDetails(Extension extension) {
+        evictNamespaceDetails(extension.getNamespace().getName());
+    }
+
+    private void evictNamespaceDetails(String namespaceName) {
         var cache = cacheManager.getCache(CACHE_NAMESPACE_DETAILS_JSON);
         if(cache == null) {
             return; // cache is not created
         }
 
-        var namespaceName = extension.getNamespace().getName();
         cache.evictIfPresent(namespaceName);
     }
 

diff --git a/server/src/main/java/org/eclipse/openvsx/storage/AwsStorageService.java b/server/src/main/java/org/eclipse/openvsx/storage/AwsStorageService.java
@@ -222,18 +222,23 @@ public TempFile downloadFile(FileResource resource) throws IOException {
 
     @Override
     public void copyFiles(List<Pair<FileResource, FileResource>> pairs) {
-        for(var pair : pairs) {
-            var oldObjectKey = getObjectKey(pair.getFirst());
-            var newObjectKey = getObjectKey(pair.getSecond());
-            var request = CopyObjectRequest.builder()
-                    .sourceBucket(bucket)
-                    .sourceKey(oldObjectKey)
-                    .destinationBucket(bucket)
-                    .destinationKey(newObjectKey)
-                    .build();
+        pairs.forEach(pair -> copy(getObjectKey(pair.getFirst()), getObjectKey(pair.getSecond())));
+    }
 
-            getS3Client().copyObject(request);
-        }
+    @Override
+    public void copyNamespaceLogo(Namespace oldNamespace, Namespace newNamespace) {
+        copy(getObjectKey(oldNamespace), getObjectKey(newNamespace));
+    }
+
+    private void copy(String oldObjectKey, String newObjectKey) {
+        var request = CopyObjectRequest.builder()
+                .sourceBucket(bucket)
+                .sourceKey(oldObjectKey)
+                .destinationBucket(bucket)
+                .destinationKey(newObjectKey)
+                .build();
+
+        getS3Client().copyObject(request);
     }
 
     @Override

diff --git a/server/src/main/java/org/eclipse/openvsx/storage/AzureBlobStorageService.java b/server/src/main/java/org/eclipse/openvsx/storage/AzureBlobStorageService.java
@@ -215,6 +215,19 @@ public void copyFiles(List<Pair<FileResource,FileResource>> pairs) {
         }
     }
 
+    @Override
+    public void copyNamespaceLogo(Namespace oldNamespace, Namespace newNamespace) {
+        var oldLocation = getNamespaceLogoLocation(oldNamespace).toString();
+        var newBlobName = getBlobName(newNamespace);
+        var poller = getContainerClient().getBlobClient(newBlobName)
+                .beginCopy(oldLocation, Duration.of(1, ChronoUnit.SECONDS));
+
+        var response = poller.waitForCompletion();
+        if(response.getValue().getCopyStatus() != CopyStatusType.SUCCESS) {
+            throw new RuntimeException(response.getValue().getError());
+        }
+    }
+
     @Override
     @Cacheable(value = CACHE_EXTENSION_FILES, keyGenerator = GENERATOR_FILES)
     public Path getCachedFile(FileResource resource) throws IOException {

diff --git a/server/src/main/java/org/eclipse/openvsx/storage/GoogleCloudStorageService.java b/server/src/main/java/org/eclipse/openvsx/storage/GoogleCloudStorageService.java
@@ -188,16 +188,21 @@ private String missingBucketIdMessage(String action, String name) {
 
     @Override
     public void copyFiles(List<Pair<FileResource,FileResource>> pairs) {
-        for(var pair : pairs) {
-            var source = getObjectId(pair.getFirst());
-            var target = getObjectId(pair.getSecond());
-            var request = new Storage.CopyRequest.Builder()
-                    .setSource(BlobId.of(bucketId, source))
-                    .setTarget(BlobId.of(bucketId, target))
-                    .build();
-
-            getStorage().copy(request).getResult();
-        }
+        pairs.forEach(pair -> copy(getObjectId(pair.getFirst()), getObjectId(pair.getSecond())));
+    }
+
+    @Override
+    public void copyNamespaceLogo(Namespace oldNamespace, Namespace newNamespace) {
+        copy(getObjectId(oldNamespace), getObjectId(newNamespace));
+    }
+
+    private void copy(String source, String target) {
+        var request = new Storage.CopyRequest.Builder()
+                .setSource(BlobId.of(bucketId, source))
+                .setTarget(BlobId.of(bucketId, target))
+                .build();
+
+        getStorage().copy(request).getResult();
     }
 
     @Override

diff --git a/server/src/main/java/org/eclipse/openvsx/storage/IStorageService.java b/server/src/main/java/org/eclipse/openvsx/storage/IStorageService.java
@@ -60,5 +60,7 @@ public interface IStorageService {
 
     void copyFiles(List<Pair<FileResource, FileResource>> pairs);
 
+    void copyNamespaceLogo(Namespace oldNamespace, Namespace newNamespace);
+
     Path getCachedFile(FileResource resource) throws IOException;
 }
diff --git a/server/src/main/java/org/eclipse/openvsx/storage/LocalStorageService.java b/server/src/main/java/org/eclipse/openvsx/storage/LocalStorageService.java
@@ -138,15 +138,22 @@ public TempFile downloadFile(FileResource resource) throws IOException {
     public void copyFiles(List<Pair<FileResource, FileResource>> pairs) {
         try {
             for (var pair : pairs) {
-                var source = getPath(pair.getFirst());
-                var target = getPath(pair.getSecond());
-                Files.copy(source, target);
+                Files.copy(getPath(pair.getFirst()), getPath(pair.getSecond()), StandardCopyOption.REPLACE_EXISTING);
             }
         } catch (IOException e) {
             throw new RuntimeException(e);
         }
     }
 
+    @Override
+    public void copyNamespaceLogo(Namespace oldNamespace, Namespace newNamespace) {
+        try {
+            Files.copy(getLogoPath(oldNamespace), getLogoPath(newNamespace), StandardCopyOption.REPLACE_EXISTING);
+        } catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
     private Path getPath(FileResource resource) {
         if(!isEnabled()) {
             throw new IllegalStateException("Cannot determine location of file. Configure the 'ovsx.storage.local.directory' property.");

diff --git a/server/src/main/java/org/eclipse/openvsx/storage/StorageUtilService.java b/server/src/main/java/org/eclipse/openvsx/storage/StorageUtilService.java
@@ -342,6 +342,26 @@ public void copyFiles(List<Pair<FileResource,FileResource>> pairs) {
         }
     }
 
+    @Override
+    public void copyNamespaceLogo(Namespace oldNamespace, Namespace newNamespace) {
+        switch (oldNamespace.getLogoStorageType()) {
+            case STORAGE_GOOGLE:
+                googleStorage.copyNamespaceLogo(oldNamespace, newNamespace);
+                break;
+            case STORAGE_AZURE:
+                azureStorage.copyNamespaceLogo(oldNamespace, newNamespace);
+                break;
+            case STORAGE_AWS:
+                awsStorage.copyNamespaceLogo(oldNamespace, newNamespace);
+                break;
+            case STORAGE_LOCAL:
+                localStorage.copyNamespaceLogo(oldNamespace, newNamespace);
+                break;
+        }
+
+        newNamespace.setLogoStorageType(oldNamespace.getLogoStorageType());
+    }
+
     @Override
     public Path getCachedFile(FileResource resource) throws IOException {
         return switch (resource.getStorageType()) {

diff --git a/server/src/main/java/org/eclipse/openvsx/util/NamingUtil.java b/server/src/main/java/org/eclipse/openvsx/util/NamingUtil.java
@@ -10,9 +10,11 @@
 package org.eclipse.openvsx.util;
 
 import org.apache.commons.lang3.StringUtils;
+import org.apache.tika.mime.MimeType;
 import org.eclipse.openvsx.adapter.ExtensionQueryResult;
 import org.eclipse.openvsx.entities.Extension;
 import org.eclipse.openvsx.entities.ExtensionVersion;
+import org.eclipse.openvsx.entities.Namespace;
 import org.eclipse.openvsx.json.ExtensionJson;
 import org.eclipse.openvsx.search.ExtensionSearch;
 
@@ -93,4 +95,11 @@ public static ExtensionId fromExtensionId(String text) {
                 : null;
     }
 
+    public static String toLogoName(Namespace namespace, MimeType logoType) {
+        return  "logo-" + namespace.getName() + "-" + System.currentTimeMillis() + logoType.getExtension();
+    }
+
+    public static String changeLogoName(Namespace oldNamespace, Namespace newNamespace) {
+        return oldNamespace.getLogoName().replace("-" + oldNamespace.getName() + "-", "-" + newNamespace.getName() + "-");
+    }
 }
diff --git a/webui/src/pages/user/user-namespace-details.tsx b/webui/src/pages/user/user-namespace-details.tsx
@@ -8,7 +8,7 @@
  * SPDX-License-Identifier: EPL-2.0
  ********************************************************************************/
 
-import React, { ChangeEvent, FunctionComponent, useContext, useEffect, useRef, useState } from 'react';
+import React, { ChangeEvent, FunctionComponent, useContext, useEffect, useMemo, useRef, useState } from 'react';
 import { Box, TextField, Typography, Grid, Button, IconButton, Slider, Stack, Dialog, DialogActions, DialogTitle,
     DialogContent, InputAdornment, Select, MenuItem, Paper, SelectChangeEvent } from '@mui/material';
 import { CheckCircleOutline } from '@mui/icons-material';
@@ -101,6 +101,11 @@ export const UserNamespaceDetails: FunctionComponent<UserNamespaceDetailsProps>
     const [prevEditorPosition, setPrevEditorPosition] = useState<Position>();
     const [linkedInAccountType, setLinkedInAccountType] = useState<string>(LINKED_IN_PERSONAL);
 
+    const noChanges = useMemo(() => {
+        const isFalsy = (x: unknown) => !!x === false;
+        return _.isEqual(_.omitBy(currentDetails, isFalsy), _.omitBy(newDetails, isFalsy));
+    }, [currentDetails, newDetails]);
+
     useEffect(() => {
         getNamespaceDetails();
         return () => abortController.current.abort();
@@ -194,8 +199,15 @@ export const UserNamespaceDetails: FunctionComponent<UserNamespaceDetailsProps>
                 throw result;
             }
 
+            if (logoPreview) {
+                const logoFile = await (await fetch(logoPreview)).blob();
+                await context.service.setNamespaceLogo(abortController.current, details.name, logoFile, details.logo as string);
+                await getNamespaceDetails();
+            } else {
+                setCurrentDetails(copy(newDetails));
+            }
+
             setDetailsUpdated(true);
-            setCurrentDetails(copy(details));
             setBannerNamespaceName(details.displayName || details.name);
         } catch (err) {
             context.handleError(err);
@@ -303,21 +315,24 @@ export const UserNamespaceDetails: FunctionComponent<UserNamespaceDetailsProps>
         setEditing(false);
     };
 
-    const handleSaveLogo = () => {
-        const canvasScaled = editor.current?.getImageScaledToCanvas();
-        if (canvasScaled) {
-            canvasScaled.toBlob(async (blob) => {
-                if (blob) {
-                    if (logoPreview) {
-                        URL.revokeObjectURL(logoPreview);
-                    }
-                    setLogoPreview(URL.createObjectURL(blob));
-                    await context.service.setNamespaceLogo(abortController.current, props.namespace.name, blob, dropzoneFile!.name);
-                    await getNamespaceDetails();
+    const handleApplyLogo = () => {
+        const avatarEditor = editor.current as AvatarEditor;
+        const canvasScaled = avatarEditor.getImageScaledToCanvas();
+        canvasScaled.toBlob(async (blob) => {
+            if (blob) {
+                if (logoPreview) {
+                    URL.revokeObjectURL(logoPreview);
                 }
-            });
-            setEditing(false);
-        }
+                setLogoPreview(URL.createObjectURL(blob));
+
+                if (newDetails) {
+                    const details = copy(newDetails);
+                    details.logo = dropzoneFile!.name;
+                    setNewDetails(details);
+                }
+            }
+        });
+        setEditing(false);
     };
 
     const adjustScale = (x: number) => {
@@ -420,8 +435,8 @@ export const UserNamespaceDetails: FunctionComponent<UserNamespaceDetailsProps>
                 </Button>
                 <Button
                     autoFocus
-                    onClick={handleSaveLogo} >
-                    Save logo
+                    onClick={handleApplyLogo} >
+                    Apply logo
                 </Button>
             </DialogActions>
         </Dialog>
@@ -603,7 +618,7 @@ export const UserNamespaceDetails: FunctionComponent<UserNamespaceDetailsProps>
                 </Grid>
             </Grid>
             <Grid item xs={12} sx={{ display: 'flex', justifyContent: 'flex-end' }}>
-                <Button sx={{ ml: { xs: 2, sm: 2, md: 2, lg: 0, xl: 0 } }} variant='outlined' disabled={_.isEqual(currentDetails, newDetails)} onClick={setNamespaceDetails}>
+                <Button sx={{ ml: { xs: 2, sm: 2, md: 2, lg: 0, xl: 0 } }} variant='outlined' disabled={noChanges} onClick={setNamespaceDetails}>
                     Save Namespace Details
                 </Button>
             </Grid>