Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions agent/app/dto/response/website.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,9 @@ type Resource struct {
}

type Database struct {
Name string `json:"name"`
Type string `json:"type"`
From string `json:"from"`
ID uint `json:"id"`
Name string `json:"name"`
DatabaseName string `json:"databaseName"`
Type string `json:"type"`
From string `json:"from"`
ID uint `json:"id"`
}
4 changes: 3 additions & 1 deletion agent/app/service/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,16 +192,18 @@ func (a AppService) GetApp(ctx *gin.Context, key string) (*response.AppDTO, erro
}
var versionsRaw []string
hasLatest := false
latestVersion := ""
for _, detail := range details {
if strings.Contains(detail.Version, "latest") {
hasLatest = true
latestVersion = detail.Version
continue
}
versionsRaw = append(versionsRaw, detail.Version)
}
appDTO.Versions = common.GetSortedVersions(versionsRaw)
if hasLatest {
appDTO.Versions = append([]string{"latest"}, appDTO.Versions...)
appDTO.Versions = append([]string{latestVersion}, appDTO.Versions...)
}
tags, err := getAppTags(app.ID, strings.ToLower(common.GetLang(ctx)))
if err != nil {
Expand Down
27 changes: 13 additions & 14 deletions agent/app/service/app_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -1834,19 +1834,18 @@ func handleOpenrestyFile(appInstall *model.AppInstall) error {
break
}
}
if !hasDefaultWebsite {
return nil
}
installDir := appInstall.GetPath()
defaultConfigPath := path.Join(installDir, "conf", "default", "00.default.conf")
fileOp := files.NewFileOp()
content, err := fileOp.GetContent(defaultConfigPath)
if err != nil {
return err
}
newContent := strings.ReplaceAll(string(content), "default_server", "")
if err := fileOp.WriteFile(defaultConfigPath, strings.NewReader(newContent), constant.FilePerm); err != nil {
return err
if hasDefaultWebsite {
installDir := appInstall.GetPath()
defaultConfigPath := path.Join(installDir, "conf", "default", "00.default.conf")
fileOp := files.NewFileOp()
content, err := fileOp.GetContent(defaultConfigPath)
if err != nil {
return err
}
newContent := strings.ReplaceAll(string(content), "default_server", "")
if err := fileOp.WriteFile(defaultConfigPath, strings.NewReader(newContent), constant.FilePerm); err != nil {
return err
}
}
return createAllWebsitesWAFConfig()
return createAllWebsitesWAFConfig(websites)
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided code is almost identical between the two versions, differing only in the placement of the call to createAllWebsitesWAFConfig. Here are my recommendations:

  • Refactor Code: Consider refactoring this code into a single function called processDefaultConfig that handles both cases where no default website is present and when it exists. This would reduce redundancy and make the code cleaner.

Here's an example of how you might refactor the code:

func processDefaultConfig(appInstall *model.AppInstall) error {
    installDir := appInstall.GetPath()
    defaultConfigPath := path.Join(installDir, "conf", "default", "00.default.conf")
    fileOp := files.NewFileOp()

    if !hasDefaultWebsite {
        return nil
    }

    content, err := fileOp.GetContent(defaultConfigPath)
    if err != nil {
        return err
    }

    newContent := strings.ReplaceAll(string(content), "default_server", "")
    if err := fileOp.WriteFile(defaultConfigPath, strings.NewReader(newContent), constant.FilePerm); err != nil {
        return err
    }

    if websites == nil {
        return createAllWebsitesWAFConfig(websites)
    } else {
        // Proceed with other processing using websites
        // ...
    }

    return nil
}

// Example usage:
if hasDefaultWebsite {
    if err := processDefaultConfig(appInstall); err != nil {
        return err
    }
} else {
    // Handle case without predefined configurations
    // ...
}

This refactoring makes sense because most of the logic remains similar whether there is a default website defined or not, thus reducing the duplication in the original code.

18 changes: 10 additions & 8 deletions agent/app/service/website.go
Original file line number Diff line number Diff line change
Expand Up @@ -3210,10 +3210,11 @@ func (w WebsiteService) ListDatabases() ([]response.Database, error) {
database, _ := databaseRepo.Get(repo.WithByName(db.MysqlName))
if database.ID > 0 {
res = append(res, response.Database{
ID: db.ID,
Name: db.Name,
Type: database.Type,
From: database.From,
ID: db.ID,
Name: db.Name,
Type: database.Type,
From: database.From,
DatabaseName: database.Name,
})
}
}
Expand All @@ -3222,10 +3223,11 @@ func (w WebsiteService) ListDatabases() ([]response.Database, error) {
database, _ := databaseRepo.Get(repo.WithByName(db.PostgresqlName))
if database.ID > 0 {
res = append(res, response.Database{
ID: db.ID,
Name: db.Name,
Type: database.Type,
From: database.From,
ID: db.ID,
Name: db.Name,
Type: database.Type,
From: database.From,
DatabaseName: database.Name,
})
}
}
Expand Down
6 changes: 1 addition & 5 deletions agent/app/service/website_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,11 +297,7 @@ func moveDefaultWafConfig(websiteDir string, defaultConfigContent []byte, defaul
return nil
}

func createAllWebsitesWAFConfig() error {
websites, _ := websiteRepo.List()
if len(websites) == 0 {
return nil
}
func createAllWebsitesWAFConfig(websites []model.Website) error {
nginxInstall, err := getAppInstallByKey(constant.AppOpenresty)
if err != nil {
return err
Expand Down
5 changes: 2 additions & 3 deletions agent/utils/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,24 +196,24 @@ func (c Client) PullImageWithProcessAndOptions(task *task.Task, imageName string
}
return err
}
timeStr := time.Now().Format("2006/01/02 15:04:05")
status, _ := progress["status"].(string)
if status == "Downloading" || status == "Extracting" {
id, _ := progress["id"].(string)
progressDetail, _ := progress["progressDetail"].(map[string]interface{})
current, _ := progressDetail["current"].(float64)
progressStr := ""
total, ok := progressDetail["total"].(float64)
timeStr := time.Now().Format("2006/01/02 15:04:05")
if ok {
progressStr = fmt.Sprintf("%s %s [%s] --- %.2f%%", timeStr, status, id, (current/total)*100)
} else {
progressStr = fmt.Sprintf("%s %s [%s] --- %.2f%%", timeStr, status, id, current)
}

_ = setLog(id, progressStr, task)
}
if status == "Pull complete" || status == "Download complete" {
id, _ := progress["id"].(string)
timeStr := time.Now().Format("2006/01/02 15:04:05")
progressStr := fmt.Sprintf("%s %s [%s] --- %.2f%%", timeStr, status, id, 100.0)
_ = setLog(id, progressStr, task)
}
Expand Down Expand Up @@ -312,7 +312,6 @@ func (c Client) BuildImageWithProcessAndOptions(task *task.Task, tar io.ReadClos
} else {
progressStr = fmt.Sprintf("%s %s [%s] --- %.2f%%", timeStr, status, id, current)
}

_ = setLog(id, progressStr, task)
case "Pull complete", "Download complete", "Verifying Checksum":
id, _ := progress["id"].(string)
Expand Down
1 change: 1 addition & 0 deletions frontend/src/api/interface/website.ts
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,7 @@ export namespace Website {
databaseID: number;
websiteID: number;
from: string;
databaseName: number;
}

export interface ChangeDatabase {
Expand Down
7 changes: 3 additions & 4 deletions frontend/src/components/log/task/index.vue
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
<template>
<el-dialog
v-model="open"
:destroy-on-close="true"
:close-on-click-modal="false"
:close-on-press-escape="false"
:show-close="showClose"
:before-close="handleClose"
@close="handleClose"
:width="width"
>
<div>
Expand Down Expand Up @@ -44,7 +43,6 @@ const config = reactive({
});
const open = ref(false);
const showTail = ref(true);
const emit = defineEmits(['close']);

const openWithTaskID = (id: string, tail: boolean) => {
config.taskID = id;
Expand All @@ -64,9 +62,10 @@ const openWithResourceID = (taskType: string, taskOperate: string, resourceID: n
open.value = true;
};

const em = defineEmits(['close']);
const handleClose = () => {
em('close', true);
open.value = false;
emit('close', true);
bus.emit('refreshTask', true);
};

Expand Down
33 changes: 30 additions & 3 deletions frontend/src/views/app-store/installed/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -273,19 +273,46 @@
<table>
<tbody>
<tr v-if="defaultLink != ''">
<td>
<td v-if="installed.httpPort > 0">
<el-button
type="primary"
link
@click="
toLink(
defaultLink +
'http://' +
defaultLink +
':' +
installed.httpPort,
)
"
>
{{ defaultLink + ':' + installed.httpPort }}
{{
'http://' +
defaultLink +
':' +
installed.httpPort
}}
</el-button>
</td>
<td v-if="installed.httpsPort > 0">
<el-button
type="primary"
link
@click="
toLink(
'https://' +
defaultLink +
':' +
installed.httpsPort,
)
"
>
{{
'https://' +
defaultLink +
':' +
installed.httpsPort
}}
</el-button>
</td>
</tr>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code appears to fix an issue where the HTTP button was not being shown when there is no valid installation information, but it may still be problematic if defaultLink is not provided or has invalid values. Additionally, it's good practice to validate the ports before attempting to create URLs since they should ideally be numbers greater than zero.

Here are some suggestions:

  1. Validate Ports: Before constructing the URL in the <el-button>'s @click handler, ensure that both HTTP and HTTPS port properties of installed have valid numeric values (greater than zero).

  2. Check Link Existence: The condition inside <td v-if="defaultLink != ''"> ensures that at least one link component will appear, even if neither HTTP nor HTTPS ports have valid data. However, this might not be optimal for clarity; you can consider showing only one type of link based on whether the other exists.

  3. Use Template Literals: Your use of template literals ({{ }}) within templates is correct, which helps with readability and prevents escaping issues.

  4. Comments for Clarity: Adding comments could help clarify the purpose of certain sections of the code, especially if the logic becomes more complex without additional documentation.

Overall, the changes address a common mistake in UI development related to conditional rendering and URL construction, ensuring robustness against invalid inputs.

Expand Down
20 changes: 3 additions & 17 deletions frontend/src/views/app-store/setting/default-domain/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,7 @@
<DrawerPro v-model="drawerVisible" :header="$t('app.defaultWebDomain')" @close="handleClose" size="small">
<el-form ref="formRef" label-position="top" :model="form" :rules="rules" @submit.prevent v-loading="loading">
<el-form-item :label="$t('app.defaultWebDomain')" prop="defaultDomain">
<el-input v-model="form.defaultDomain">
<template #prepend>
<el-select v-model="protocol" placeholder="Select" class="p-w-100">
<el-option label="HTTP" value="http://" />
<el-option label="HTTPS" value="https://" />
</el-select>
</template>
</el-input>
<el-input v-model="form.defaultDomain"></el-input>
<span class="input-help">{{ $t('app.defaultWebDomainHepler') }}</span>
</el-form-item>
</el-form>
Expand All @@ -36,18 +29,15 @@ const form = reactive({
defaultDomain: '',
});
const rules = reactive({
defaultDomain: [Rules.requiredInput],
defaultDomain: [Rules.requiredInput, Rules.ipV4V6OrDomain],
});
const formRef = ref<FormInstance>();
const protocol = ref('http://');
interface DialogProps {
protocol: string;
domain: string;
}

const acceptParams = (config: DialogProps): void => {
form.defaultDomain = config.domain;
protocol.value = config.protocol;
drawerVisible.value = true;
};

Expand All @@ -64,12 +54,8 @@ const submit = async () => {
}
loading.value = true;
try {
let defaultDomain = '';
if (form.defaultDomain) {
defaultDomain = protocol.value + form.defaultDomain;
}
const req = {
defaultDomain: defaultDomain,
defaultDomain: form.defaultDomain,
};
await updateAppStoreConfig(req);
MsgSuccess(i18n.global.t('commons.msg.updateSuccess'));
Expand Down
29 changes: 1 addition & 28 deletions frontend/src/views/app-store/setting/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,6 @@
<el-col :xs="24" :sm="20" :md="15" :lg="12" :xl="12">
<el-form-item :label="$t('app.defaultWebDomain')" prop="defaultDomain">
<el-input v-model="config.defaultDomain" disabled>
<template #prepend>
<el-select v-model="protocol" placeholder="Select" class="p-w-100" disabled>
<el-option label="HTTP" value="http://" />
<el-option label="HTTPS" value="https://" />
</el-select>
</template>
<template #append>
<el-button @click="setDefaultDomain()" icon="Setting">
{{ $t('commons.button.set') }}
Expand Down Expand Up @@ -54,35 +48,15 @@ const config = ref({
});
const loading = ref(false);
const configForm = ref();
const protocol = ref('http://');
const domainRef = ref();
const useCustomApp = ref(false);

function getUrl(url: string) {
const regex = /^(https?:\/\/)(.*)/;
const match = url.match(regex);
if (match) {
const protocol = match[1];
const remainder = match[2];
return {
protocol: protocol,
remainder: remainder,
};
} else {
return null;
}
}

const search = async () => {
loading.value = true;
try {
const res = await getAppStoreConfig();
if (res.data.defaultDomain != '') {
const url = getUrl(res.data.defaultDomain);
if (url) {
config.value.defaultDomain = url.remainder;
protocol.value = url.protocol;
}
config.value.defaultDomain = res.data.defaultDomain;
}
} catch (error) {
} finally {
Expand All @@ -93,7 +67,6 @@ const search = async () => {
const setDefaultDomain = () => {
domainRef.value.acceptParams({
domain: config.value.defaultDomain,
protocol: protocol.value,
});
};

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reviewed the code and identified several improvements to make it more robust, clean, and efficient:

  1. Remove getUrl Function: The function is not used anywhere in the component and can be removed.

  2. Consolidate URL Handling: Move the logic to handle default domain retrieval into the search method, where it's currently used but not consistently applied across all components/functions.

  3. Simplify Protocol Management: Since there are only two options ('HTTP' and 'HTTPS'), consider using a simple enum or switch statement instead of setting a separate ref variable.

  4. Use TypeScript Types for Configuation Object: Ensure that the data returned from getAppStoreConfig() matches what is expected with appropriate types, such as string.

Here’s an optimized version of your code considering these changes:

<template>
  <el-form :model="config">
    <el-col :xs="24" :sm="20" :md="15" :lg="12" :xl="12">
      <el-form-item :label="$t('app.defaultWebDomain')" prop="defaultDomain">
        <el-input v-model="config.defaultDomain" disabled>
          <!-- No need for prepend slot -->
          <template #append>
            <el-button @click="setDefaultDomain()" icon="Setting">{{ $t('commons.button.set') }}</el-button>
          </template>
        </el-input>
      </el-form-item>
    </el-col>
    <!-- ... (rest of the template remains unchanged) ... -->
  </el-form>
</template>

<script lang='ts'>
import { defineComponent, ref } from 'vue';
import { getAppStoreConfig } from '@/api'; // Adjust import path according your project structure

export default defineComponent({
  setup() {
    const loading = ref(false);
    const configForm = ref();
    const config = ref({ defaultDomain: '' }); // Initialize with empty string
    const useCustomApp = ref(false);

    const search = async () => {
      try {
        loading.value = true;
        const res = await getAppStoreConfig();
        if (res.data.defaultDomain && !res.data.defaultDomain.startsWith('/')) {
          config.value.defaultDomain = '/' + res.data.defaultDomain; // Add leading slash if missing
          /* You might want to update the protocol here based on response */
        }
      } catch (error) {
        console.error(error); // Consider better error handling
      } finally {
        loading.value = false;
      }
    };

    const setDefaultDomain = () => {
      domainRef.value.acceptParams({
        domain: config.value.defaultDomain.replace(/^\/|\/$/, ''), // Strip optional leading/trailing slashes
        /* Use custom protocol handling or leave blank if always HTTPS */
      });
    };

    // Fetch config when needed or load page initially if required
    search();

    return {
      loading,
      config Form,
      config,
      search,
      setDefaultDomain,
      useCustomApp
    };
  }
});
</script>

Key Changes:

  • Removed unnecessary getUrl function.
  • Combined default domain retrieval logic into the search method.
  • Simplified setDefaultDomain by stripping optional trailing/leading slashes.
  • Used a ref object for configuration (config) and initialized it with an empty string.

Expand Down
17 changes: 6 additions & 11 deletions frontend/src/views/website/website/config/basic/resource/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
<div class="flex justify-between items-center">
<span>{{ item.name }}</span>
<div>
<el-tag>{{ item.type }}</el-tag>
<el-tag>{{ item.databaseName }}</el-tag>
<el-tag class="ml-1">
{{ item.from === 'local' ? $t('commons.table.local') : $t('database.remote') }}
</el-tag>
Expand Down Expand Up @@ -79,16 +79,11 @@ const listDatabases = async () => {
try {
const res = await getWebsiteDatabase();
databases.value = res.data;
if (databases.value.length > 0) {
if (website.value.dbID > 0) {
for (let i = 0; i < databases.value.length; i++) {
if (
databases.value[i].id === website.value.dbID &&
databases.value[i].type === website.value.dbType
) {
req.db = databases.value[i].id + databases.value[i].type;
break;
}
if (databases.value.length > 0 && website.value.dbID > 0) {
for (let i = 0; i < databases.value.length; i++) {
if (databases.value[i].id === website.value.dbID && databases.value[i].type === website.value.dbType) {
req.db = databases.value[i].id + databases.value[i].type;
break;
}
}
}
Expand Down
Loading