-
Notifications
You must be signed in to change notification settings - Fork 37
Adds extraction links for moonmining observers #319
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 6 commits
cbf8187
d814ea5
445821f
29ca26d
fa30811
71f3a11
fdfde79
ca69bb4
699d1ff
d541eeb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,10 @@ | |
| namespace Seat\Eveapi\Models\Industry; | ||
|
|
||
| use Illuminate\Database\Eloquent\Model; | ||
| use Illuminate\Support\Facades\DB; | ||
| use Seat\Eveapi\Models\Character\CharacterInfo; | ||
| use Seat\Eveapi\Models\Sde\InvType; | ||
| use Seat\Eveapi\Models\Universe\UniverseName; | ||
|
|
||
| /** | ||
| * Class CorporationIndustryMiningObserverData. | ||
|
|
@@ -50,4 +54,63 @@ class CorporationIndustryMiningObserverData extends Model | |
| * @var bool | ||
| */ | ||
| public $incrementing = true; | ||
|
|
||
| /** | ||
| * @inheritdoc | ||
| */ | ||
| protected static function boot() | ||
| { | ||
| parent::boot(); | ||
|
|
||
| static::creating(function ($data) { | ||
| if (! isset($data->extraction_id) || ($data->extraction_id == 0)) { | ||
| $minDate = DB::raw("DATE_SUB('{$data->last_updated}', INTERVAL 5 DAY)"); | ||
|
vo1 marked this conversation as resolved.
Outdated
|
||
| $maxDate = DB::raw("DATE_ADD('{$data->last_updated}', INTERVAL 5 DAY)"); | ||
|
|
||
| $extraction = CorporationIndustryMiningExtraction::select() | ||
| ->where('structure_id', $data->observer_id) | ||
| ->whereBetween('chunk_arrival_time', [$minDate, $maxDate]) | ||
| ->get() | ||
| ->first(); | ||
| if ($extraction) { | ||
| $data->extraction_id = $extraction->id; | ||
| } | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * @return \Illuminate\Database\Eloquent\Relations\BelongsTo | ||
| */ | ||
| public function extraction() | ||
| { | ||
| return $this->belongsTo(CorporationIndustryMiningExtraction::class, 'extraction_id', 'id'); | ||
| } | ||
|
|
||
| /** | ||
| * @return \Illuminate\Database\Eloquent\Relations\BelongsTo | ||
| */ | ||
| public function character() | ||
| { | ||
| return $this->belongsTo(CharacterInfo::class, 'character_id', 'character_id'); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since corporation mining data can also track non corporate character, it would be better to make this relationship to
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added separate link to universe_name since both should be present in my opinion. What do you think?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem you run into is that by splitting into two relationships, you now need to decide which one to call at runtime. What is the application you are using the relationship for? For name resolution, you are much better-suited sourcing from universe_name as there will be more entries. For linking back to users as I suspect you are doing, this raises an interesting question... As we cannot link back to the character from universe_name without doing individual DB lookups this would be quite expensive.. Makes me have thoughts about polymorphic relationships from UniverseName back to a relevant class (but that is not in scope of this PR). WIll defer to @warlof on this one.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm thinking about a poly for a while, but there are few issues using it :
you mostly get the primary question in your concern, what will be the purpose of that relation ? And there, the answer is : he's looking for a way to group entries per user - which is not possible on behalf against having both
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, the idea is to group. |
||
| } | ||
|
|
||
| /** | ||
| * @return \Illuminate\Database\Eloquent\Relations\BelongsTo | ||
| */ | ||
| public function unverse_name() | ||
|
vo1 marked this conversation as resolved.
Outdated
|
||
| { | ||
| return $this->belongsTo(UniverseName::class, 'character_id', 'entity_id'); | ||
| } | ||
|
|
||
| /** | ||
| * @return \Illuminate\Database\Eloquent\Relations\HasOne | ||
| */ | ||
| public function type() | ||
| { | ||
| return $this->hasOne(InvType::class, 'typeID', 'type_id') | ||
| ->withDefault([ | ||
| 'typeName' => trans('seat::web.unknown'), | ||
| ]); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| <?php | ||
|
|
||
| /* | ||
| * This file is part of SeAT | ||
| * | ||
| * Copyright (C) 2015 to 2021 Leon Jacobs | ||
| * | ||
| * This program is free software; you can redistribute it and/or modify | ||
| * it under the terms of the GNU General Public License as published by | ||
| * the Free Software Foundation; either version 2 of the License, or | ||
| * (at your option) any later version. | ||
| * | ||
| * This program is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| * GNU General Public License for more details. | ||
| * | ||
| * You should have received a copy of the GNU General Public License along | ||
| * with this program; if not, write to the Free Software Foundation, Inc., | ||
| * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. | ||
| */ | ||
|
|
||
| use Illuminate\Database\Migrations\Migration; | ||
| use Illuminate\Database\Schema\Blueprint; | ||
| use Illuminate\Support\Facades\Schema; | ||
|
|
||
| /** | ||
| * Class AddExtractionIdToCorporationIndustryMiningObserverData. | ||
| */ | ||
| class AddExtractionIdToCorporationIndustryMiningObserverData extends Migration | ||
| { | ||
| public function up() | ||
| { | ||
| Schema::table('corporation_industry_mining_observer_data', function (Blueprint $table) { | ||
| $table->integer('extraction_id')->default(0); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is a problematic foreign key. I don't know yet how to handle that though.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We basically have two options here, the first is to create a key to the extraction_id as has been done. The second is to base the relationship in the extractions model which will grab all mining data with that structure_id and between the known datetimes of those extractions. In my dev env I had done the latter.. It hurt performance slightly, but kept the DB free of the extra key. The benefit of the relationship though is you can be much more sure you have the correct extraction as it is based on the dates reported by eve. The problem with the key that needs to be addressed is the interval used to find the extraction. It should not be plus-minus 5 days. The reason being is you may get overlapping extractions with this range. The minimum drill time is 6 days, which can be started as soon as the current chunk is fractured. An unrigged structure will have an asteroid field that lasts 48h. Combined with the The other issue, is that if the extraction does not get populated in the DB prior to the ObserverData then the extraction_id resolution occurring in the boot of the ObserverData model will fail to find an extraction_id and will never try again to populate this, losing the link in the data. The only way then to resolve that would be to add a correction job that runs periodically and attaches all orphaned data to extraction_ids if possible. This in my opinion would be clunky. My proposal instead would be to add an index to last_updated, and then create the relationship in the Extraction to the ObserverData based on matching structure_id and use the days of the extraction key dates (plus 1 on the end) as bounds.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it tries to find date by chunk_arrival_time BETWEEN (6 .. last_updated .. 6)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem is that last_updated will continue to update right up until the mining stops. This means it will slowly travel from the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, what we're sure about is :
Early I said : corporation mining has not be implemented because it's not robust yet - but it appears none of you put this in consideration and mostly assume our modeling is right. What if the observer is falsy recorded in our current architecture ?
We already know that Based on this, we can notice all So now, last question, what is the relation between observer and extraction ? Is there something else than the structure ID ? Like, we get an event every time an extraction cycle is pass (start, chunk arrival, explode/natural decay time) ? Since I'm non longer involving in a corporation, I will not be able to check those upper theory. But, I think it could be interesting to apply those few refactor on a dev env, collect data. Beeing able to validate all upper could help a lot to achieve what you're trying to do without all those complexe things.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My corporation is having one moon per day with max cycle. So far this code works good.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @warlof You are right. We have not discussed if the structure we have in the DB is the best structure. What we have at the moment mirrors the results we get back from ESI. This data structure is more flexible for possible future expansion as hinted at by the This table is the one @vo1 wanted to deprecate as at the moment it serves little purpose currently. The reason it is of little use for the application of this PR is due to the fact that currently, the only
@vo1 To address your last post see below.
Congrats on running so many moons, but this is not the issue I presented to you above. The issue with the 6-day interval you have chosen is when the moon drills are set to minimum cycle, with the rig for maximum asteroid field life. Then you run into problems with a 6-day interval overlapping across multiple
The fact that
Yes, with the
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I lowered to 5 days since 3d I think is kinda unsafe. |
||
| }); | ||
| } | ||
|
|
||
| public function down() | ||
| { | ||
| Schema::table('corporation_industry_mining_observer_data', function (Blueprint $table) { | ||
| $table->dropColumn('extraction_id'); | ||
| }); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.