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
1 change: 1 addition & 0 deletions agent/app/dto/request/website_ssl.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import "github.com/1Panel-dev/1Panel/agent/app/dto"
type WebsiteSSLSearch struct {
dto.PageInfo
AcmeAccountID string `json:"acmeAccountID"`
Domain string `json:"domain"`
}

type WebsiteSSLCreate struct {
Expand Down
6 changes: 6 additions & 0 deletions agent/app/repo/website_ssl.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ type ISSLRepo interface {
WithByAcmeAccountId(acmeAccountId uint) DBOption
WithByDnsAccountId(dnsAccountId uint) DBOption
WithByCAID(caID uint) DBOption
WithByDomain(domain string) DBOption
Page(page, size int, opts ...DBOption) (int64, []model.WebsiteSSL, error)
GetFirst(opts ...DBOption) (*model.WebsiteSSL, error)
List(opts ...DBOption) ([]model.WebsiteSSL, error)
Expand Down Expand Up @@ -51,6 +52,11 @@ func (w WebsiteSSLRepo) WithByCAID(caID uint) DBOption {
return db.Where("ca_id = ?", caID)
}
}
func (w WebsiteSSLRepo) WithByDomain(domain string) DBOption {
return func(db *gorm.DB) *gorm.DB {
return db.Where("primary_domain Like ? or domains Like ?", "%"+domain+"%", "%"+domain+"%")
}
}

func (w WebsiteSSLRepo) Page(page, size int, opts ...DBOption) (int64, []model.WebsiteSSL, error) {
var sslList []model.WebsiteSSL
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.

This is a code snippet from an interface (ISSLRepo) and its implementations for querying SSL websites based on various conditions. The changes made include adding a new method WithByDomain to allow filtering by domain name, modifying the existing Page function slightly to incorporate pagination parameters, and implementing error handling where necessary.

Irregularities/Issues:

  • Error Handling for Pagination: The Page function currently returns two values (the total count and a slice of models). While this pattern can be useful, it might not work correctly if errors occur during database operations. Consider wrapping these in a single return value with custom types that handle both data and potential errors.

Optimization Suggestions:

  1. Indexing: If using SQLite, consider creating indexes on columns used frequently in WHERE clauses like acme_account_id, dns_account_id, ca_id, and primary_domain. This can significantly speed up queries.

  2. Consistency in Functionality: Ensure consistency across all filter functions regarding whether they return the original model or a modified model with additional fields. For example, use the same approach for returning the "first" model as you do for paginated results.

  3. Performance Tuning: Depending on the complexity of your query and usage patterns, consider optimizing SQL performance by analyzing slow queries, rewriting complex queries into simpler ones, or caching common query results.

  4. Refactoring for Readability: Break down larger functions or methods into smaller, more focused parts if they become difficult to manage visually. This improves readability and maintainability.

These suggestions aim to improve the robustness, efficiency, and overall quality of the codebase managing SSL website records.

Expand Down
2 changes: 1 addition & 1 deletion agent/app/service/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -935,7 +935,7 @@ func (a AppService) SyncAppListFromRemote(taskID string) (err error) {
t.Log(i18n.GetMsgByKey("AppStoreIsSyncing"))
return nil
}
t.Log(i18n.GetMsgByKey("AppStoreIsLastVersion"))
t.Log(i18n.GetMsgByKey("AppStoreIsUpToDate"))
return nil
}
list := &dto.AppList{}
Expand Down
7 changes: 6 additions & 1 deletion agent/app/service/website_ssl.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,13 @@ func NewIWebsiteSSLService() IWebsiteSSLService {
func (w WebsiteSSLService) Page(search request.WebsiteSSLSearch) (int64, []response.WebsiteSSLDTO, error) {
var (
result []response.WebsiteSSLDTO
opts []repo.DBOption
)
total, sslList, err := websiteSSLRepo.Page(search.Page, search.PageSize, repo.WithOrderBy("created_at desc"))
opts = append(opts, repo.WithOrderBy("created_at desc"))
if search.Domain != "" {
opts = append(opts, websiteSSLRepo.WithByDomain(search.Domain))
}
total, sslList, err := websiteSSLRepo.Page(search.Page, search.PageSize, opts...)
if err != nil {
return 0, nil, err
}
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 does not contain any significant irregularities or major issues, but there are a few optimizations and improvements that could be made:

  1. Consistent Interface: Ensure that Page method's return types and parameters follow the same consistency with other methods in the WebsiteSSLService. This can make the interface more intuitive.

  2. Null Domain Handling: Consider adding null checks for the search.Domain before appending it to opts. If the domain is empty, you might want to skip this option rather than filtering directly which can improve performance.

if search.Domain == "" {
    // Skip this filter if the domain is not provided
} else {
    opts = append(opts, websiteSSLRepo.WithByDomain(search.Domain))
}
  1. Variable Declaration: You can declare all variables at the beginning of the method to reduce cognitive complexity and improve readability.

  2. Use of Variadic Parameters: Instead of manually managing variadic arguments via slices, consider using Go's built-in variadic functions where applicable. For example:

total, sslList, err := websiteSSLRepo.Page(search.Page, search.PageSize, opts...)

This way, changes like additional sorting criteria can be easily added without modifying multiple calls to the append function.

  1. Doc Comments: Add comments explaining each step of the logic within Page method to enhance its documentation and maintainability.

  2. Error Handling Flexibility: Consider returning detailed error messages that include information about what caused the error, particularly when dealing with non-specific errors like "failed to fetch data".

Here's a slightly refactored version incorporating some of these suggestions:

func (w WebsiteSSLService) Page(search request.WebsiteSSLSearch) (int64, []response.WebsiteSSLDTO, error) {
	opts := []repo.DBOption{
		repo.WithOrderBy("created_at desc"),
	}

	if search.Domain != "" {
		opts = append(opts, websiteSSLRepo.WithByDomain(search.Domain))
	}

	total, sslList, err := websiteSSLRepo.PageWithOptions(search.Page, search.PageSize, opts...)

	if err != nil && total > 0 {
		err = fmt.Errorf("%d records returned while fetching SSL data failed: %v", total, err)
	}

	return total, sslList, err
}

These suggestions will help make the Page method more robust, readable, and maintainable.

Expand Down
1 change: 0 additions & 1 deletion agent/i18n/lang/zh.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ PullImageStart: "开始拉取镜像 {{ .name }}"
PullImageSuccess: "镜像拉取成功"
UpgradeAppStart: "开始升级应用 {{ .name }}"
UpgradeAppSuccess: "应用 {{ .name }} 升级成功"
AppStoreIsLastVersion: "应用商店已经是最新版本"
AppStoreSyncSuccess: "应用商店同步成功"
SyncAppDetail: "同步应用配置"
AppVersionNotMatch: "{{ .name }} 应用需要更高的 1Panel 版本,跳过同步"
Expand Down
43 changes: 24 additions & 19 deletions frontend/src/views/website/ssl/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,15 @@
</el-button>
</template>
<template #rightToolBar>
<TableSearch @search="search()" v-model:searchName="req.domain" />
<TableRefresh @search="search()" />
<fu-table-column-select
:columns="columns"
trigger="hover"
:title="$t('commons.table.selectColumn')"
popper-class="popper-class"
:only-icon="true"
/>
</template>
<template #main>
<ComplexTable
Expand All @@ -32,41 +40,33 @@
@search="search()"
v-model:selects="selects"
v-loading="loading"
:columns="columns"
localKey="sslColumn"
>
<el-table-column type="selection" width="30" />
<el-table-column
:label="$t('website.domain')"
fix
show-overflow-tooltip
prop="primaryDomain"
min-width="150px"
></el-table-column>
<el-table-column
:label="$t('website.otherDomains')"
fix
show-overflow-tooltip
prop="domains"
min-width="90px"
></el-table-column>
<el-table-column
:label="$t('ssl.applyType')"
fix
show-overflow-tooltip
prop="provider"
min-width="110px"
>
<el-table-column :label="$t('ssl.applyType')" show-overflow-tooltip prop="provider" width="90px">
<template #default="{ row }">{{ getProvider(row.provider) }}</template>
</el-table-column>
<el-table-column
:label="$t('ssl.acmeAccount')"
fix
show-overflow-tooltip
prop="acmeAccount.email"
min-width="110px"
width="150px"
></el-table-column>
<el-table-column
:label="$t('commons.table.status')"
fix
show-overflow-tooltip
prop="status"
width="100px"
Expand Down Expand Up @@ -94,7 +94,7 @@
</div>
</template>
</el-table-column>
<el-table-column :label="$t('commons.button.log')" width="100px">
<el-table-column :label="$t('commons.button.log')" width="80px">
<template #default="{ row }">
<el-button @click="openSSLLog(row)" link type="primary" v-if="row.provider != 'manual'">
{{ $t('website.check') }}
Expand All @@ -103,11 +103,11 @@
</el-table-column>
<el-table-column
:label="$t('website.brand')"
fix
show-overflow-tooltip
prop="organization"
width="150px"
></el-table-column>
<el-table-column :label="$t('website.remark')" fix prop="description" min-width="100px">
<el-table-column :label="$t('website.remark')" prop="description" width="100px">
<template #default="{ row }">
<fu-read-write-switch>
<template #read>
Expand All @@ -119,7 +119,7 @@
</fu-read-write-switch>
</template>
</el-table-column>
<el-table-column :label="$t('ssl.autoRenew')" fix min-width="100px">
<el-table-column :label="$t('ssl.autoRenew')" width="100px">
<template #default="{ row }">
<el-switch
:disabled="row.provider === 'dnsManual' || row.provider === 'manual'"
Expand All @@ -133,7 +133,7 @@
:label="$t('website.expireDate')"
:formatter="dateFormat"
show-overflow-tooltip
width="200px"
width="180px"
/>
<fu-table-operations
:ellipsis="3"
Expand Down Expand Up @@ -198,6 +198,10 @@ const logRef = ref();
const caRef = ref();
const obtainRef = ref();
let selects = ref<any>([]);
const columns = ref([]);
const req = reactive({
domain: '',
});

const routerButton = [
{
Expand Down Expand Up @@ -286,12 +290,13 @@ const mobile = computed(() => {
});

const search = () => {
const req = {
const request = {
page: paginationConfig.currentPage,
pageSize: paginationConfig.pageSize,
domain: req.domain,
};
loading.value = true;
searchSSL(req)
searchSSL(request)
.then((res) => {
data.value = res.data.items || [];
paginationConfig.total = res.data.total;
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.

Here are some observations and suggestions regarding the code:

  1. Empty Array Initialization: The columns array is initialized before being populated with values (ref([])), but this initialization might not be necessary unless it's intended to hold predefined column definitions.

  2. Conditional Check in Template Slot for SSL Log Button: In the table main slot, there's a conditional check for row.provider != 'custom'. If this condition is true, the custom icon will be displayed; otherwise, only regular buttons will appear. It would be better if the logic was simplified or if additional information about row.provider could be provided.

  3. Code Duplication: There’s some repetition around defining columns within different parts of the template. Reusing variables like props or using functions that populate arrays can help avoid code duplication.

  4. Comments on Columns: Comments near each <el-table-column> define what they represent (e.g., "website.domain"). Consider updating these comments based on new translations or changes in data structure.

  5. Inline Templates: Inline templates inside slots such as #default are generally concise and work well when used sparingly. However, consider whether more complex templating needs could benefit from extracting those components into separate reusable Vue components.

  6. Reactive Object Properties: The properties under req (domain, etc.), seem reactive since they are declared with reactive(). Ensure that updates made through one property cause re-renders where needed without inadvertently triggering unnecessary computations elsewhere in the component.

  7. Function Naming Consistency: Function names like getProvider, are prefixed differently between usage (uppercase vs. camel case). This may lead to confusion or inconsistencies, especially if similar function types exist later on.

  8. Data Loading Logic: The use of loading.value to manage loading indicators is clean and straightforward. Make sure that appropriate error handling scenarios have been accounted for (like network errors).

Overall, you're making good progress and most elements look reasonable given their current complexity and functionality requirements. Here's an optimized version focusing on reducing redundancy, improving readability, and ensuring consistency across your component:

// Simplified Column Definition in Script Block
const defaultColumns = [
    { label: $t('website.domain'), prop: 'primaryDomain', min_width: 150 },
    // Add other common columns here
];
columns.value = defaultColumns;

function searchSSL(params) {
    const request = {
        page: paginationConfig.currentPage,
        pageSize: paginationConfig.pageSize,
        ...params,
    };
    
    fetch(sslApiUrl, {
        method: 'GET',
        headers: apiHeaders,
        body: JSON.stringify(request),
    })
        .then(response => response.json())
        .then(res => {
            console.log(res);
            if (!res.success) throw Error(`Error fetching ssl list`);
            
            data.value = res.data.items || [];
            paginationConfig.total = res.data.total;
        })
        .catch(error => {
            loading.value = false;
            handleError();
        });
}

fetchCaList(searchParams)
.then(data => {
...
});

obtainCertification({ domains, email }) 
...
};

By addressing these areas, your code should become cleaner and potentially more efficient while retaining its existing behavior and functionalities. Always validate performance impact and adjust accordingly during development.

Expand Down
Loading